Message ID | 20220510150228.250435-1-christophe.lyon@arm.com |
---|---|
State | New |
Headers | show |
Series | aarch64: Fix va_arg alignment handling (PR target/105549) | expand |
I've reworked my patch for this PR, so this one is obsolete. The new one is: https://gcc.gnu.org/pipermail/gcc-patches/2022-June/596326.html On 5/10/22 17:02, Christophe Lyon wrote: > While working on enabling DFP for AArch64, I noticed new failures in > gcc.dg/compat/struct-layout-1.exp (t028) which were not actually > caused by DFP types handling. These tests are generated during 'make > check' and enabling DFP made generation different (not sure if new > non-DFP tests are generated, or if existing ones are generated > differently, the tests in question are huge and difficult to compare). > > Anyway, I reduced the problem to what I attach at the end of the new > gcc.target/aarch64/aapcs64/va_arg-17.c test and rewrote it in the same > scheme as other va_arg* AArch64 tests. > > This is a tough case mixing bitfields and alignment, where > aarch64_gimplify_va_arg_expr did not follow the exact same rule as > aarch64_layout_arg. When the va_arg parameter uses only one general > register, we do not want to introduce double-word alignment. > > The fix is thus very small, and this patch adds a new test > (va_arg-17.c), which contains the reduced offending testcase from > struct-layout-1.exp for reference. > > 2022-04-25 Christophe Lyon <christophe.lyon@arm.com> > > gcc/ > PR target/105549 > * config/aarch64/aarch64.cc (aarch64_gimplify_va_arg_expr): Fix > alignment of single-register parameters. > > gcc/testssuite/ > PR target/105549 > * gcc.target/aarch64/aapcs64/va_arg-17.c: New. > --- > gcc/config/aarch64/aarch64.cc | 4 +- > .../gcc.target/aarch64/aapcs64/va_arg-17.c | 105 ++++++++++++++++++ > 2 files changed, 108 insertions(+), 1 deletion(-) > create mode 100644 gcc/testsuite/gcc.target/aarch64/aapcs64/va_arg-17.c > > diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc > index f650abbc4ce..bd855758778 100644 > --- a/gcc/config/aarch64/aarch64.cc > +++ b/gcc/config/aarch64/aarch64.cc > @@ -19667,7 +19667,9 @@ aarch64_gimplify_va_arg_expr (tree valist, tree type, gimple_seq *pre_p, > rsize = ROUND_UP (size, UNITS_PER_WORD); > nregs = rsize / UNITS_PER_WORD; > > - if (align > 8) > + /* Align on double-word only if we need 2 registers, like in > + aarch64_layout_arg. */ > + if (align > 8 && nregs == 2) > { > if (abi_break && warn_psabi) > inform (input_location, "parameter passing for argument of type " > diff --git a/gcc/testsuite/gcc.target/aarch64/aapcs64/va_arg-17.c b/gcc/testsuite/gcc.target/aarch64/aapcs64/va_arg-17.c > new file mode 100644 > index 00000000000..24895c3ab48 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/aarch64/aapcs64/va_arg-17.c > @@ -0,0 +1,105 @@ > +/* Test AAPCS64 layout and __builtin_va_arg. > + > + This test covers a corner case where a composite type parameter fits in one > + register: we do not need a double-word alignment when accessing it in the > + va_arg stack area. */ > + > +/* { dg-do run { target aarch64*-*-* } } */ > + > +#ifndef IN_FRAMEWORK > +#define AAPCS64_TEST_STDARG > +#define TESTFILE "va_arg-17.c" > +#include "type-def.h" > + > +enum E6 { e6_0, e6_1, e6_2, e6_3, e6_65533 = 65533, e6_65534, e6_65535 }; > +typedef enum E6 Tal16E6 __attribute__((aligned (16))); > +typedef unsigned int Tuint; > + > +int fails; > + > +union S2844 { > + Tuint a:((((10) - 1) & 31) + 1); > + Tal16E6 __attribute__((aligned (2), packed)) b:31; > + struct{}c[0]; > +} ; > +union S2844 s2844; > +union S2844 a2844[5]; > + > +#define HAS_DATA_INIT_FUNC > +void init_data () > +{ > + memset (&s2844, '\0', sizeof (s2844)); > + memset (a2844, '\0', sizeof (a2844)); > + s2844.a = 799U; > + a2844[2].a = 586U; > +} > + > +#include "abitest.h" > +#else > + ARG (int , 1 , W0 , LAST_NAMED_ARG_ID) > + DOTS > + ANON_PROMOTED (float , 1.0f, double, 1.0, D0, 1) > + ANON (union S2844 , s2844 , X1 , 2) > + ANON (long long , 2LL , X2 , 3) > + ANON (union S2844 , a2844[2] , X3 , 4) > + LAST_ANON (union S2844 , a2844[2] , X4 , 5) > +#endif > + > +#if 0 > + /* This test is derived from a case generated by struct-layout-1.exp: */ > + > +enum E6 { e6_0, e6_1, e6_2, e6_3, e6_65533 = 65533, e6_65534, e6_65535 }; > +typedef enum E6 Tal16E6 __attribute__((aligned (16))); > +typedef unsigned int Tuint; > + > +int fails; > + > +union S2844 { > + Tuint a:((((10) - 1) & 31) + 1); > + Tal16E6 __attribute__((aligned (2), packed)) b:31; > + struct{}c[0]; > +} ; > +union S2844 s2844; > +union S2844 a2844[5]; > + > +typedef __builtin_va_list __gnuc_va_list; > +typedef __gnuc_va_list va_list; > + > +void check2844va (int z, ...) { > + union S2844 arg, *p; > + va_list ap; > + > + __builtin_va_start(ap,z); > + if (__builtin_va_arg(ap,double) != 1.0) > + printf ("fail %d.%d\n", 2844, 0), ++fails; > + > + p = &s2844; > + arg = __builtin_va_arg(ap,union S2844); /* This would fail. */ > + if (p->a != arg.a) > + printf ("fail %d.%d\n", 2844, 1), ++fails; > + > + if (__builtin_va_arg(ap,long long) != 3LL) > + printf ("fail %d.%d\n", 2844, 2), ++fails; > + > + p = &a2844[2]; > + arg = __builtin_va_arg(ap,union S2844); /* This would fail. */ > + if (p->a != arg.a) > + printf ("fail %d.%d\n", 2844, 3), ++fails; > + > + arg = __builtin_va_arg(ap,union S2844); /* This would fail. */ > + if (p->a != arg.a) > + printf ("fail %d.%d\n", 2844, 4), ++fails; > + > + __builtin_va_end(ap); > +} > + > +int main (void) { > + int i, j; > + memset (&s2844, '\0', sizeof (s2844)); > + memset (a2844, '\0', sizeof (a2844)); > + s2844.a = 799U; > + a2844[2].a = 586U; > + check2844va (1, 1.0, s2844, 2LL, a2844[2], a2844[2]); > + exit (fails != 0); > +} > +#endif /* 0 */
diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc index f650abbc4ce..bd855758778 100644 --- a/gcc/config/aarch64/aarch64.cc +++ b/gcc/config/aarch64/aarch64.cc @@ -19667,7 +19667,9 @@ aarch64_gimplify_va_arg_expr (tree valist, tree type, gimple_seq *pre_p, rsize = ROUND_UP (size, UNITS_PER_WORD); nregs = rsize / UNITS_PER_WORD; - if (align > 8) + /* Align on double-word only if we need 2 registers, like in + aarch64_layout_arg. */ + if (align > 8 && nregs == 2) { if (abi_break && warn_psabi) inform (input_location, "parameter passing for argument of type " diff --git a/gcc/testsuite/gcc.target/aarch64/aapcs64/va_arg-17.c b/gcc/testsuite/gcc.target/aarch64/aapcs64/va_arg-17.c new file mode 100644 index 00000000000..24895c3ab48 --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/aapcs64/va_arg-17.c @@ -0,0 +1,105 @@ +/* Test AAPCS64 layout and __builtin_va_arg. + + This test covers a corner case where a composite type parameter fits in one + register: we do not need a double-word alignment when accessing it in the + va_arg stack area. */ + +/* { dg-do run { target aarch64*-*-* } } */ + +#ifndef IN_FRAMEWORK +#define AAPCS64_TEST_STDARG +#define TESTFILE "va_arg-17.c" +#include "type-def.h" + +enum E6 { e6_0, e6_1, e6_2, e6_3, e6_65533 = 65533, e6_65534, e6_65535 }; +typedef enum E6 Tal16E6 __attribute__((aligned (16))); +typedef unsigned int Tuint; + +int fails; + +union S2844 { + Tuint a:((((10) - 1) & 31) + 1); + Tal16E6 __attribute__((aligned (2), packed)) b:31; + struct{}c[0]; +} ; +union S2844 s2844; +union S2844 a2844[5]; + +#define HAS_DATA_INIT_FUNC +void init_data () +{ + memset (&s2844, '\0', sizeof (s2844)); + memset (a2844, '\0', sizeof (a2844)); + s2844.a = 799U; + a2844[2].a = 586U; +} + +#include "abitest.h" +#else + ARG (int , 1 , W0 , LAST_NAMED_ARG_ID) + DOTS + ANON_PROMOTED (float , 1.0f, double, 1.0, D0, 1) + ANON (union S2844 , s2844 , X1 , 2) + ANON (long long , 2LL , X2 , 3) + ANON (union S2844 , a2844[2] , X3 , 4) + LAST_ANON (union S2844 , a2844[2] , X4 , 5) +#endif + +#if 0 + /* This test is derived from a case generated by struct-layout-1.exp: */ + +enum E6 { e6_0, e6_1, e6_2, e6_3, e6_65533 = 65533, e6_65534, e6_65535 }; +typedef enum E6 Tal16E6 __attribute__((aligned (16))); +typedef unsigned int Tuint; + +int fails; + +union S2844 { + Tuint a:((((10) - 1) & 31) + 1); + Tal16E6 __attribute__((aligned (2), packed)) b:31; + struct{}c[0]; +} ; +union S2844 s2844; +union S2844 a2844[5]; + +typedef __builtin_va_list __gnuc_va_list; +typedef __gnuc_va_list va_list; + +void check2844va (int z, ...) { + union S2844 arg, *p; + va_list ap; + + __builtin_va_start(ap,z); + if (__builtin_va_arg(ap,double) != 1.0) + printf ("fail %d.%d\n", 2844, 0), ++fails; + + p = &s2844; + arg = __builtin_va_arg(ap,union S2844); /* This would fail. */ + if (p->a != arg.a) + printf ("fail %d.%d\n", 2844, 1), ++fails; + + if (__builtin_va_arg(ap,long long) != 3LL) + printf ("fail %d.%d\n", 2844, 2), ++fails; + + p = &a2844[2]; + arg = __builtin_va_arg(ap,union S2844); /* This would fail. */ + if (p->a != arg.a) + printf ("fail %d.%d\n", 2844, 3), ++fails; + + arg = __builtin_va_arg(ap,union S2844); /* This would fail. */ + if (p->a != arg.a) + printf ("fail %d.%d\n", 2844, 4), ++fails; + + __builtin_va_end(ap); +} + +int main (void) { + int i, j; + memset (&s2844, '\0', sizeof (s2844)); + memset (a2844, '\0', sizeof (a2844)); + s2844.a = 799U; + a2844[2].a = 586U; + check2844va (1, 1.0, s2844, 2LL, a2844[2], a2844[2]); + exit (fails != 0); +} +#endif /* 0 */