Applying FUNCTION_ARG_PADDING to register arguments

Jim Wilson wilson@cygnus.com
Thu Oct 23 17:28:00 GMT 1997


	While tracking down a varargs bug, I found that calls.c doesn't respect
	FUNCTION_ARG_PADDING for register arguments.  I think that is a bug.

I agree.  However, the problem is not quite this simple to fix, because adding
code that uses FUNCTION_ARG_PADDING may break the compiler for other targets
if FUNCTION_ARG_PADDING happens to be defined wrong for this case.  This
should at least be checked.  This particular code is used for the PA and
Irix6 ports.  I suggest you test them with your patch.

The irix6 port has a related problem.  I wrote up a description of
this once so I could discuss the problem with Kenner, but never had time
to finish the work I started.   My message follows.  I don't claim that
everything in it is correct, but there might be some useful info in it.

--------------------

I am trying to fix some problems with the irix6 calling convention support.
Gcc is emitting code that is incompatible with the ABI and the SGI C compiler.
The ABI specifies that structure arguments and return values are always
left aligned.  This is not how gcc traditionally handles structures in
registers, and fixing it gets to be very tricky.

There is a similar problem with the standard mips ABI.  However, there
are fewer cases that need to be dealt with.  It handles the left alignment of
struct arguments smaller than a word by using a function_arg trick to cause
logical left shift instructions to be emitted when calling a function, and
logical right shift instructions to be emitted in function prologues.  This
trick can not be easily extended to handle all irix6 problems.

I have a tentative patch, but it needs cleaning up.  Due to the significant
amount of time it will take to clean it up and test it, I am looking for
general guidance as to what you are willing to accept.

The right way to fix this problem seems to be to use FUNCTION_ARG_PADDING.
Setting the padding to upward for a big-endian machine forces left alignment.
However, there are three problems here.

The first problem is that MUST_PASS_IN_STACK macro in expr.h forces BLKmode
arguments to be passed in the stack if the padding is opposite the natural
direction.  This is wrong for irix6.  In general, MUST_PASS_IN_STACK should
not be overriding FUNCTION_ARG if FUNCTION_ARG says that something is only
passed in registers.  This seems to be a relic left over from GCC 1.x.  I
suspect that other ports may also be generating incorrect (non ABI compliant)
code because of this.  For instance, I noticed that the standard MIPS ABI
support suffers from this bug also.  However, at this point, it seems unsafe to
just delete this macro, because that will introduce unexpected and unnecessary
incompatibilites for targets where gcc is the only compiler.  Providing a
way for tm.h files to override MUST_PASS_IN_STACK seems the safest way for
now.  For now I have just modified expr.h but I don't think this is safe.

The second problem is that there are many places which do not use
FUNCTION_ARG_PADDING when loading arguments.  It is not used when loading
non-BLKmode arguments into registers.  It is not used when loading structure
arguments with insufficient alignment into registers.  The first case exists
because gcc will change structure modes from BLKmode to an integer mode
if it is sufficiently aligned.  This problem is worked around in the standard
mips ABI via the function_arg trick.  It may make sense to add code to
emit_calls which checks the padding, and then emits a left-shift if necessary.
However, I am concerned that an attempt at an elegant solution may break
other ports that don't define FUNCTION_ARG_PADDING correctly.
For now, I am handling this via the function_arg hack.
The second case concerns code that is only used by the PA and MIPS ports,
so it should be safe to change it to use FUNCTION_ARG_PADDING.  I have
an example of this in my patch.  

The third problem is that FUNCTION_ARG_PADDING is not used when returning
values.  It seems a bit odd to try to use it here, but it will do the
right thing for the Irix 6 port.  Perhaps we should create a new macro?
As above, there are two cases to be dealt with, the `unaligned' structure
return case, and the regular case.  I have a change for the `unaligned' return
case in my patch.  The regular case I am handling via a function_epilogue
hack.  As above, I have concerns about breaking other ports
that don't define FUNCTION_ARG_PADDING correctly, but this is not a problem
if we create a new macro, which would then only be used by the Irix 6 port.

