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, v2] Fix aggregate alignment ABI issue


Hello,

this patch updates the prior version:
https://gcc.gnu.org/ml/gcc-patches/2014-07/msg00634.html
to add a -Wpsabi note as discussed.

Note that the warning triggers whenever type is encountered whose alignment
*requirement* changes due to this patch.  In some cases, this does not actually
change the ABI of a given function call since the argument happened to be
already aligned for the higher requirement as well.

Except for a few cases in the ABI compat suite (as expected), this note
triggers in three test cases:

In the ELFv1 ABI only:
gcc.c-torture/compile/20040709-1.c:7:1: note: the ABI of passing aggregates with 16-byte alignment has changed in GCC 4.10
union foo {
  long double ld;
} bar;

Note that this union counts as homogeneous aggregate according to
the ELFv2 ABI, so it is treated the same before and after this patch
(i.e. aligned to 8 bytes).

In the ELFv1 ABI, however, this union does *not* count as a single-element
float struct (because its mode is TImode, not TFmode, since common code
does the latter only for single-element structs, not unions).  Therefore,
this union used to be 8-byte aligned but is now 16-byte aligned.

In both the ELFv1 and ELFv2 ABIs:
gcc.dg/pr32293.c:46:1: note: the ABI of passing aggregates with 16-byte alignment has changed in GCC 4.10
typedef
__attribute__ ((aligned(16)))
     struct {
       UINT64 w[2];
     } UINT128;

g++.dg/abi/param2.C:9:1: note: the ABI of passing aggregates with 16-byte alignment has changed in GCC 4.10
struct S { union {} a; } __attribute__((aligned));

In both cases, we have a struct of size 16 bytes (in the second case due to
alignment padding) and alignment requirement 16 bytes, which is recognized
as TImode and not BLKmode by common code, and thus used to be 8-byte aligned
but is now 16-byte aligned.

The tests still PASS since the dg-test infrastructure prunes all informational
messages from the GCC output.

Based on the discussion after the previous patch submission, the patch
leaves the Darwin ABI unchanged.

Tested on powerpc64-linux and powerpc64le-linux.

OK for mainline?

Bye,
Ulrich


gcc/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.  Issue -Wpsabi note when a type
	is now treated differently than before.

gcc/testsuite/ChangLog:

	* gcc.target/powerpc/ppc64-abi-warn-2.c: New test.


Index: gcc/config/rs6000/rs6000.c
===================================================================
--- gcc/config/rs6000/rs6000.c	(revision 212147)
+++ gcc/config/rs6000/rs6000.c	(working copy)
@@ -9182,14 +9182,48 @@
 	   || (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))
- 	   && mode == BLKmode
-	   && type && TYPE_ALIGN (type) > 64)
+
+  /* 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.  */
+  if (((DEFAULT_ABI == ABI_AIX && !rs6000_compat_align_parm)
+       || DEFAULT_ABI == ABI_ELFv2)
+      && type && TYPE_ALIGN (type) > 64)
+    {
+      /* "Aggregate" means any AGGREGATE_TYPE except for single-element
+         or homogeneous float/vector aggregates here.  We already handled
+         vector aggregates above, but still need to check for float here. */
+      bool aggregate_p = (AGGREGATE_TYPE_P (type)
+			  && !SCALAR_FLOAT_MODE_P (elt_mode));
+
+      /* We used to check for BLKmode instead of the above aggregate type
+	 check.  Warn when this results in any difference to the ABI.  */
+      if (aggregate_p != (mode == BLKmode))
+	{
+	  static bool warned;
+	  if (!warned && warn_psabi)
+	    {
+	      warned = true;
+	      inform (input_location,
+		      "the ABI of passing aggregates with %d-byte alignment"
+		      " has changed in GCC 4.10",
+		      (int) TYPE_ALIGN (type) / BITS_PER_UNIT);
+	    }
+	}
+
+      if (aggregate_p)
+	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.  */
+  if (TARGET_MACHO && rs6000_darwin64_abi
+      && mode == BLKmode
+      && type && TYPE_ALIGN (type) > 64)
     return 128;
-  else
-    return PARM_BOUNDARY;
+
+  return PARM_BOUNDARY;
 }
 
 /* The offset in words to the start of the parameter save area.  */
Index: gcc/testsuite/gcc.target/powerpc/ppc64-abi-warn-2.c
===================================================================
--- /dev/null
+++ gcc/testsuite/gcc.target/powerpc/ppc64-abi-warn-2.c	(revision 0)
@@ -0,0 +1,11 @@
+/* { dg-do compile { target { powerpc*-*-linux* && lp64 } } } */
+
+struct test
+  {
+    long a __attribute__((aligned (16)));
+  };
+
+void test (struct test a) /* { dg-message "note: the ABI of passing aggregates with 16-byte alignment has changed" } */
+{
+}
+
-- 
  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]