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]

[committed] MIPS argument alignment fixes


This MIPS patch fixes the struct-layout-1/t025 failures for n32 and n64
and a whole load of struct-layout-1 failures for o32.

The problem is that, for any type T, FUNCTION_ARG_BOUNDARY returns
MAX (PARM_BOUNDARY, TYPE_ALIGN (T)).  That's OK for naturally-aligned
standard-defined types, since every such type has an alignment of
STACK_BOUNDARY bits or less.  But it's nonsense for types that have
bigger alignments, such as large vector types or types with an
explicit alignment attribute.

The patch below fixes this by limiting the alignment to STACK_BOUNDARY.
This is of course an ABI change, and one that I'll document in changes.html
before the release, but I suspect very little real code will be affected.
Besides, we already have other ABI changes that more invasive than this.
(And yes, I'm going to document those changes too ;)

Since we're now in regression-only mode: although the bug itself is very
old, and probably isn't a regression in that sense, affected code will
work or fail depending on whether a given stack frame happens to be
aligned to more than STACK_BOUNDARY bits.  That in turn depends on the
size of the stack frame, which is heavily dependent on the compiler
version and optimisation flags.

There are one of two other warts in related code that I couldn't
resist cleaning up at the same time.  I suspect they date from the
days when we had MEABI:

  #define PARM_BOUNDARY ((mips_abi == ABI_O64 \
                          || TARGET_NEWABI \
                          || (mips_abi == ABI_EABI && TARGET_64BIT)) ? 64 : 32)

This is equivalent to BITS_PER_WORD.

  #define STACK_BOUNDARY ((TARGET_OLDABI || mips_abi == ABI_EABI) ? 64 : 128)

Would be clearer as (TARGET_NEWABI ? 128 : 64), since TARGET_NEWABI
is the odd-one-out here.

  #define MIPS_STACK_ALIGN(LOC)						\
    ((TARGET_OLDABI || mips_abi == ABI_EABI)				\
     ? ((LOC) + 7) & ~7							\
     : ((LOC) + 15) & ~15)

As for STACK_BOUNDARY.  I was also a bit uneasy about the int-sized ~foo
constants (what if LOC is a HOST_WIDE_INT?) so I changed it to use -8 and
-16 instead.

Bootstrapped & regression tested on mips-sgi-irix6.5.  I also ran the
compat testsuite against the pre-patch compiler to check for unintended
ABI changes.  There were no failures.  Applied to mainline.

Richard


	* config/mips/mips-protos.h (function_arg_boundary): Declare.
	* config/mips/mips.h (PARM_BOUNDARY): Simplify definition.
	(STACK_BOUNDARY, MIPS_STACK_ALIGN): Likewise.
	(FUNCTION_ARG_BOUNDARY): Use new function_arg_boundary function.
	* config/mips/mips.c (function_arg_boundary): New function.
	Impose a maximum alignment of STACK_BOUNDARY.

