From c590597c45948c6e6fa282878198fd226da95998 Mon Sep 17 00:00:00 2001 From: Richard Earnshaw Date: Fri, 25 Jan 2019 17:09:33 +0000 Subject: [PATCH] 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 --- gcc/ChangeLog | 10 +++ gcc/config/aarch64/aarch64.c | 72 +++++++++++++++---- gcc/testsuite/ChangeLog | 7 ++ .../aarch64/aapcs64/test_align-10.c | 44 ++++++++++++ .../aarch64/aapcs64/test_align-11.c | 44 ++++++++++++ .../aarch64/aapcs64/test_align-12.c | 45 ++++++++++++ 6 files changed, 208 insertions(+), 14 deletions(-) create mode 100644 gcc/testsuite/gcc.target/aarch64/aapcs64/test_align-10.c create mode 100644 gcc/testsuite/gcc.target/aarch64/aapcs64/test_align-11.c create mode 100644 gcc/testsuite/gcc.target/aarch64/aapcs64/test_align-12.c diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 2f7731afc22..4b8b1bd1c87 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,13 @@ +2019-01-25 Richard Earnshaw + + 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 PR middle-end/89037 diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c index 8bddff9029b..d7c453cdad0 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/ChangeLog b/gcc/testsuite/ChangeLog index 29d6426febd..cdb816e9066 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,10 @@ +2019-01-25 Richard Earnshaw + + 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 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 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 -- 2.30.2