Commit c590597c by Richard Earnshaw Committed by Richard Earnshaw

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
parent 3c35efc3
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> 2019-01-25 Richard Sandiford <richard.sandiford@arm.com>
PR middle-end/89037 PR middle-end/89037
......
...@@ -3765,12 +3765,16 @@ aarch64_vfp_is_call_candidate (cumulative_args_t pcum_v, machine_mode mode, ...@@ -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 /* Given MODE and TYPE of a function argument, return the alignment in
bits. The idea is to suppress any stronger alignment requested by 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). the user and opt for the natural alignment (specified in AAPCS64 \S
This is a helper function for local use only. */ 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 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) if (!type)
return GET_MODE_ALIGNMENT (mode); return GET_MODE_ALIGNMENT (mode);
...@@ -3786,9 +3790,22 @@ aarch64_function_arg_alignment (machine_mode mode, const_tree type) ...@@ -3786,9 +3790,22 @@ aarch64_function_arg_alignment (machine_mode mode, const_tree type)
return TYPE_ALIGN (TREE_TYPE (type)); return TYPE_ALIGN (TREE_TYPE (type));
unsigned int alignment = 0; unsigned int alignment = 0;
unsigned int bitfield_alignment = 0;
for (tree field = TYPE_FIELDS (type); field; field = DECL_CHAIN (field)) for (tree field = TYPE_FIELDS (type); field; field = DECL_CHAIN (field))
if (TREE_CODE (field) == FIELD_DECL) 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; return alignment;
} }
...@@ -3805,6 +3822,7 @@ aarch64_layout_arg (cumulative_args_t pcum_v, machine_mode mode, ...@@ -3805,6 +3822,7 @@ aarch64_layout_arg (cumulative_args_t pcum_v, machine_mode mode,
int ncrn, nvrn, nregs; int ncrn, nvrn, nregs;
bool allocate_ncrn, allocate_nvrn; bool allocate_ncrn, allocate_nvrn;
HOST_WIDE_INT size; HOST_WIDE_INT size;
bool abi_break;
/* We need to do this once per argument. */ /* We need to do this once per argument. */
if (pcum->aapcs_arg_processed) if (pcum->aapcs_arg_processed)
...@@ -3881,7 +3899,6 @@ aarch64_layout_arg (cumulative_args_t pcum_v, machine_mode mode, ...@@ -3881,7 +3899,6 @@ aarch64_layout_arg (cumulative_args_t pcum_v, machine_mode mode,
entirely general registers. */ entirely general registers. */
if (allocate_ncrn && (ncrn + nregs <= NUM_ARG_REGS)) if (allocate_ncrn && (ncrn + nregs <= NUM_ARG_REGS))
{ {
gcc_assert (nregs == 0 || nregs == 1 || nregs == 2); gcc_assert (nregs == 0 || nregs == 1 || nregs == 2);
/* C.8 if the argument has an alignment of 16 then the NGRN is /* C.8 if the argument has an alignment of 16 then the NGRN is
...@@ -3892,8 +3909,12 @@ aarch64_layout_arg (cumulative_args_t pcum_v, machine_mode mode, ...@@ -3892,8 +3909,12 @@ aarch64_layout_arg (cumulative_args_t pcum_v, machine_mode mode,
comparison is there because for > 16 * BITS_PER_UNIT comparison is there because for > 16 * BITS_PER_UNIT
alignment nregs should be > 2 and therefore it should be alignment nregs should be > 2 and therefore it should be
passed by reference rather than value. */ 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; ++ncrn;
gcc_assert (ncrn + nregs <= NUM_ARG_REGS); gcc_assert (ncrn + nregs <= NUM_ARG_REGS);
} }
...@@ -3931,9 +3952,18 @@ aarch64_layout_arg (cumulative_args_t pcum_v, machine_mode mode, ...@@ -3931,9 +3952,18 @@ aarch64_layout_arg (cumulative_args_t pcum_v, machine_mode mode,
on_stack: on_stack:
pcum->aapcs_stack_words = size / UNITS_PER_WORD; pcum->aapcs_stack_words = size / UNITS_PER_WORD;
if (aarch64_function_arg_alignment (mode, type) == 16 * BITS_PER_UNIT) if (aarch64_function_arg_alignment (mode, type, &abi_break)
pcum->aapcs_stack_size = ROUND_UP (pcum->aapcs_stack_size, == 16 * BITS_PER_UNIT)
16 / UNITS_PER_WORD); {
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; return;
} }
...@@ -4022,7 +4052,13 @@ aarch64_function_arg_regno_p (unsigned regno) ...@@ -4022,7 +4052,13 @@ aarch64_function_arg_regno_p (unsigned regno)
static unsigned int static unsigned int
aarch64_function_arg_boundary (machine_mode mode, const_tree type) 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); 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, ...@@ -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), stack = build3 (COMPONENT_REF, TREE_TYPE (f_stack), unshare_expr (valist),
f_stack, NULL_TREE); f_stack, NULL_TREE);
size = int_size_in_bytes (type); 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; dw_align = false;
adjust = 0; adjust = 0;
...@@ -13367,7 +13406,12 @@ aarch64_gimplify_va_arg_expr (tree valist, tree type, gimple_seq *pre_p, ...@@ -13367,7 +13406,12 @@ aarch64_gimplify_va_arg_expr (tree valist, tree type, gimple_seq *pre_p,
nregs = rsize / UNITS_PER_WORD; nregs = rsize / UNITS_PER_WORD;
if (align > 8) if (align > 8)
{
if (abi_break && warn_psabi)
inform (input_location, "parameter passing for argument of type "
"%qT changed in GCC 9.1", type);
dw_align = true; dw_align = true;
}
if (BLOCK_REG_PADDING (mode, type, 1) == PAD_DOWNWARD if (BLOCK_REG_PADDING (mode, type, 1) == PAD_DOWNWARD
&& size < UNITS_PER_WORD) && size < UNITS_PER_WORD)
......
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> 2019-01-25 Richard Sandiford <richard.sandiford@arm.com>
PR middle-end/89037 PR middle-end/89037
......
/* 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
/* 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
/* 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
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment