This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: PATCH RFA: ARM iWMMXt code can mishandle function return


On Tue, Jan 27, 2004 at 12:54:20AM -0500, Ian Lance Taylor wrote:
> 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.

I've been using the patch below.  Pretty much the same except for the
if conditions, and I think the difference is harmless.

> 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?

Running tests using the simulator required two local hacks for me:
  Change the #if 0 to #if 1 in gas to emit .note.gnu.arm.ident.
  Twiddle the simulator startup code to enable the iwmmxt coprocessors,
    since newlib's startup code doesn't.
All around, much easier to just do it on hardware... :)


Once again, I'm sorry for not submitting my iwmmxt patches; I ran out
of time.  I have a couple of others, mostly things that showed up in
the libstdc++ testsuite.  I will try to bring my simulator environment
back up to date and merge them to HEAD, or ping me if you want a copy
of the current mess of them.

> 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.  */

This seems like a terribly un-robust way to write a testcase.  How
about using an asm to clobber the other registers instead?  I imagine
that the code generated for this function is going to change
drastically with tree-ssa and lno-branch...

> +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;
> +}
> 

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

--- gcc-3.3/gcc/config/arm/arm.c.orig	2003-07-29 12:45:39.000000000 -0400
+++ gcc-3.3/gcc/config/arm/arm.c	2003-07-29 12:53:42.000000000 -0400
@@ -7625,15 +7625,20 @@ output_return_instruction (operand, real
 	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 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.  */
+	  /* Or, on iWMMXt, it might just be saved for alignment....  */
+	  if (! IS_INTERRUPT (func_type) && frame_pointer_needed)
+	    {
+	      live_regs_mask &= ~ (1 << IP_REGNUM);
+	      live_regs_mask |=   (1 << SP_REGNUM);
+	    }
+	  else if (! frame_pointer_needed && ! TARGET_REALLY_IWMMXT)
+	    abort ();
+	}
 
       /* If r3 is marked as a saved register, than it was saved just to
 	 preserve stack alignment.  We don't want to actually restore


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]