diff mbox

[rs6000,4.8/4.9] Fix aggregate alignment ABI issue

Message ID 201407170039.s6H0dwaA023988@d06av02.portsmouth.uk.ibm.com
State New
Headers show

Commit Message

Ulrich Weigand July 17, 2014, 12:39 a.m. UTC 
Hello,

this is the variant intended for the 4.8/4.9 branches of the patch:
https://gcc.gnu.org/ml/gcc-patches/2014-07/msg00995.html

As discussed, it does *not* actually change ABI, but only warn when
encountering a situation where the ABI will change in a future GCC.
(Avoiding the specific term "GCC 4.10" here since I'm not certain
whether the next GCC release will in fact be called that ...)

Tested on powerpc64-linux and powerpc64le-linux; also verified using
the ABI compat suite (against an unpatched GCC) that this patch does
not change the ABI.

OK for 4.8/4.9 once the mainline patch is in?

Bye,
Ulrich


gcc/ChangeLog:

	* config/rs6000/rs6000.c (rs6000_function_arg_boundary): Issue
	-Wpsabi note when encountering a type where future GCC releases
	will apply different alignment requirements.

gcc/testsuite/ChangeLog:

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

Comments

David Edelsohn July 18, 2014, 4:11 p.m. UTC | #1 
On Wed, Jul 16, 2014 at 8:39 PM, Ulrich Weigand <uweigand@de.ibm.com> wrote:
> Hello,
>
> this is the variant intended for the 4.8/4.9 branches of the patch:
> https://gcc.gnu.org/ml/gcc-patches/2014-07/msg00995.html
>
> As discussed, it does *not* actually change ABI, but only warn when
> encountering a situation where the ABI will change in a future GCC.
> (Avoiding the specific term "GCC 4.10" here since I'm not certain
> whether the next GCC release will in fact be called that ...)
>
> Tested on powerpc64-linux and powerpc64le-linux; also verified using
> the ABI compat suite (against an unpatched GCC) that this patch does
> not change the ABI.
>
> OK for 4.8/4.9 once the mainline patch is in?
>
> Bye,
> Ulrich
>
>
> gcc/ChangeLog:
>
>         * config/rs6000/rs6000.c (rs6000_function_arg_boundary): Issue
>         -Wpsabi note when encountering a type where future GCC releases
>         will apply different alignment requirements.
>
> gcc/testsuite/ChangeLog:
>
>         * gcc.target/powerpc/ppc64-abi-warn-2.c: New test.

This patch is okay if it is okay with the Release Managers.

Thanks, David
Jakub Jelinek July 18, 2014, 4:52 p.m. UTC | #2 
On Fri, Jul 18, 2014 at 12:11:56PM -0400, David Edelsohn wrote:
> > gcc/ChangeLog:
> >
> >         * config/rs6000/rs6000.c (rs6000_function_arg_boundary): Issue
> >         -Wpsabi note when encountering a type where future GCC releases
> >         will apply different alignment requirements.
> >
> > gcc/testsuite/ChangeLog:
> >
> >         * gcc.target/powerpc/ppc64-abi-warn-2.c: New test.
> 
> This patch is okay if it is okay with the Release Managers.

Ok.

	Jakub
diff mbox

Patch

Index: gcc-4_9-branch/gcc/config/rs6000/rs6000.c
===================================================================
--- gcc-4_9-branch.orig/gcc/config/rs6000/rs6000.c
+++ gcc-4_9-branch/gcc/config/rs6000/rs6000.c
@@ -9180,14 +9180,51 @@  rs6000_function_arg_boundary (enum machi
 	   || (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"
+		      " will change in a future GCC release",
+		      (int) TYPE_ALIGN (type) / BITS_PER_UNIT);
+	    }
+	}
+
+      /* GCC 4.8/4.9 Note: To avoid any ABI change on a release branch, we
+	 keep using the BLKmode check, but warn if there will be differences
+	 in future GCC releases.  */
+      if (mode == BLKmode)
+	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-4_9-branch/gcc/testsuite/gcc.target/powerpc/ppc64-abi-warn-2.c
===================================================================
--- /dev/null
+++ gcc-4_9-branch/gcc/testsuite/gcc.target/powerpc/ppc64-abi-warn-2.c
@@ -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 will change" } */
+{
+}
+