Hi Tamar, > On 7 Jul 2026, at 10:55, Tamar Christina <[email protected]> wrote: > >> -----Original Message----- >> From: [email protected] <[email protected]> >> Sent: 17 June 2026 15:45 >> To: [email protected] >> Cc: Richard Earnshaw <[email protected]>; Kyrylo Tkachov >> <[email protected]> >> Subject: [PATCH] aarch64: Fix ICE in aarch64_function_arg_alignment with >> may_alias [PR124146] >> >> From: Kyrylo Tkachov <[email protected]> >> >> aarch64_function_arg_alignment returns the ABI alignment of an argument. >> For scalars and vectors this is the natural alignment of the type, >> ignoring any user-specified alignment. The code obtains the natural >> alignment from the TYPE_MAIN_VARIANT, relying on the main variant having >> no user alignment. PR108910 showed that this does not hold for pointers, >> and that case is handled explicitly. >> >> PR124146 is another counterexample. An attribute that affects type >> identity, such as may_alias, makes a type its own TYPE_MAIN_VARIANT, so >> the main variant retains the user alignment requested by the aligned >> attribute. TYPE_MAIN_VARIANT therefore does not strip the alignment and >> the gcc_assert (!TYPE_USER_ALIGN (type)) fires. >> >> In the testcase the may_alias+aligned type reaches the argument-passing >> code because foo is inlined into bar and forwprop propagates the value >> of that type directly into the recursive call to bar. >> >> Fix it by ignoring user alignment explicitly: when the type still has >> user alignment, use the natural alignment of its mode, exactly as the >> !type path at the top of the function already does. Behaviour is >> unchanged for the existing (non-user-aligned) cases, since the natural >> alignment of a scalar or vector equals its mode alignment. This also >> gives the natural alignment when a may_alias typedef lowers the >> alignment of a 16-byte type, matching the AAPCS64 (and Clang). >> >> Bootstrapped and tested on aarch64-none-linux-gnu. >> >> Ok for trunk? > > LGTM, using the MODE alignment here looks correct. > > Should we add a testcase for _BigInt? > > typedef __attribute__((aligned(8), may_alias)) _BitInt(128) ma8_bi128; > void consume_bi128 (int, _BitInt(128)); > void pass_bi128 (ma8_bi128 y) { consume_bi128 (5, y); } > > It seems to trip the same assert and I think your fix should handle it > correctly. > > i.e. that <= 64 _BitInts are DImode and >= 64 && <= 128 are TImode and > 129 > are passed by reference?
Yes, I agree. I posted an updated patch that I will push to trunk soon. Thanks, Kyrill > > Thanks, > Tamar > >> Thanks, >> Kyrill >> >> Signed-off-by: Kyrylo Tkachov <[email protected]> >> >> gcc/ >> >> PR target/124146 >> * config/aarch64/aarch64.cc (aarch64_function_arg_alignment): >> Ignore user alignment left on a type's main variant; use the >> mode's natural alignment instead. >> >> gcc/testsuite/ >> >> PR target/124146 >> * gcc.target/aarch64/pr124146.c: New test. >> * gcc.target/aarch64/pr124146-2.c: New test. >> * gcc.target/aarch64/pr124146-3.c: New test. >> --- >> gcc/config/aarch64/aarch64.cc | 10 ++++- >> gcc/testsuite/gcc.target/aarch64/pr124146-2.c | 38 ++++++++++++++++ >> gcc/testsuite/gcc.target/aarch64/pr124146-3.c | 45 >> +++++++++++++++++++ >> gcc/testsuite/gcc.target/aarch64/pr124146.c | 29 ++++++++++++ >> 4 files changed, 121 insertions(+), 1 deletion(-) >> create mode 100644 gcc/testsuite/gcc.target/aarch64/pr124146-2.c >> create mode 100644 gcc/testsuite/gcc.target/aarch64/pr124146-3.c >> create mode 100644 gcc/testsuite/gcc.target/aarch64/pr124146.c >> >> diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc >> index 42e56512c61..924dbcea0bd 100644 >> --- a/gcc/config/aarch64/aarch64.cc >> +++ b/gcc/config/aarch64/aarch64.cc >> @@ -7493,7 +7493,15 @@ aarch64_function_arg_alignment >> (machine_mode mode, const_tree type, >> *abi_break_gcc_14 = TYPE_ALIGN (type); >> type = TYPE_MAIN_VARIANT (TREE_TYPE (type)); >> } >> - gcc_assert (!TYPE_USER_ALIGN (type)); >> + /* Ignore any user-specified alignment: the AAPCS64 uses the >> + type's natural alignment for scalars and vectors. We normally >> + strip user alignment by taking the TYPE_MAIN_VARIANT above, but >> + an attribute that affects type identity (such as may_alias) can >> + make a type its own main variant while still recording the user >> + alignment, so handle that case explicitly here (PR124146). For >> + a scalar or vector the natural alignment is that of its mode. */ >> + if (TYPE_USER_ALIGN (type)) >> + return GET_MODE_ALIGNMENT (mode); >> return TYPE_ALIGN (type); >> } >> >> diff --git a/gcc/testsuite/gcc.target/aarch64/pr124146-2.c >> b/gcc/testsuite/gcc.target/aarch64/pr124146-2.c >> new file mode 100644 >> index 00000000000..3dabdc59e02 >> --- /dev/null >> +++ b/gcc/testsuite/gcc.target/aarch64/pr124146-2.c >> @@ -0,0 +1,38 @@ >> +/* PR target/124146 */ >> +/* { dg-do compile } */ >> +/* { dg-options "-O2" } */ >> + >> +/* A type that combines an alignment attribute with an attribute that >> affects >> + type identity (may_alias) is its own TYPE_MAIN_VARIANT and keeps >> + TYPE_USER_ALIGN set. aarch64_function_arg_alignment used to assert >> that >> + the main variant had no user alignment, which ICEd for such types. >> Verify >> + that a wide range of them can be passed (alone and after another >> argument, >> + over-aligned and under-aligned) and returned without an ICE. */ >> + >> +typedef int v4si __attribute__((__vector_size__ (16))); >> + >> +#define TEST(BASE, SUF) >> \ >> + typedef __attribute__((__aligned__, __may_alias__)) BASE big_##SUF; >> \ >> + typedef __attribute__((__aligned__ (8), __may_alias__)) BASE al8_##SUF; \ >> + void gbig_##SUF (big_##SUF); >> \ >> + void hbig_##SUF (int, big_##SUF); \ >> + void gal8_##SUF (al8_##SUF); >> \ >> + void hal8_##SUF (int, al8_##SUF); \ >> + void call_##SUF (big_##SUF a, al8_##SUF b) \ >> + { \ >> + gbig_##SUF (a); \ >> + hbig_##SUF (1, a); \ >> + gal8_##SUF (b); \ >> + hal8_##SUF (1, b); \ >> + } \ >> + big_##SUF ret_##SUF (big_##SUF a) { return a; } >> + >> +TEST (unsigned char, uc) >> +TEST (unsigned short, us) >> +TEST (unsigned int, ui) >> +TEST (unsigned long, ul) >> +TEST (long long, ll) >> +TEST (__int128, i128) >> +TEST (float, f) >> +TEST (double, d) >> +TEST (v4si, v) >> diff --git a/gcc/testsuite/gcc.target/aarch64/pr124146-3.c >> b/gcc/testsuite/gcc.target/aarch64/pr124146-3.c >> new file mode 100644 >> index 00000000000..f9ca9319d77 >> --- /dev/null >> +++ b/gcc/testsuite/gcc.target/aarch64/pr124146-3.c >> @@ -0,0 +1,45 @@ >> +/* PR target/124146 */ >> +/* { dg-do compile } */ >> +/* { dg-options "-O2" } */ >> +/* { dg-final { check-function-bodies "**" "" } } */ >> + >> +/* The AAPCS64 passes scalars and vectors using their natural alignment, >> + ignoring user alignment. A 16-byte __int128 must therefore use its >> + natural 16-byte alignment (so the C.8 rule rounds NGRN up to an even >> + register), even when a may_alias typedef records a different user >> + alignment. These checks would have ICEd before the PR124146 fix. */ >> + >> +typedef __attribute__((__aligned__, __may_alias__)) __int128 ma_i128; >> +typedef __attribute__((__aligned__ (8), __may_alias__)) __int128 ma8_i128; >> +typedef __attribute__((__aligned__, __may_alias__)) unsigned long ma_ul; >> + >> +void consume_i128 (int, __int128); >> +void consume_ul (int, unsigned long); >> + >> +/* Over-aligned __int128: natural alignment 16 -> argument in x2/x3. >> +** pass_i128: >> +** mov x2, x0 >> +** mov x3, x1 >> +** mov w0, 5 >> +** b consume_i128 >> +*/ >> +void pass_i128 (ma_i128 y) { consume_i128 (5, y); } >> + >> +/* Under-aligned (aligned(8)) __int128: natural alignment is still 16, so >> the >> + argument must still land in x2/x3, not x1/x2. >> +** pass_i128_underaligned: >> +** mov x2, x0 >> +** mov x3, x1 >> +** mov w0, 5 >> +** b consume_i128 >> +*/ >> +void pass_i128_underaligned (ma8_i128 y) { consume_i128 (5, y); } >> + >> +/* Over-aligned unsigned long: a single 8-byte register, alignment is >> + irrelevant to placement -> argument in x1. >> +** pass_ul: >> +** mov x1, x0 >> +** mov w0, 5 >> +** b consume_ul >> +*/ >> +void pass_ul (ma_ul y) { consume_ul (5, y); } >> diff --git a/gcc/testsuite/gcc.target/aarch64/pr124146.c >> b/gcc/testsuite/gcc.target/aarch64/pr124146.c >> new file mode 100644 >> index 00000000000..29142847ed4 >> --- /dev/null >> +++ b/gcc/testsuite/gcc.target/aarch64/pr124146.c >> @@ -0,0 +1,29 @@ >> +/* PR target/124146 */ >> +/* { dg-do compile } */ >> +/* { dg-options "-O1" } */ >> + >> +/* The may_alias attribute makes the typedef T its own >> TYPE_MAIN_VARIANT >> + while still recording the user alignment from the aligned attribute. >> + Taking the main variant in aarch64_function_arg_alignment therefore did >> + not strip the user alignment, which used to trigger an assertion failure >> + (ICE) when foo was inlined into bar and the value of type T was passed >> + directly to bar. */ >> + >> +long a; >> +void *b; >> +char c; >> + >> +long >> +foo (void *p) >> +{ >> + typedef __attribute__((__aligned__)) __attribute__((__may_alias__)) >> unsigned long T; >> + a = *(T *) b; >> + return a; >> +} >> + >> +void >> +bar (unsigned long x) >> +{ >> + long d = foo (&c); >> + bar (d); >> +} >> -- >> 2.50.1 (Apple Git-155) >
