Committed with the suggested info.
Thank you for your review,
Claudiu
On Thu, Apr 11, 2019 at 12:47 AM Andrew Burgess
<andrew.burg...@embecosm.com> wrote:
>
> * Claudiu Zissulescu <claz...@gmail.com> [2019-03-25 12:03:11 +0100]:
>
> > 1.The delay slot scheduler can reschedule some of the frame related
> > instructions resulting in having incorect CFI information. This patch
> > introduces a schedule blockage to avoid this problem.
> >
> > 2.There are cases when an interrupt may happen and not all the current
> > function stack operations are done, which may result in stack
> > corruption. Such an example is accessing an returning a local
> > structure members, which members are allocated on stack. The stack
> > adjustment and the accessing of the struct member can be reorder as
> > they may not use both the SP register for the access.
> >
> > 3.Also, do not save/restore SP when in interrupt. The SP is switch by
> > the core IRQ machinery.
> >
> > gcc/
> > xxxx-xx-xx Claudiu Zissulescu <claz...@synopsys.com>
> >
> > * config/arc/arc.c (arc_expand_prologue): Emit blockage regardless
> > to avoid delay slot scheduling.
> > * config/arc/arc.md (stack_tie): Remove.
> > (UNSPEC_ARC_STKTIE): Likewise.
> > (arc_must_save_register): Don't save SP.
> > (arc_expand_epilogue): Emit blockage.
>
> This looks fine, just one minor issue...
>
> > ---
> > gcc/config/arc/arc.c | 17 +++++++++--------
> > gcc/config/arc/arc.md | 13 -------------
> > 2 files changed, 9 insertions(+), 21 deletions(-)
> >
> > diff --git a/gcc/config/arc/arc.c b/gcc/config/arc/arc.c
> > index fa49c562b46..62f435b0a1d 100644
> > --- a/gcc/config/arc/arc.c
> > +++ b/gcc/config/arc/arc.c
> > @@ -2658,6 +2658,7 @@ arc_must_save_register (int regno, struct function
> > *func)
> >
> > if ((regno) != RETURN_ADDR_REGNUM
> > && (regno) != FRAME_POINTER_REGNUM
> > + && (regno) != STACK_POINTER_REGNUM
> > && df_regs_ever_live_p (regno)
> > && (!call_used_regs[regno]
> > || ARC_INTERRUPT_P (fn_type))
>
> The comment at the head of this function mentions the special handling
> for ra and fp, but wasn't updated with this change. Would it make
> sense to do that?
>
> Thanks,
> Andrew
>
> > @@ -3731,14 +3732,10 @@ arc_expand_prologue (void)
> >
> > /* Allocate the stack frame. */
> > if (frame_size_to_allocate > 0)
> > - {
> > - frame_stack_add ((HOST_WIDE_INT) 0 - frame_size_to_allocate);
> > - /* If the frame pointer is needed, emit a special barrier that
> > - will prevent the scheduler from moving stores to the frame
> > - before the stack adjustment. */
> > - if (arc_frame_pointer_needed ())
> > - emit_insn (gen_stack_tie (stack_pointer_rtx, hard_frame_pointer_rtx));
> > - }
> > + frame_stack_add ((HOST_WIDE_INT) 0 - frame_size_to_allocate);
> > +
> > + /* Emit a blockage to avoid delay slot scheduling. */
> > + emit_insn (gen_blockage ());
> > }
> >
> > /* Do any necessary cleanup after a function to restore stack, frame,
> > @@ -3775,6 +3772,10 @@ arc_expand_epilogue (int sibcall_p)
> > if (!can_trust_sp_p)
> > gcc_assert (arc_frame_pointer_needed ());
> >
> > + /* Emit a blockage to avoid/flush all pending sp operations. */
> > + if (size)
> > + emit_insn (gen_blockage ());
> > +
> > if (TARGET_CODE_DENSITY
> > && TARGET_CODE_DENSITY_FRAME
> > && !ARC_AUTOFP_IRQ_P (fn_type)
> > diff --git a/gcc/config/arc/arc.md b/gcc/config/arc/arc.md
> > index 0482980122d..3a903d4224a 100644
> > --- a/gcc/config/arc/arc.md
> > +++ b/gcc/config/arc/arc.md
> > @@ -136,7 +136,6 @@
> > UNSPEC_ARC_VMAC2HU
> > UNSPEC_ARC_VMPY2H
> > UNSPEC_ARC_VMPY2HU
> > - UNSPEC_ARC_STKTIE
> >
> > VUNSPEC_ARC_RTIE
> > VUNSPEC_ARC_SYNC
> > @@ -6301,18 +6300,6 @@ core_3, archs4x, archs4xd, archs4xd_slow"
> > (set_attr "predicable" "yes,no,no,yes,no")
> > (set_attr "cond" "canuse,nocond,nocond,canuse_limm,nocond")])
> >
> > -(define_insn "stack_tie"
> > - [(set (mem:BLK (scratch))
> > - (unspec:BLK [(match_operand:SI 0 "register_operand" "r")
> > - (match_operand:SI 1 "register_operand" "r")]
> > - UNSPEC_ARC_STKTIE))]
> > - ""
> > - ""
> > - [(set_attr "length" "0")
> > - (set_attr "iscompact" "false")
> > - (set_attr "type" "block")]
> > - )
> > -
> > (define_insn "*add_shift"
> > [(set (match_operand:SI 0 "register_operand" "=q,r,r")
> > (plus:SI (ashift:SI (match_operand:SI 1 "register_operand" "q,r,r")
> > --
> > 2.20.1
> >
