Re: [PATCH] aarch64: Fix va_arg alignment handling (PR target/105549)
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
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 000..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 (, '\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 =
+ 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 = [2];
+ arg = __builtin_va_arg(ap,union S2844); /* This would fail. */
+ if (p->a != arg.a)
+
[PATCH] aarch64: Fix va_arg alignment handling (PR target/105549)
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
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 000..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 (, '\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 =
+ 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 = [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;
+
+
