PATCH RFA: ARM iWMMXt code can mishandle function return

[Ian Lance Taylor <ian at wasabisystems dot com>]

When the ARM generates code for iWMMXt, it requires the stack pointer
to be aligned to an 8-byte boundary.  If the number of regular 4-byte
ARM registers to be saved on the stack at function entry is odd, that
might leave the stack pointer misaligned.  In that case, the ARM
backend will save an extra register, even though it does not have to
be saved, to maintain the alignment.  This approach is presumably
used, instead of simply adjusting the stack pointer, because the extra
register can be included in the register mask for the stmfd and ldmfd
instructions, thus saving the instructions required to adjust the
stack pointer.  This code appears in arm_compute_save_reg_mask().
There is nothing very wrong with any of this.

However, there is currently a special case in
output_return_instruction().  That function assumes that if IP appears
in the register mask, it must be because it was saved when entering a
function which uses a frame pointer.  In that case, the ARM code will
save the original value of the stack pointer into IP as the first
instruction in the function, and then save IP onto the stack.  It is
then slightly more efficient to restore IP directly into the stack
pointer with the ldmfd instruction, rather than restoring IP as usual
and then moving IP to the stack pointer.  This special case is
implemented in output_return_instruction().  It is also implemented in
arm_output_epilogue(), but in the latter case it is conditioned on
frame_pointer_needed, which is safe.  In output_return_instruction(),
the only condition is ! IS_INTERRUPT (func_type).

This means that if we write a function which uses an odd number of
registers, and if the iWMMXt special case then decides to save IP on
the stack to maintain stack alignment, and if the frame pointer is not
needed, that output_return_instruction() will do the wrong thing.

It turns out to be possible to write such a test case.  The iWMMXt
special case will only save IP if FP is used in the function, so we
need to use the -mno-apcs-frame option to make FP a non-fixed,
non-frame-pointer, register.

This patch fixes the problem.  It also adds a test case.  The test
case is miscompiled by the current ARM compiler.

I've run the compilation tests.  I'm currently trying to see if I can
run the execution tests using the simulator.

OK for mainline if testing passes?

Ian


ChangeLog:

2004-01-27  Ian Lance Taylor  <ian@wasabisystems.com>

	* config/arm/arm.c (output_return_instruction): Only restore IP
	into SP if frame_pointer_needed.

testsuite/ChangeLog:

2004-01-27  Ian Lance Taylor  <ian@wasabisystems.com>

	* gcc.dg/arm-mmx-1.c: New test.


Index: config/arm/arm.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/arm/arm.c,v
retrieving revision 1.321
diff -p -u -r1.321 arm.c
--- config/arm/arm.c	26 Jan 2004 16:35:43 -0000	1.321
+++ config/arm/arm.c	27 Jan 2004 05:30:07 -0000
@@ -8260,15 +8260,25 @@ output_return_instruction (rtx operand, 
 	return_reg = reg_names[LR_REGNUM];
 
       if ((live_regs_mask & (1 << IP_REGNUM)) == (1 << IP_REGNUM))
-	/* There are two possible reasons for the IP register being saved.
-	   Either a stack frame was created, in which case IP contains the
-	   old stack pointer, or an ISR routine corrupted it.  If this in an
-	   ISR routine then just restore IP, otherwise restore IP into SP.  */
-	if (! IS_INTERRUPT (func_type))
-	  {
-	    live_regs_mask &= ~ (1 << IP_REGNUM);
-	    live_regs_mask |=   (1 << SP_REGNUM);
-	  }
+	{
+	  /* There are three possible reasons for the IP register
+	     being saved.  1) a stack frame was created, in which case
+	     IP contains the old stack pointer, or 2) an ISR routine
+	     corrupted it, or 3) it was saved to align the stack on
+	     iWMMXt.  In case 1, restore IP into SP, otherwise just
+	     restore IP.  */
+	  if (frame_pointer_needed)
+	    {
+	      live_regs_mask &= ~ (1 << IP_REGNUM);
+	      live_regs_mask |=   (1 << SP_REGNUM);
+	    }
+	  else
+	    {
+	      if (! IS_INTERRUPT (func_type)
+		  && ! TARGET_REALLY_IWMMXT)
+		abort ();
+	    }
+	}
 
       /* On some ARM architectures it is faster to use LDR rather than
 	 LDM to load a single register.  On other architectures, the
Index: testsuite/gcc.dg/arm-mmx-1.c
===================================================================
RCS file: testsuite/gcc.dg/arm-mmx-1.c
diff -N testsuite/gcc.dg/arm-mmx-1.c
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gcc.dg/arm-mmx-1.c	27 Jan 2004 05:43:50 -0000
@@ -0,0 +1,26 @@
+/* Verify that if IP is saved to ensure stack alignment, we don't load
+   it into sp.  */
+/* { dg-do compile { target arm*-*-* strongarm*-*-* xscale*-*-*} } */
+/* { dg-options "-O -mno-apcs-frame -mcpu=iwmmxt" } */
+/* { dg-final { scan-assembler "ldmfd\[ 	]sp!.*ip,\[ ]*pc" } } */
+
+/* This function uses all the call-saved registers, namely r4, r5, r6,
+   r7, r8, r9, sl, fp.  Since we also save pc, that leaves an odd
+   number of registers, and the compiler will push ip to align the
+   stack.  Make sure that we restore ip into ip, not into sp as is
+   done when using a frame pointer.  The -mno-apcs-frame option
+   permits the frame pointer to be used as an ordinary register.  */
+int
+foo(int *a, int *b, int *c, int *d, int *tot)
+{
+  int i, j, k, l, m, n, o;
+
+  *tot = 0;
+  for (i = *a; i < *b; i += *c)
+    for (j = *a; j < *b; j += *d)
+      for (k = *a; k < *c; k += *d)
+	for (l = *b; k < *c; k += *d)
+	  for (m = *d; k < *c; k += *b)
+	    *tot += i + j + k + l + m;
+  return *tot;
+}