| Message ID | 000c01ce4040$e548c840$afda58c0$@yorsh@arm.com |
|---|---|
| State | New |
| Headers | show |
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; >
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;
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. > -----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.