This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 1/2] [ARC] Fix and refurbish the interrupts.


Hi Jeff,

Originally, I had the scheduler barrier as you suggested. However,
there were some user cases when an ISR messed up with SP register
leading to errors. As a solution was to add barriers on either part of
frame operations. However, I would need to recheck the original
rationale of that issue, as it may not be the case with newer cores.

On the other hand, I found a small error of the current patch when one
is having the fno-omit-frame-pointer option in the order how some
auxiliary registers are saved. What will be the best solution having
in mind that I haven't pushed this patch to the mainline yet:
- to have the current patch re-spin?
- to have a separate patch for the new issue.

Thanks,
Claudiu

On Wed, Jul 3, 2019 at 2:15 AM Jeff Law <law@redhat.com> wrote:
>
> On 6/28/19 7:39 AM, Claudiu Zissulescu wrote:
> > When entering an interrupt, not only the call save registers needs to
> > be place on stack but also the call clobbers one. More over, the
> > ARC700 return from interrupt instruction needs to be rtie, the same
> > like ARCv2 CPUs. While the ARC6xx family uses j.f [ilinkX]
> > instruction. Additionally, we need to save the state of the ZOL
> > machinery, namely the lp_count, lp_end and lp_start registers. For
> > architectures which are using extension registers (i.e., HS48) we need
> > to save/restore them as well.
> >
> > Ok to apply?
> > Claudiu
> >
> > gcc/
> > xxxx-xx-xx  Claudiu Zissulescu  <claziss@synopsys.com>
> >
> >       * config/arc/arc-protos.h (arc_output_function_epilogue): Delete
> >       declaration.
> >       (arc_return_address_register): Likewise.
> >       (arc_compute_function_type): Likewise.
> >       (arc_compute_frame_size): Likewise.
> >       (secondary_reload_info): Likewise.
> >       (arc_get_unalign): Likewise.
> >       (arc_can_use_return_insn): Declare.
> >       * config/arc/arc.c (AUX_LP_START): Define
> >       (AUX_LP_END): Likewise.
> >       (arc_frame_info): Update gmask member to 64-bit datum.
> >       (GMASK_LEN): Update.
> >       (arc_compute_function_type): Make it static, move it forward.
> >       (arc_must_save_register): Update, consider the extra regs.
> >       (arc_compute_millicode_save_restore_regs): Update to use the 64
> >       bit gmask.
> >       (arc_compute_frame_size): Likewise.
> >       (arc_enter_leave_p): Likewise.
> >       (arc_save_callee_saves): Likewise.
> >       (arc_restore_callee_saves): Likewise.
> >       (arc_save_callee_enter): Likewise.
> >       (arc_restore_callee_leave): Likewise.
> >       (arc_save_callee_milli): Likewise.
> >       (arc_restore_callee_milli): Likewise.
> >       (arc_expand_prologue): Add new interrupt handling.
> >       (arc_return_address_register): Make it static, move it forward.
> >       (arc_expand_epilogue): Add new interrupt handling.
> >       (arc_get_unalign): Delete.
> >       (arc_epilogue_uses): Make sure we do not remove the extra
> >       saved/restored registers when interrupt.
> >       (arc_can_use_return_insn): New function.
> >       * config/arc/arc.md (VUNSPEC_ARC_ARC600_RTIE): Define.
> >       (R33_REG): Likewise.
> >       (R34_REG): Likewise.
> >       (R35_REG): Likewise.
> >       (R36_REG): Likewise.
> >       (R37_REG): Likewise.
> >       (R38_REG): Likewise.
> >       (R39_REG): Likewise.
> >       (R45_REG): Likewise.
> >       (R46_REG): Likewise.
> >       (R47_REG): Likewise.
> >       (R48_REG): Likewise.
> >       (R49_REG): Likewise.
> >       (R50_REG): Likewise.
> >       (R51_REG): Likewise.
> >       (R52_REG): Likewise.
> >       (R53_REG): Likewise.
> >       (R54_REG): Likewise.
> >       (R55_REG): Likewise.
> >       (R56_REG): Likewise.
> >       (R58_REG): Likewise.
> >       (type): Add rtie attribute.
> >       (in_call_delay_slot): Use RETURN_ADDR_REGNUM.
> >       (movsi_insn): Accept moves to lp_count.
> >       (rtie): Update pattern.
> >       (simple_return): Simplify it, don't use this pattern as a return
> >       from an interrupt.
> >       (arc600_rtie): New pattern.
> >       (p_return_i): Clean up.
> >       (return): Likewise.
> >       * config/arc/builtins.def (rtie): Only available for non ARC6xx
> >       family CPUs.
> >       * config/arc/predicates.md (move_src_operand): Consider lp_count
> >       as a register.
> >
> > gcc/testsuite
> > xxxx-xx-xx  Claudiu Zissulescu  <claziss@synopsys.com>
> >
> >       * gcc.target/arc/arc.exp (check_effective_target_accregs): New
> >       predicate.
> >       * gcc.target/arc/builtin_special.c: Update test/
> >       * gcc.target/arc/interrupt-1.c: Likewise.
> >       * gcc.target/arc/interrupt-10.c: New test.
> >       * gcc.target/arc/interrupt-11.c: Likewise.
> > ---
> > diff --git a/gcc/config/arc/arc.c b/gcc/config/arc/arc.c
> > index 4fec8fd8929..910c8d2ad30 100644
> > --- a/gcc/config/arc/arc.c
> > +++ b/gcc/config/arc/arc.c
> > @@ -206,6 +206,13 @@ static int rgf_banked_register_count;
> >     this to be no less than the 1/p  */
> >  #define MAX_INSNS_SKIPPED 3
> >
> > +/* ZOL control registers.  */
> > +#define AUX_LP_START 0x02
> > +#define AUX_LP_END 0x03
> > +
> > +/* FPX AUX registers.  */
> > +#define AUX_DPFP_START 0x301
> > +
> >  /* A nop is needed between a 4 byte insn that sets the condition codes and
> >     a branch that uses them (the same isn't true for an 8 byte insn that sets
> >     the condition codes).  Set by arc_ccfsm_advance.  Used by
> > @@ -298,6 +305,131 @@ static bool arc_use_by_pieces_infrastructure_p (unsigned HOST_WIDE_INT,
> >  /* Globally visible information about currently selected cpu.  */
> >  const arc_cpu_t *arc_selected_cpu;
> >
> > +/* Traditionally, we push saved registers first in the prologue,
> > +   then we allocate the rest of the frame - and reverse in the epilogue.
> > +   This has still its merits for ease of debugging, or saving code size
> > +   or even execution time if the stack frame is so large that some accesses
> > +   can't be encoded anymore with offsets in the instruction code when using
> > +   a different scheme.
> > +   Also, it would be a good starting point if we got instructions to help
> > +   with register save/restore.
> > +
> > +   However, often stack frames are small, and the pushing / popping has
> > +   some costs:
> > +   - the stack modification prevents a lot of scheduling.
> > +   - frame allocation / deallocation needs extra instructions.
> > +   - unless we know that we compile ARC700 user code, we need to put
> > +     a memory barrier after frame allocation / before deallocation to
> > +     prevent interrupts clobbering our data in the frame.
> Note that you need a scheduler barrier before frame deallocation, but
> not after frame allocation.  At least that's the general case.
>
>
> > +     In particular, we don't have any such guarantees for library functions,
> > +     which tend to, on the other hand, to have small frames.> +
> > +   Thus, for small frames, we'd like to use a different scheme:
> > +   - The frame is allocated in full with the first prologue instruction,
> > +     and deallocated in full with the last epilogue instruction.
> > +     Thus, the instructions in-betwen can be freely scheduled.
> s/betwen/between/
>
>
> > +   - If the function has no outgoing arguments on the stack, we can allocate
> > +     one register save slot at the top of the stack.  This register can then
> > +     be saved simultanously with frame allocation, and restored with
> s/simultanously/simultaneously/
>
>
>
> OK with the nits fixed.
>
> jeff


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]