This is pretty unlikely in real code...

This is pretty unlikely in real code, but similar to Arm, the AArch64
ABI has a bug with the handling of 128-bit bit-fields, where if the
bit-field dominates the overall alignment the back-end code may end up
passing the argument correctly.  This is a regression that started in
gcc-6 when the ABI support code was updated to support overaligned
types.  The fix is very similar in concept to the Arm fix.  128-bit
bit-fields are fortunately extremely rare, so I'd be very surprised if
anyone has been bitten by this.

PR target/88469
gcc/
	* config/aarch64/aarch64.c (aarch64_function_arg_alignment): Add new
	argument ABI_BREAK.  Set to true if the calculated alignment has
	changed in gcc-9.  Check bit-fields for their base type alignment.
	(aarch64_layout_arg): Warn if argument passing has changed in gcc-9.
	(aarch64_function_arg_boundary): Likewise.
	(aarch64_gimplify_va_arg_expr): Likewise.

gcc/testsuite/
	* gcc.target/aarch64/aapcs64/test_align-10.c: New test.
	* gcc.target/aarch64/aapcs64/test_align-11.c: New test.
	* gcc.target/aarch64/aapcs64/test_align-12.c: New test.

From-SVN: r268273

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 2f7731a..4b8b1bd 100644 (file)
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,13 @@
+2019-01-25  Richard Earnshaw  <rearnsha@arm.com>
+
+       PR target/88469
+       * config/aarch64/aarch64.c (aarch64_function_arg_alignment): Add new
+       argument ABI_BREAK.  Set to true if the calculated alignment has
+       changed in gcc-9.  Check bit-fields for their base type alignment.
+       (aarch64_layout_arg): Warn if argument passing has changed in gcc-9.
+       (aarch64_function_arg_boundary): Likewise.
+       (aarch64_gimplify_va_arg_expr): Likewise.
+
 2019-01-25  Richard Sandiford  <richard.sandiford@arm.com>
 
        PR middle-end/89037

diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 8bddff9..d7c453c 100644 (file)
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -3765,12 +3765,16 @@ aarch64_vfp_is_call_candidate (cumulative_args_t pcum_v, machine_mode mode,
 
 /* Given MODE and TYPE of a function argument, return the alignment in
    bits.  The idea is to suppress any stronger alignment requested by
-   the user and opt for the natural alignment (specified in AAPCS64 \S 4.1).
-   This is a helper function for local use only.  */
+   the user and opt for the natural alignment (specified in AAPCS64 \S
+   4.1).  ABI_BREAK is set to true if the alignment was incorrectly
+   calculated in versions of GCC prior to GCC-9.  This is a helper
+   function for local use only.  */
 
 static unsigned int
-aarch64_function_arg_alignment (machine_mode mode, const_tree type)
+aarch64_function_arg_alignment (machine_mode mode, const_tree type,
+                               bool *abi_break)
 {
+  *abi_break = false;
   if (!type)
     return GET_MODE_ALIGNMENT (mode);
 
@@ -3786,9 +3790,22 @@ aarch64_function_arg_alignment (machine_mode mode, const_tree type)
     return TYPE_ALIGN (TREE_TYPE (type));
 
   unsigned int alignment = 0;
+  unsigned int bitfield_alignment = 0;
   for (tree field = TYPE_FIELDS (type); field; field = DECL_CHAIN (field))
     if (TREE_CODE (field) == FIELD_DECL)
-      alignment = std::max (alignment, DECL_ALIGN (field));
+      {
+       alignment = std::max (alignment, DECL_ALIGN (field));
+       if (DECL_BIT_FIELD_TYPE (field))
+         bitfield_alignment
+           = std::max (bitfield_alignment,
+                       TYPE_ALIGN (DECL_BIT_FIELD_TYPE (field)));
+      }
+
+  if (bitfield_alignment > alignment)
+    {
+      *abi_break = true;
+      return bitfield_alignment;
+    }
 
   return alignment;
 }
@@ -3805,6 +3822,7 @@ aarch64_layout_arg (cumulative_args_t pcum_v, machine_mode mode,
   int ncrn, nvrn, nregs;
   bool allocate_ncrn, allocate_nvrn;
   HOST_WIDE_INT size;
+  bool abi_break;
 
   /* We need to do this once per argument.  */
   if (pcum->aapcs_arg_processed)
@@ -3881,25 +3899,28 @@ aarch64_layout_arg (cumulative_args_t pcum_v, machine_mode mode,
      entirely general registers.  */
   if (allocate_ncrn && (ncrn + nregs <= NUM_ARG_REGS))
     {
-
       gcc_assert (nregs == 0 || nregs == 1 || nregs == 2);
 
       /* C.8 if the argument has an alignment of 16 then the NGRN is
-         rounded up to the next even number.  */
+        rounded up to the next even number.  */
       if (nregs == 2
          && ncrn % 2
          /* The == 16 * BITS_PER_UNIT instead of >= 16 * BITS_PER_UNIT
             comparison is there because for > 16 * BITS_PER_UNIT
             alignment nregs should be > 2 and therefore it should be
             passed by reference rather than value.  */
-         && aarch64_function_arg_alignment (mode, type) == 16 * BITS_PER_UNIT)
+         && (aarch64_function_arg_alignment (mode, type, &abi_break)
+             == 16 * BITS_PER_UNIT))
        {
+         if (abi_break && warn_psabi && currently_expanding_gimple_stmt)
+           inform (input_location, "parameter passing for argument of type "
+                   "%qT changed in GCC 9.1", type);
          ++ncrn;
          gcc_assert (ncrn + nregs <= NUM_ARG_REGS);
        }
 
       /* NREGS can be 0 when e.g. an empty structure is to be passed.
-         A reg is still generated for it, but the caller should be smart
+        A reg is still generated for it, but the caller should be smart
         enough not to use it.  */
       if (nregs == 0 || nregs == 1 || GET_MODE_CLASS (mode) == MODE_INT)
        pcum->aapcs_reg = gen_rtx_REG (mode, R0_REGNUM + ncrn);
@@ -3931,9 +3952,18 @@ aarch64_layout_arg (cumulative_args_t pcum_v, machine_mode mode,
 on_stack:
   pcum->aapcs_stack_words = size / UNITS_PER_WORD;
 
-  if (aarch64_function_arg_alignment (mode, type) == 16 * BITS_PER_UNIT)
-    pcum->aapcs_stack_size = ROUND_UP (pcum->aapcs_stack_size,
-                                      16 / UNITS_PER_WORD);
+  if (aarch64_function_arg_alignment (mode, type, &abi_break)
+      == 16 * BITS_PER_UNIT)
+    {
+      int new_size = ROUND_UP (pcum->aapcs_stack_size, 16 / UNITS_PER_WORD);
+      if (pcum->aapcs_stack_size != new_size)
+       {
+         if (abi_break && warn_psabi && currently_expanding_gimple_stmt)
+           inform (input_location, "parameter passing for argument of type "
+                   "%qT changed in GCC 9.1", type);
+         pcum->aapcs_stack_size = new_size;
+       }
+    }
   return;
 }
 
@@ -4022,7 +4052,13 @@ aarch64_function_arg_regno_p (unsigned regno)
 static unsigned int
 aarch64_function_arg_boundary (machine_mode mode, const_tree type)
 {
-  unsigned int alignment = aarch64_function_arg_alignment (mode, type);
+  bool abi_break;
+  unsigned int alignment = aarch64_function_arg_alignment (mode, type,
+                                                          &abi_break);
+  if (abi_break & warn_psabi)
+    inform (input_location, "parameter passing for argument of type "
+           "%qT changed in GCC 9.1", type);
+
   return MIN (MAX (alignment, PARM_BOUNDARY), STACK_BOUNDARY);
 }
 
@@ -13320,7 +13356,10 @@ aarch64_gimplify_va_arg_expr (tree valist, tree type, gimple_seq *pre_p,
   stack = build3 (COMPONENT_REF, TREE_TYPE (f_stack), unshare_expr (valist),
                  f_stack, NULL_TREE);
   size = int_size_in_bytes (type);
-  align = aarch64_function_arg_alignment (mode, type) / BITS_PER_UNIT;
+
+  bool abi_break;
+  align
+    = aarch64_function_arg_alignment (mode, type, &abi_break) / BITS_PER_UNIT;
 
   dw_align = false;
   adjust = 0;
@@ -13367,7 +13406,12 @@ aarch64_gimplify_va_arg_expr (tree valist, tree type, gimple_seq *pre_p,
       nregs = rsize / UNITS_PER_WORD;
 
       if (align > 8)
-       dw_align = true;
+       {
+         if (abi_break && warn_psabi)
+           inform (input_location, "parameter passing for argument of type "
+                   "%qT changed in GCC 9.1", type);
+         dw_align = true;
+       }
 
       if (BLOCK_REG_PADDING (mode, type, 1) == PAD_DOWNWARD
          && size < UNITS_PER_WORD)

diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 29d6426..cdb816e 100644 (file)
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,10 @@
+2019-01-25  Richard Earnshaw  <rearnsha@arm.com>
+
+       PR target/88469
+       * gcc.target/aarch64/aapcs64/test_align-10.c: New test.
+       * gcc.target/aarch64/aapcs64/test_align-11.c: New test.
+       * gcc.target/aarch64/aapcs64/test_align-12.c: New test.
+
 2019-01-25  Richard Sandiford  <richard.sandiford@arm.com>
 
        PR middle-end/89037

diff --git a/gcc/testsuite/gcc.target/aarch64/aapcs64/test_align-10.c b/gcc/testsuite/gcc.target/aarch64/aapcs64/test_align-10.c
new file mode 100644 (file)
index 0000000..af0c8a1
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/aapcs64/test_align-10.c
@@ -0,0 +1,44 @@
+/* Test AAPCS layout (alignment).  */
+
+/* { dg-do run { target aarch64*-*-* } } */
+
+#ifndef IN_FRAMEWORK
+#define TESTFILE "test_align-10.c"
+
+struct s
+{
+  /* Should have 128-bit alignment.  */
+  __int128 y : 65;
+  char z: 7;
+};
+
+typedef struct s T;
+
+#define EXPECTED_STRUCT_SIZE 16
+extern void link_failure (void);
+int
+foo ()
+{
+  /* Optimization gets rid of this before linking.  */
+  if (sizeof (struct s) != EXPECTED_STRUCT_SIZE)
+    link_failure ();
+}
+
+T a = { 1, 4 };
+T b = { 9, 16 };
+T c = { 25, 36 };
+
+#include "abitest.h"
+#else
+  ARG (int, 3, W0)
+  ARG (T, a, X2)
+  ARG (int, 5, W4)
+  ARG (T, b, X6)
+#ifndef __AAPCS64_BIG_ENDIAN__
+  ARG (int, 7, STACK)
+#else
+  ARG (int, 7, STACK + 4)
+#endif
+  /* Natural alignment should be 16.  */
+  LAST_ARG (T, c, STACK + 16)
+#endif

diff --git a/gcc/testsuite/gcc.target/aarch64/aapcs64/test_align-11.c b/gcc/testsuite/gcc.target/aarch64/aapcs64/test_align-11.c
new file mode 100644 (file)
index 0000000..3576949
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/aapcs64/test_align-11.c
@@ -0,0 +1,44 @@
+/* Test AAPCS layout (alignment).  */
+
+/* { dg-do run { target aarch64*-*-* } } */
+
+#ifndef IN_FRAMEWORK
+#define TESTFILE "test_align-11.c"
+
+struct s
+{
+  /* Should have 128-bit alignment and still detected as a bitfield.  */
+  __int128 y : 64;
+  char z: 7;
+};
+
+typedef struct s T;
+
+#define EXPECTED_STRUCT_SIZE 16
+extern void link_failure (void);
+int
+foo ()
+{
+  /* Optimization gets rid of this before linking.  */
+  if (sizeof (struct s) != EXPECTED_STRUCT_SIZE)
+    link_failure ();
+}
+
+T a = { 1, 4 };
+T b = { 9, 16 };
+T c = { 25, 36 };
+
+#include "abitest.h"
+#else
+  ARG (int, 3, W0)
+  ARG (T, a, X2)
+  ARG (int, 5, W4)
+  ARG (T, b, X6)
+#ifndef __AAPCS64_BIG_ENDIAN__
+  ARG (int, 7, STACK)
+#else
+  ARG (int, 7, STACK + 4)
+#endif
+  /* Natural alignment should be 16.  */
+  LAST_ARG (T, c, STACK + 16)
+#endif

diff --git a/gcc/testsuite/gcc.target/aarch64/aapcs64/test_align-12.c b/gcc/testsuite/gcc.target/aarch64/aapcs64/test_align-12.c
new file mode 100644 (file)
index 0000000..5b3f74b
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/aapcs64/test_align-12.c
@@ -0,0 +1,45 @@
+/* Test AAPCS layout (alignment).  */
+
+/* { dg-do run { target aarch64*-*-* } } */
+
+#ifndef IN_FRAMEWORK
+#define TESTFILE "test_align-12.c"
+
+struct s
+{
+  /* Should have 64-bit alignment.  */
+  long long y : 57;
+  char z: 7;
+};
+
+typedef struct s T;
+
+#define EXPECTED_STRUCT_SIZE 8
+extern void link_failure (void);
+int
+foo ()
+{
+  /* Optimization gets rid of this before linking.  */
+  if (sizeof (struct s) != EXPECTED_STRUCT_SIZE)
+    link_failure ();
+}
+
+T a = { 1, 4 };
+T b = { 9, 16 };
+T c = { 25, 36 };
+
+#include "abitest.h"
+#else
+  ARG (int, 3, W0)
+  ARG (T, a, X1)
+  ARG (int, 5, W2)
+  ARG (T, b, X3)
+  ARG (__int128, 11, X4)
+  ARG (__int128, 13, X6)
+#ifndef __AAPCS64_BIG_ENDIAN__
+  ARG (int, 7, STACK)
+#else
+  ARG (int, 7, STACK + 4)
+#endif
+  LAST_ARG (T, c, STACK + 8)
+#endif