Re: [PATCH, ARM] Fix PR56797

[Richard Earnshaw <rearnsha at arm dot com>]

On 23/04/13 17:37, Greta Yorsh wrote:
> Ok to backport to gcc4.8?
>
> I'm attaching an updated version - just fixed a spelling error in the
> comment.
>
> Thanks,
> Greta
>
> gcc/ChangeLog
>
> 	PR target/56797
> 	* config/arm/arm.c (load_multiple_sequence): Require SP
>          as base register for loads if SP is in the register list.

OK.

R.

> > -----Original Message-----
> > From: Richard Earnshaw
> > Sent: 19 April 2013 12:34
> > To: Greta Yorsh
> > Cc: GCC Patches; raj.khem@gmail.com; Ramana Radhakrishnan
> > Subject: Re: [PATCH, ARM] Fix PR56797
> >
> > On 19/04/13 10:34, Greta Yorsh wrote:
> > > Fix PR56797
> > > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56797
> > >
> > > The problem is that peephole optimizer thinks it can generate an ldm,
> > but
> > > the pattern for ldm no longer matches, because after r188738 it
> > requires
> > > that if one of the destination registers is SP then the base register
> > must
> > > be SP, and it's not SP in the test case.
> > >
> > > The test case fails on armv5t but doesn't fail on armv6t2 or armv7-a
> > because
> > > peephole doesn't trigger there (because there is a different epilogue
> > > sequence). It looks like a latent problem for other architecture or
> > CPUs.
> > > This patch adds this condition to the peephole optimizer.
> > >
> > > No regression on qemu for arm-none-eabi and fixes the test reported
> > in the
> > > PR. I couldn't minimize the test sufficiently to include it in the
> > > testsuite.
> > >
> > > Ok for trunk?
> > >
> > > Thanks,
> > > Greta
> > >
> > > gcc/
> > >
> > > 2013-04-18  Greta Yorsh  <Greta.Yorsh@arm.com>
> > >
> > > 	PR target/56797
> > > 	* config/arm/arm.c (load_multiple_sequence): Require SP
> > >           as base register for loads if SP is in the register list.
> >
> > OK.
> >
> > R.
>
>
> pr56797-ldm-peep-sp.patch.txt
>
>
> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
> index d00849c..60fef78 100644
> --- a/gcc/config/arm/arm.c
> +++ b/gcc/config/arm/arm.c
> @@ -10347,6 +10347,13 @@ load_multiple_sequence (rtx *operands, int nops, int *regs, int *saved_order,
>   	      || (i != nops - 1 && unsorted_regs[i] == base_reg))
>   	    return 0;
>
> +          /* Don't allow SP to be loaded unless it is also the base
> +             register.  It guarantees that SP is reset correctly when
> +             an LDM instruction is interrupted.  Otherwise, we might
> +             end up with a corrupt stack.  */
> +          if (unsorted_regs[i] == SP_REGNUM && base_reg != SP_REGNUM)
> +            return 0;
> +
>   	  unsorted_offsets[i] = INTVAL (offset);
>   	  if (i == 0 || unsorted_offsets[i] < unsorted_offsets[order[0]])
>   	    order[0] = i;