Index: config/mips/mips-protos.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/mips/mips-protos.h,v
retrieving revision 1.79
diff -u -p -F^\([(a-zA-Z0-9_]\|#define\) -r1.79 mips-protos.h
--- config/mips/mips-protos.h	21 Oct 2004 00:53:37 -0000	1.79
+++ config/mips/mips-protos.h	26 Nov 2004 12:16:51 -0000
@@ -143,6 +143,7 @@ extern struct rtx_def *function_arg (con
 				     enum machine_mode, tree, int);
 extern int function_arg_partial_nregs (const CUMULATIVE_ARGS *,
 				       enum machine_mode, tree, int);
+extern int function_arg_boundary (enum machine_mode, tree);
 extern bool mips_pad_arg_upward (enum machine_mode, tree);
 extern bool mips_pad_reg_upward (enum machine_mode, tree);
 extern void mips_va_start (tree, rtx);
Index: config/mips/mips.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/mips/mips.h,v
retrieving revision 1.376
diff -u -p -F^\([(a-zA-Z0-9_]\|#define\) -r1.376 mips.h
--- config/mips/mips.h	16 Nov 2004 19:24:16 -0000	1.376
+++ config/mips/mips.h	26 Nov 2004 12:17:05 -0000
@@ -1300,10 +1300,7 @@ #define POINTER_SIZE ((TARGET_LONG64 && 
 #endif
 
 /* Allocation boundary (in *bits*) for storing arguments in argument list.  */
-#define PARM_BOUNDARY ((mips_abi == ABI_O64 \
-			|| TARGET_NEWABI \
-			|| (mips_abi == ABI_EABI && TARGET_64BIT)) ? 64 : 32)
-
+#define PARM_BOUNDARY BITS_PER_WORD
 
 /* Allocation boundary (in *bits*) for the code of a function.  */
 #define FUNCTION_BOUNDARY 32
@@ -2145,7 +2142,7 @@ #define REG_PARM_STACK_SPACE(FNDECL) 			
    `current_function_outgoing_args_size'.  */
 #define OUTGOING_REG_PARM_STACK_SPACE
 
-#define STACK_BOUNDARY ((TARGET_OLDABI || mips_abi == ABI_EABI) ? 64 : 128)
+#define STACK_BOUNDARY (TARGET_NEWABI ? 128 : 64)
 
 #define RETURN_POPS_ARGS(FUNDECL,FUNTYPE,SIZE) 0
 
@@ -2285,18 +2282,7 @@ #define FUNCTION_ARG(CUM, MODE, TYPE, NA
 #define FUNCTION_ARG_PARTIAL_NREGS(CUM, MODE, TYPE, NAMED) \
   function_arg_partial_nregs (&CUM, MODE, TYPE, NAMED)
 
-/* If defined, a C expression that gives the alignment boundary, in
-   bits, of an argument with the specified mode and type.  If it is
-   not defined,  `PARM_BOUNDARY' is used for all arguments.  */
-
-#define FUNCTION_ARG_BOUNDARY(MODE, TYPE)				\
-  (((TYPE) != 0)							\
-	? ((TYPE_ALIGN(TYPE) <= PARM_BOUNDARY)				\
-		? PARM_BOUNDARY						\
-		: TYPE_ALIGN(TYPE))					\
-	: ((GET_MODE_ALIGNMENT(MODE) <= PARM_BOUNDARY)			\
-		? PARM_BOUNDARY						\
-		: GET_MODE_ALIGNMENT(MODE)))
+#define FUNCTION_ARG_BOUNDARY function_arg_boundary
 
 #define FUNCTION_ARG_PADDING(MODE, TYPE)		\
   (mips_pad_arg_upward (MODE, TYPE) ? upward : downward)
@@ -2318,10 +2304,8 @@ #define EPILOGUE_USES(REGNO) ((REGNO) ==
 
 /* Treat LOC as a byte offset from the stack pointer and round it up
    to the next fully-aligned offset.  */
-#define MIPS_STACK_ALIGN(LOC)						\
-  ((TARGET_OLDABI || mips_abi == ABI_EABI)				\
-   ? ((LOC) + 7) & ~7							\
-   : ((LOC) + 15) & ~15)
+#define MIPS_STACK_ALIGN(LOC) \
+  (TARGET_NEWABI ? ((LOC) + 15) & -16 : ((LOC) + 7) & -8)
 
 
 /* Implement `va_start' for varargs and stdarg.  */
Index: config/mips/mips.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/mips/mips.c,v
retrieving revision 1.477
diff -u -p -F^\([(a-zA-Z0-9_]\|#define\) -r1.477 mips.c
--- config/mips/mips.c	16 Nov 2004 19:24:12 -0000	1.477
+++ config/mips/mips.c	26 Nov 2004 12:17:02 -0000
@@ -3326,6 +3326,23 @@ function_arg_partial_nregs (const CUMULA
 }
 
 
+/* Implement FUNCTION_ARG_BOUNDARY.  Every parameter gets at least
+   PARM_BOUNDARY bits of alignment, but will be given anything up
+   to STACK_BOUNDARY bits if the type requires it.  */
+
+int
+function_arg_boundary (enum machine_mode mode, tree type)
+{
+  unsigned int alignment;
+
+  alignment = type ? TYPE_ALIGN (type) : GET_MODE_ALIGNMENT (mode);
+  if (alignment < PARM_BOUNDARY)
+    alignment = PARM_BOUNDARY;
+  if (alignment > STACK_BOUNDARY)
+    alignment = STACK_BOUNDARY;
+  return alignment;
+}
+
 /* Return true if FUNCTION_ARG_PADDING (MODE, TYPE) should return
    upward rather than downward.  In other words, return true if the
    first byte of the stack slot has useful data, false if the last


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