Patchwork [ARM] Fix PR56797

login
register
mail settings
Submitter Greta Yorsh
Date April 19, 2013, 9:34 a.m.
Message ID <000801ce3ce1$23fbdd60$6bf39820$@yorsh@arm.com>
Download mbox | patch
Permalink /patch/237891/
State New
Headers show

Comments

Greta Yorsh - April 19, 2013, 9:34 a.m.
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.
Richard Earnshaw - April 19, 2013, 11:34 a.m.
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.

Patch

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 interruptted.  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;