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]

[PATCH, rs6000] Fix aggregate alignment ABI issue


Hello,

last year, Bill added a patch to address PR 57949 by aligning aggregates
requiring at least 128-bit alignment at a quadword boundary in the
parameter save area:
https://gcc.gnu.org/ml/gcc-patches/2013-08/msg00803.html

Unfortunately, to implement this check, Bill's patch used a pre-existing
piece of code originally used only on Darwin, which uses a "mode == BLKmode"
check to test for aggregate types.

However, GCC may sometimes choose non-BLKmode modes to represent aggregate
types.  One case the single-element float/vector aggregates; that's OK,
because those are handled separately by the ABI anyway.

But there are more cases: many structures get some integer mode simply
because their size happen to be 1, 2, 4, 8, or 16 bytes.  The precise
rules *which* aggregates GCC uses such a mode for are intricate, and
even differ slightly between types created by the C vs. C++ front-ends.

This normally doesn't matter since the mode used to back an aggregate
type only matters for internal code generation (basically, whether GCC
may use a register to hold a local variable of that type, or whether
they must all go to memory).

Due to this check in rs6000_function_arg_boundary, however, those GCC
internal details have now leaked into the public ABI.  We have thought
of simply accepting that ABI as the de-facto ABI now and documenting
it, but that turned out to be too fragile; it is hard to precisely
describe the mode selection in a way that it can be reliably implemented
by another (non-GCC based) compiler.

After various off-line discussions, we came to the conclusion that the
best way is to fix the GCC implementation to actually align *all* aggregate
types, not just those backed by BLKmode.  [ The exception remain single-
element (or ELFv2 homogeneous) float/vector aggregates, which are handled
as before: float is doubleword aligned, vector is quadword aligned. ]

This change does break the ABI in certain cases.  However, we hope that
this is acceptable because:

- The change only affects rare cases: passing a struct by value that is
   * not a float/vector special case, and
   * has a size of 1, 2, 4, 8, or 16 bytes, and
   * has an alignment requirement of 16 bytes or more
  [ Not *all* these cases will see change, but all cases that change
  will share these properties.  ]

- This aspect of the ABI already changed recently with Bill's patch,
  and the current version hasn't seen very widespread use yet.

Note that patch below only changes the ABI for the AIX/ELFv1 and ELFv2
cases; the Darwin ABI (which shared the same problem) is left as-is.
It's up to the Darwin maintainers whether they prefer to change as well
or rather keep everything as it has been on Darwin for a long time.

Tested on powerpc64-linux and powerpc64le-linux.

OK for mainline?
[ The patch should then also go into the 4.8 and 4.9 branches for
consistency. ]

Bye,
Ulrich


ChangeLog:

	* config/rs6000/rs6000.c (rs6000_function_arg_boundary): In the AIX
	and ELFv2 ABI, do not use the "mode == BLKmode" check to test for
	aggregate types.  Instead, *all* aggregate types, except for single-
	element or homogeneous float/vector aggregates, are quadword-aligned
	if required by their type alignment.

Index: gcc/config/rs6000/rs6000.c
===================================================================
--- gcc/config/rs6000/rs6000.c	(revision 212147)
+++ gcc/config/rs6000/rs6000.c	(working copy)
@@ -9182,9 +9182,20 @@
 	   || (type && TREE_CODE (type) == VECTOR_TYPE
 	       && int_size_in_bytes (type) >= 16))
     return 128;
-  else if (((TARGET_MACHO && rs6000_darwin64_abi)
-	    || DEFAULT_ABI == ABI_ELFv2
-            || (DEFAULT_ABI == ABI_AIX && !rs6000_compat_align_parm))
+  /* Aggregate types that need > 8 byte alignment are quadword-aligned
+     in the parameter area in the ELFv2 ABI, and in the AIX ABI unless
+     -mcompat-align-parm is used.  This does not apply to single-element
+     (or homogeneous) float/vector aggregrates.  We already checked for
+     vector above; we still need to check for float here.  */
+  else if (((DEFAULT_ABI == ABI_AIX && !rs6000_compat_align_parm)
+            || DEFAULT_ABI == ABI_ELFv2)
+	   && type && AGGREGATE_TYPE_P (type) && TYPE_ALIGN (type) > 64
+ 	   && !SCALAR_FLOAT_MODE_P (elt_mode))
+    return 128;
+  /* Similar for the Darwin64 ABI.  Note that for historical reasons we
+     implement the "aggregate type" check as a BLKmode check here; this
+     means certain aggregate types are in fact not aligned.  */
+  else if (TARGET_MACHO && rs6000_darwin64_abi
  	   && mode == BLKmode
 	   && type && TYPE_ALIGN (type) > 64)
     return 128;
-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  Ulrich.Weigand@de.ibm.com


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