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]

Two problems with current_function_pretend_args_size


Noticed while digging in the mcore port...

1) If an argument is passed partially in registers, there will be
   pretend_args_size bytes between virtual_incoming_args and the first
   real stack argument.  The problem is that pretend_args_size is only
   rounded to PARM_BOUNDARY bits, not STACK_BOUNDARY bits.  This can
   throw off the location of later stack arguments.

   See the first test case for a MIPS n32 example.  The arguments are
   passed as follows:

       Word   Where
        0            +---------------+
                 $4  |      pa       |
        1            +---------------+  <-- virtual_incoming_args
                 $5  |  pb (word 1)  |
        ...........  | ............. |
                     |               |
        7            +---------------+
                $11  |  pb (word 7)  |
        8            +---------------+
              stack  |  pb (word 8)  |
        9            +---------------+  <-- virtual_incoming_args + 64
                     |    padding    |
       10            +---------------+
                     |  pc (word 1)  |
       11            +---------------+
                     |  pc (word 2)  |
                     +---------------+

   Here, the first stack byte is at an offset of 7 * UNITS_PER_WORD = 56 bytes
   from virtual_incoming_args.  PB ends at an offset of 8 * UNITS_PER_WORD
   = 64 bytes.  assign_parm then thinks that no padding is needed for PC.


2) assign_parms passes current_function_pretend_args_size directly to
   SETUP_INCOMING_VARARGS.  However, some ports (including mips) store 0
   in the pretend_args argument when no registers need saving.  This will
   clobber any previous value for partial-register arguments.

   Although this might be seen as a bug in the backends, the tm.texi
   description doesn't really seem to forbid it.

Patch bootstrapped & regression tested on mips64{,-el}-linux-gnu and
mips-sgi-irix6.5{,o32}.  Also tested on mipsisa64-elf and mcore-elf.
OK to install?

Richard

 [ BTW, as far as (1) goes, I wondered whether we should always point
   virtual_incoming_args to the first real stack argument.  We could
   then access the pretend arguments at a negative[*] offset from that.

   It would be a very invasive change though, and some ports would
   probably reject negative offsets from the argument pointer anyway...

     [*] positive if ARGS_GROW_DOWNWARD. ]


	* function.c (STACK_BYTES): Move definition to head of file.
	(assign_parms): Don't pass current_function_pretend_args_size
	directly to SETUP_INCOMING_VARARGS.  For partial register arguments,
	round current_function_pretend_args_size up to STACK_BYTES.  Skip any
	excess before laying out the argument.

testsuite/
	* gcc.c-torture/execute/20030830-[12].c: New tests.

Index: function.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/function.c,v
retrieving revision 1.450
diff -c -d -p -F^\([(a-zA-Z0-9_]\|#define\) -r1.450 function.c
*** function.c	19 Aug 2003 20:29:00 -0000	1.450
--- function.c	2 Sep 2003 14:02:00 -0000
*************** #define LOCAL_ALIGNMENT(TYPE, ALIGNMENT)
*** 75,80 ****
--- 75,82 ----
  #define STACK_ALIGNMENT_NEEDED 1
  #endif
  
+ #define STACK_BYTES (STACK_BOUNDARY / BITS_PER_UNIT)
+ 
  /* Some systems use __main in a way incompatible with its use in gcc, in these
     cases use the macros NAME__MAIN to give a quoted symbol and SYMBOL__MAIN to
     give the same symbol without quotes for an alternative entry point.  You
*************** assign_parms (tree fndecl)
*** 4319,4324 ****
--- 4321,4327 ----
        int last_named = 0, named_arg;
        int in_regs;
        int partial = 0;
+       int pretend_bytes = 0;
  
        /* Set LAST_NAMED if this is last named arg before last
  	 anonymous args.  */
*************** assign_parms (tree fndecl)
*** 4433,4441 ****
  	 Also, indicate when RTL generation is to be suppressed.  */
        if (last_named && !varargs_setup)
  	{
  	  SETUP_INCOMING_VARARGS (args_so_far, promoted_mode, passed_type,
! 				  current_function_pretend_args_size, 0);
  	  varargs_setup = 1;
  	}
  #endif
  
--- 4436,4451 ----
  	 Also, indicate when RTL generation is to be suppressed.  */
        if (last_named && !varargs_setup)
  	{
+ 	  int varargs_pretend_bytes = 0;
  	  SETUP_INCOMING_VARARGS (args_so_far, promoted_mode, passed_type,
! 				  varargs_pretend_bytes, 0);
  	  varargs_setup = 1;
+ 
+ 	  /* If the back-end has requested extra stack space, record how
+ 	     much is needed.  Do not change pretend_args_size otherwise
+ 	     since it may be nonzero from an earlier partial argument.  */
+ 	  if (varargs_pretend_bytes > 0)
+ 	    current_function_pretend_args_size = varargs_pretend_bytes;
  	}
  #endif
  
*************** assign_parms (tree fndecl)
*** 4479,4486 ****
  
  #ifdef FUNCTION_ARG_PARTIAL_NREGS
        if (entry_parm)
! 	partial = FUNCTION_ARG_PARTIAL_NREGS (args_so_far, promoted_mode,
! 					      passed_type, named_arg);
  #endif
  
        memset (&locate, 0, sizeof (locate));
--- 4489,4524 ----
  
  #ifdef FUNCTION_ARG_PARTIAL_NREGS
        if (entry_parm)
! 	{
! 	  partial = FUNCTION_ARG_PARTIAL_NREGS (args_so_far, promoted_mode,
! 						passed_type, named_arg);
! 	  if (partial
! #ifndef MAYBE_REG_PARM_STACK_SPACE
! 	      /* The caller might already have allocated stack space
! 		 for the register parameters.  */
! 	      && reg_parm_stack_space == 0
! #endif
! 	      )
! 	    {
! 	      /* Part of this argument is passed in registers and part
! 		 is passed on the stack.  Ask the prologue code to extend
! 		 the stack part so that we can recreate the full value.
! 
! 		 PRETEND_BYTES is the size of the registers we need to store.
! 
! 		 CURRENT_FUNCTION_PRETEND_ARGS_SIZE is the amount of extra
! 		 stack space that the prologue should allocate.  It must be
! 		 a multiple of STACK_BYTES so that the first real stack slot
! 		 is correctly aligned wrt the argument pointer.  */
! 	      pretend_bytes = partial * UNITS_PER_WORD;
! 	      current_function_pretend_args_size
! 		= CEIL_ROUND (pretend_bytes, STACK_BYTES);
! 
! 	      /* Skip over any bytes needed to pad to STACK_BYTES.  */
! 	      stack_args_size.constant
! 		+= (current_function_pretend_args_size - pretend_bytes);
! 	    }
! 	}
  #endif
  
        memset (&locate, 0, sizeof (locate));
*************** assign_parms (tree fndecl)
*** 4525,4541 ****
  
        if (partial)
  	{
- #ifndef MAYBE_REG_PARM_STACK_SPACE
- 	  /* When REG_PARM_STACK_SPACE is nonzero, stack space for
- 	     split parameters was allocated by our caller, so we
- 	     won't be pushing it in the prolog.  */
- 	  if (reg_parm_stack_space == 0)
- #endif
- 	  current_function_pretend_args_size
- 	    = (((partial * UNITS_PER_WORD) + (PARM_BOUNDARY / BITS_PER_UNIT) - 1)
- 	       / (PARM_BOUNDARY / BITS_PER_UNIT)
- 	       * (PARM_BOUNDARY / BITS_PER_UNIT));
- 
  	  /* Handle calls that pass values in multiple non-contiguous
  	     locations.  The Irix 6 ABI has examples of this.  */
  	  if (GET_CODE (entry_parm) == PARALLEL)
--- 4563,4568 ----
*************** assign_parms (tree fndecl)
*** 4579,4588 ****
  #endif
  	  )
  	{
! 	  stack_args_size.constant += locate.size.constant;
! 	  /* locate.size doesn't include the part in regs.  */
! 	  if (partial)
! 	    stack_args_size.constant += current_function_pretend_args_size;
  	  if (locate.size.var)
  	    ADD_PARM_SIZE (stack_args_size, locate.size.var);
  	}
--- 4606,4612 ----
  #endif
  	  )
  	{
! 	  stack_args_size.constant += pretend_bytes + locate.size.constant;
  	  if (locate.size.var)
  	    ADD_PARM_SIZE (stack_args_size, locate.size.var);
  	}
*************** assign_parms (tree fndecl)
*** 5154,5161 ****
  				    REG_PARM_STACK_SPACE (fndecl));
  #endif
  #endif
- 
- #define STACK_BYTES (STACK_BOUNDARY / BITS_PER_UNIT)
  
    current_function_args_size
      = ((current_function_args_size + STACK_BYTES - 1)
--- 5178,5183 ----
*** /dev/null	Tue Jun 17 23:06:41 2003
--- testsuite/gcc.c-torture/execute/20030830-1.c	Tue Sep  2 13:21:16 2003
***************
*** 0 ****
--- 1,26 ----
+ /* On IRIX 6, PB is passed partially in registers and partially on the
+    stack, with an odd number of words in the register part.  Check that
+    the long double stack argument (PC) is still accessed properly.  */
+ 
+ struct s { int val[16]; };
+ 
+ long double f (int pa, struct s pb, long double pc)
+ {
+   int i;
+ 
+   for (i = 0; i < 16; i++)
+     pc += pb.val[i];
+   return pc;
+ }
+ 
+ int main ()
+ {
+   struct s x;
+   int i;
+ 
+   for (i = 0; i < 16; i++)
+     x.val[i] = i + 1;
+   if (f (1, x, 10000.0L) != 10136.0L)
+     abort ();
+   exit (0);
+ }
*** /dev/null	Tue Jun 17 23:06:41 2003
--- testsuite/gcc.c-torture/execute/20030830-2.c	Tue Sep  2 15:14:20 2003
***************
*** 0 ****
--- 1,21 ----
+ /* On IRIX 6, PA is passed partially in registers and partially on the
+    stack.  We therefore have two potential uses of pretend_args_size:
+    one for the partial argument and one for the varargs save area.
+    Make sure that these uses don't conflict.  */
+ 
+ struct s { int i[18]; };
+ 
+ int f (struct s pa, int pb, ...)
+ {
+   return pb;
+ }
+ 
+ struct s gs;
+ 
+ int main ()
+ {
+   if (f (gs, 0x1234) != 0x1234)
+     abort ();
+ 
+   exit (0);
+ }


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