The following patch seems to work, but is not very elegant.  I have tested
it for irix6 and for the PA (for the `unaligned' structure arguments in
registers case) against a number of small testcases, but it needs more
extensive testing.

diff -pr clean-ss-961219/calls.c ss-961219/calls.c
*** clean-ss-961219/calls.c	Thu Dec 12 16:46:42 1996
--- ss-961219/calls.c	Fri Dec 27 15:06:25 1996
*************** expand_call (exp, target, ignore)
*** 1763,1773 ****
  	  args[i].aligned_regs = (rtx *) alloca (sizeof (rtx)
  						 * args[i].n_aligned_regs);
  
! 	  /* Structures smaller than a word are aligned to the least
! 	     significant byte (to the right).  On a BYTES_BIG_ENDIAN machine,
! 	     this means we must skip the empty high order bytes when
! 	     calculating the bit offset.  */
! 	  if (BYTES_BIG_ENDIAN && bytes < UNITS_PER_WORD)
  	    big_endian_correction = (BITS_PER_WORD  - (bytes * BITS_PER_UNIT));
  
  	  for (j = 0; j < args[i].n_aligned_regs; j++)
--- 1763,1776 ----
  	  args[i].aligned_regs = (rtx *) alloca (sizeof (rtx)
  						 * args[i].n_aligned_regs);
  
! 	  /* Structures whose size is not a multiple of a word are aligned to
! 	     the least significant byte (to the right).  On a BYTES_BIG_ENDIAN
! 	     machine, this means we must skip the empty high order bytes when
! 	     calculating the bit offset if arguments are padded downwards.  */
! 	  if (BYTES_BIG_ENDIAN && (bytes % UNITS_PER_WORD)
! 	      && (FUNCTION_ARG_PADDING (TYPE_MODE (TREE_TYPE (args[i].tree_value)),
! 					TREE_TYPE (args[i].tree_value))
! 		  == downward))
  	    big_endian_correction = (BITS_PER_WORD  - (bytes * BITS_PER_UNIT));
  
  	  for (j = 0; j < args[i].n_aligned_regs; j++)
diff -pr clean-ss-961219/config/mips/abi64.h ss-961219/config/mips/abi64.h
*** clean-ss-961219/config/mips/abi64.h	Tue Oct 22 15:23:24 1996
--- ss-961219/config/mips/abi64.h	Fri Dec 27 12:54:15 1996
*************** Boston, MA 02111-1307, USA.  */
*** 77,88 ****
  #define FUNCTION_ARG_PADDING(MODE, TYPE)				\
    (! BYTES_BIG_ENDIAN							\
     ? upward								\
!    : (((MODE) == BLKmode						\
         ? ((TYPE) && TREE_CODE (TYPE_SIZE (TYPE)) == INTEGER_CST		\
! 	  && int_size_in_bytes (TYPE) < (PARM_BOUNDARY / BITS_PER_UNIT))\
         : (GET_MODE_BITSIZE (MODE) < PARM_BOUNDARY			\
! 	  && (mips_abi == ABI_32 || mips_abi == ABI_EABI		\
! 	      || GET_MODE_CLASS (MODE) == MODE_INT)))			\
        ? downward : upward))
  
  #undef RETURN_IN_MEMORY
--- 77,103 ----
  #define FUNCTION_ARG_PADDING(MODE, TYPE)				\
    (! BYTES_BIG_ENDIAN							\
     ? upward								\
!    : (mips_abi == ABI_32 || mips_abi == ABI_EABI)			\
!    /* This is the default definition.  */				\
!    ? (((MODE) == BLKmode						\
         ? ((TYPE) && TREE_CODE (TYPE_SIZE (TYPE)) == INTEGER_CST		\
! 	  && int_size_in_bytes (TYPE) < (PARM_BOUNDARY / BITS_PER_UNIT)) \
!        : GET_MODE_BITSIZE (MODE) < PARM_BOUNDARY)			\
!       ? downward : upward)						\
!    /* This is the ABI_N32/ABI_64 definition.  Structures are always	\
!       left aligned.  This only matters for structures which are not	\
!       a multiple of 64 bits.  Integers smaller than 64 bits are right	\
!       aligned.  Floats smaller than 64 bits are left aligned.  Otherwise \
!       align the same way as the default.  See the comment in function_arg \
!       about left-adjusting of parameters for non-BLKmode structures.  */ \
!    : ((((MODE) == BLKmode						\
! 	|| ((TYPE) && (TREE_CODE (TYPE) == RECORD_TYPE			\
! 		       || TREE_CODE (TYPE) == UNION_TYPE)))		\
!        ? ((TYPE) && TREE_CODE (TYPE_SIZE (TYPE)) == INTEGER_CST		\
! 	  && (int_size_in_bytes (TYPE)					\
! 	      % (PARM_BOUNDARY / BITS_PER_UNIT) == 0))			\
         : (GET_MODE_BITSIZE (MODE) < PARM_BOUNDARY			\
! 	  && GET_MODE_CLASS (MODE) != MODE_FLOAT))			\
        ? downward : upward))
  
  #undef RETURN_IN_MEMORY
diff -pr clean-ss-961219/config/mips/mips.c ss-961219/config/mips/mips.c
*** clean-ss-961219/config/mips/mips.c	Thu Dec 12 17:27:46 1996
--- ss-961219/config/mips/mips.c	Fri Dec 27 15:54:58 1996
*************** function_arg (cum, mode, type, named)
*** 3267,3273 ****
  	 calling convention for now.  */
  
        if (struct_p && int_size_in_bytes (type) < UNITS_PER_WORD
! 	  && ! TARGET_64BIT && mips_abi != ABI_EABI)
  	{
  	  rtx amount = GEN_INT (BITS_PER_WORD
  				- int_size_in_bytes (type) * BITS_PER_UNIT);
--- 3267,3274 ----
  	 calling convention for now.  */
  
        if (struct_p && int_size_in_bytes (type) < UNITS_PER_WORD
! 	  && (mips_abi != ABI_32 || ! TARGET_64BIT) && mips_abi != ABI_EABI
! 	  && (mips_abi == ABI_32 || mode != BLKmode))
  	{
  	  rtx amount = GEN_INT (BITS_PER_WORD
  				- int_size_in_bytes (type) * BITS_PER_UNIT);
*************** function_epilogue (file, size)
*** 5547,5552 ****
--- 5548,5563 ----
  
    if (!noepilogue)
      {
+       /* Must left shift the return value if it is a structure with an integral
+ 	 mode, and we are emitting code for the N32 or N64 ABIs.  */
+       tree retval_type = TREE_TYPE (TREE_TYPE (current_function_decl));
+       if (TYPE_MODE (retval_type) != BLKmode
+ 	  && AGGREGATE_TYPE_P (retval_type)
+ 	  && int_size_in_bytes (retval_type) < UNITS_PER_WORD
+ 	  && mips_abi != ABI_32 && mips_abi != ABI_EABI)
+ 	fprintf (file, "\tdsll\t$2,$2,%d\n",
+ 		 BITS_PER_WORD - int_size_in_bytes (retval_type) * BITS_PER_UNIT);
+ 
        /* In the reload sequence, we don't need to fill the load delay
  	 slots for most of the loads, also see if we can fill the final
  	 delay slot if not otherwise filled by the reload sequence.  */
*************** mips_expand_epilogue ()
*** 5732,5737 ****
--- 5743,5764 ----
    long tsize = current_frame_info.total_size;
    rtx tsize_rtx = GEN_INT (tsize);
    rtx tmp_rtx = (rtx)0;
+   tree retval_type;
+ 
+   /* Must left shift the return value if it is a structure with an integral
+      mode, and we are emitting code for the N32 or N64 ABIs.  */
+   retval_type = TREE_TYPE (TREE_TYPE (current_function_decl));
+   if (TYPE_MODE (retval_type) != BLKmode
+       && AGGREGATE_TYPE_P (retval_type)
+       && int_size_in_bytes (retval_type) < UNITS_PER_WORD
+       && mips_abi != ABI_32 && mips_abi != ABI_EABI)
+     {
+       /* TARGET_64BIT is assumed to be true here.  */
+       rtx retval = gen_rtx (REG, DImode, 2);
+       rtx amount = GEN_INT (BITS_PER_WORD
+ 			    - int_size_in_bytes (retval_type) * BITS_PER_UNIT);
+       emit_insn (gen_ashldi3 (retval, retval, amount));
+     }
  
    if (tsize > 32767)
      {
diff -pr clean-ss-961219/expr.h ss-961219/expr.h
*** clean-ss-961219/expr.h	Tue Oct 22 21:15:54 1996
--- ss-961219/expr.h	Fri Dec 27 13:09:55 1996
*************** enum direction {none, upward, downward};
*** 207,219 ****
  #define MUST_PASS_IN_STACK(MODE,TYPE)			\
    ((TYPE) != 0						\
     && (TREE_CODE (TYPE_SIZE (TYPE)) != INTEGER_CST	\
!        || TREE_ADDRESSABLE (TYPE)			\
!        || ((MODE) == BLKmode 				\
! 	   && ! ((TYPE) != 0 && TREE_CODE (TYPE_SIZE (TYPE)) == INTEGER_CST \
! 		 && 0 == (int_size_in_bytes (TYPE)	\
! 			  % (PARM_BOUNDARY / BITS_PER_UNIT))) \
! 	   && (FUNCTION_ARG_PADDING (MODE, TYPE)	\
! 	       == (BYTES_BIG_ENDIAN ? upward : downward)))))
  
  /* Nonzero if type TYPE should be returned in memory.
     Most machines can use the following default definition.  */
--- 207,213 ----
  #define MUST_PASS_IN_STACK(MODE,TYPE)			\
    ((TYPE) != 0						\
     && (TREE_CODE (TYPE_SIZE (TYPE)) != INTEGER_CST	\
!        || TREE_ADDRESSABLE (TYPE)))
  
  /* Nonzero if type TYPE should be returned in memory.
     Most machines can use the following default definition.  */
diff -pr clean-ss-961219/ginclude/va-mips.h ss-961219/ginclude/va-mips.h
*** clean-ss-961219/ginclude/va-mips.h	Tue Oct 22 15:26:29 1996
--- ss-961219/ginclude/va-mips.h	Fri Dec 27 14:39:20 1996
*************** void va_end (__gnuc_va_list);		/* Define
*** 234,248 ****
     right aligned).  */
  #ifdef __mips64
  #ifdef __MIPSEB__
  #define va_arg(__AP, __type)                                    \
    ((__type *) (void *) (__AP = (char *) ((((__PTRDIFF_TYPE__)__AP + 8 - 1) & -8) \
  					 + __va_rounded_size (__type))))[-1]
! #else
  #define va_arg(__AP, __type)                                    \
    ((__AP = (char *) ((((__PTRDIFF_TYPE__)__AP + 8 - 1) & -8)	\
  		     + __va_rounded_size (__type))),		\
     *(__type *) (void *) (__AP - __va_rounded_size (__type)))
! #endif
  
  #else /* not __mips64 */
  
--- 234,271 ----
     right aligned).  */
  #ifdef __mips64
  #ifdef __MIPSEB__
+ 
+ #if defined(_MIPS_SIM) && (_MIPS_SIM == _MIPS_SIM_ABI64 || _MIPS_SIM == _MIPS_SIM_NABI32)
+ /* For ABI_N32 and ABI_64, structures are left aligned, integers smaller
+    than a word are right aligned, and floats smaller than a word are left
+    aligned.  We can ignore the float case for now, since floats must always
+    be promoted to double.  */
+ #define va_arg(__AP, __type)                                    \
+   ((__builtin_classify_type (* (__type *) 0) == __record_type_class \
+     || __builtin_classify_type (* (__type *) 0) == __union_type_class) \
+    ? ((__AP = (char *) ((((__PTRDIFF_TYPE__)__AP + 8 - 1) & -8)	\
+ 		     + __va_rounded_size (__type))),		\
+       *(__type *) (void *) (__AP - __va_rounded_size (__type)))	\
+    : ((__type *) (void *)					\
+       (__AP = (char *) ((((__PTRDIFF_TYPE__)__AP + 8 - 1) & -8) \
+ 			+ __va_rounded_size (__type))))[-1])
+ #else /* ! N32/64 ABI */
  #define va_arg(__AP, __type)                                    \
    ((__type *) (void *) (__AP = (char *) ((((__PTRDIFF_TYPE__)__AP + 8 - 1) & -8) \
  					 + __va_rounded_size (__type))))[-1]
! #endif /* N32/64 ABI */
! 
! #else /* ! __MIPSEB__ */
! 
! #if defined(_MIPS_SIM) && (_MIPS_SIM == _MIPS_SIM_ABI64 || _MIPS_SIM == _MIPS_SIM_NABI32)
! #error va_arg is not defined for little-endian ABI_N32/ABI_64
! #endif
! 
  #define va_arg(__AP, __type)                                    \
    ((__AP = (char *) ((((__PTRDIFF_TYPE__)__AP + 8 - 1) & -8)	\
  		     + __va_rounded_size (__type))),		\
     *(__type *) (void *) (__AP - __va_rounded_size (__type)))
! #endif  /* ! __MIPSEB__ */
  
  #else /* not __mips64 */
  
diff -pr clean-ss-961219/stmt.c ss-961219/stmt.c
*** clean-ss-961219/stmt.c	Wed Oct 30 12:50:39 1996
--- ss-961219/stmt.c	Fri Dec 27 15:29:04 1996
*************** expand_return (retval)
*** 2847,2857 ****
        rtx result_val = expand_expr (retval_rhs, NULL_RTX, VOIDmode, 0);
        enum machine_mode tmpmode, result_reg_mode;
  
!       /* Structures whose size is not a multiple of a word are aligned
! 	 to the least significant byte (to the right).  On a BYTES_BIG_ENDIAN
  	 machine, this means we must skip the empty high order bytes when
! 	 calculating the bit offset.  */
!       if (BYTES_BIG_ENDIAN && bytes % UNITS_PER_WORD)
  	big_endian_correction = (BITS_PER_WORD - ((bytes % UNITS_PER_WORD)
  						  * BITS_PER_UNIT));
  
--- 2847,2860 ----
        rtx result_val = expand_expr (retval_rhs, NULL_RTX, VOIDmode, 0);
        enum machine_mode tmpmode, result_reg_mode;
  
!       /* Structures whose size is not a multiple of a word are aligned to
! 	 the least significant byte (to the right).  On a BYTES_BIG_ENDIAN
  	 machine, this means we must skip the empty high order bytes when
! 	 calculating the bit offset if arguments are padded downwards.  */
!       if (BYTES_BIG_ENDIAN && (bytes % UNITS_PER_WORD)
! 	  && (FUNCTION_ARG_PADDING (TYPE_MODE (TREE_TYPE (retval_rhs)),
! 				    TREE_TYPE (retval_rhs))
! 	      == downward))
  	big_endian_correction = (BITS_PER_WORD - ((bytes % UNITS_PER_WORD)
  						  * BITS_PER_UNIT));
  



More information about the Gcc mailing list