* [aarch64] Fix ABI breakage with 128-bit bitfield types.
@ 2019-01-25 17:14 Richard Earnshaw (lists)
0 siblings, 0 replies; only message in thread
From: Richard Earnshaw (lists) @ 2019-01-25 17:14 UTC (permalink / raw)
To: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1108 bytes --]
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.
Committed to trunk.
[-- Attachment #2: a64-bitfield.patch --]
[-- Type: text/x-patch, Size: 9168 bytes --]
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 5df5a8b7843..d6a9955804f 100644
--- 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/gcc.target/aarch64/aapcs64/test_align-10.c b/gcc/testsuite/gcc.target/aarch64/aapcs64/test_align-10.c
new file mode 100644
index 00000000000..af0c8a1f412
--- /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
index 00000000000..357694902cd
--- /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
index 00000000000..5b3f74b51dc
--- /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
^ permalink raw reply [flat|nested] only message in thread
only message in thread, other threads:[~2019-01-25 17:13 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-25 17:14 [aarch64] Fix ABI breakage with 128-bit bitfield types Richard Earnshaw (lists)
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).