Re: [PATCH v3 2/2] aarch64: Fix bit-field alignment in param passing [PR105549]
On 1/17/23 13:48, Jakub Jelinek wrote: On Tue, Jan 17, 2023 at 01:43:35PM +0100, Christophe Lyon wrote: As a follow-up to this, I ran the full testsuite with -fstack-protector-all and this results in lots of failures (~65000 in gcc.sum alone). I guess that is way too much. Since you also mentioned -fstack-protector-strong, I ran the full testsuite with it, which results in more failures too but the difference is much smaller than with -fstack-protector=all (from 126 FAIL to 309) But this could be doable by adding explicit -fno-stack-protector options to test that can't handle those. For instance, I see many failures with -fstack-protector-strong in: gcc.target/aarch64/sve/pcs/ It looks like you have them too, according to the logs I downloaded from your link above. So is it worth adding -fno-stack-protector to my few new testcases? (I can, no problem, but just wondering why you appear to notice the problem with my new tests, and not with the ones in gcc.target/aarch64/sve/pcs/) Because I mainly look for regressions (compare the test_summary dumps against older gcc build); if something fails for years, it doesn't show up in the regression diffs. OK that's what I thought, thanks for confirming. I'll add -fno-stack-protector to my tests. Thanks, Christophe Jakub
Re: [PATCH v3 2/2] aarch64: Fix bit-field alignment in param passing [PR105549]
On Tue, Jan 17, 2023 at 01:43:35PM +0100, Christophe Lyon wrote: > As a follow-up to this, I ran the full testsuite with -fstack-protector-all > and this results in lots of failures (~65000 in gcc.sum alone). I guess that is way too much. > Since you also mentioned -fstack-protector-strong, I ran the full testsuite > with it, which results in more failures too but the difference is much > smaller than with -fstack-protector=all (from 126 FAIL to 309) But this could be doable by adding explicit -fno-stack-protector options to test that can't handle those. > For instance, I see many failures with -fstack-protector-strong in: > gcc.target/aarch64/sve/pcs/ > It looks like you have them too, according to the logs I downloaded from > your link above. > > So is it worth adding -fno-stack-protector to my few new testcases? > (I can, no problem, but just wondering why you appear to notice the problem > with my new tests, and not with the ones in gcc.target/aarch64/sve/pcs/) Because I mainly look for regressions (compare the test_summary dumps against older gcc build); if something fails for years, it doesn't show up in the regression diffs. Jakub
Re: [PATCH v3 2/2] aarch64: Fix bit-field alignment in param passing [PR105549]
Hi Jakub, On 1/15/23 17:54, Christophe Lyon via Gcc-patches wrote: Hi! On 1/13/23 16:38, Jakub Jelinek wrote: On Wed, Jan 11, 2023 at 03:18:06PM +0100, Christophe Lyon via Gcc-patches 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. Richard Sandiford further reduced this to a non-vararg function, added as a second testcase. This is a tough case mixing bit-fields and alignment, where aarch64_function_arg_alignment did not follow what its descriptive comment says: we want to use the natural alignment of the bit-field type only if the user didn't reduce the alignment for the bit-field itself. The patch also adds a comment and assert that would help someone who has to look at this area again. The fix would be very small, except that this introduces a new ABI break, and we have to warn about that. Since this actually fixes a problem introduced in GCC 9.1, we keep the old computation to detect when we now behave differently. This patch adds two new tests (va_arg-17.c and pr105549.c). va_arg-17.c contains the reduced offending testcase from struct-layout-1.exp for reference. We update some tests introduced by the previous patch, where parameters with bit-fields and packed attribute now emit a different warning. I'm seeing +FAIL: g++.target/aarch64/bitfield-abi-warning-align16-O2.C scan-assembler-times and\\tw0, w1, 1 10 +FAIL: g++.target/aarch64/bitfield-abi-warning-align32-O2.C scan-assembler-times and\\tw0, w1, 1 10 +FAIL: g++.target/aarch64/bitfield-abi-warning-align8-O2.C scan-assembler-times and\\tw0, w0, 1 11 +FAIL: g++.target/aarch64/bitfield-abi-warning-align8-O2.C scan-assembler-times and\\tw0, w1, 1 18 +FAIL: gcc.target/aarch64/sve/pcs/struct_3_128.c -march=armv8.2-a+sve (internal compiler error: in aarch64_layout_arg, at config/aarch64/aarch64.cc:7696) +FAIL: gcc.target/aarch64/sve/pcs/struct_3_128.c -march=armv8.2-a+sve (test for excess errors) +FAIL: gcc.target/aarch64/sve/pcs/struct_3_256.c -march=armv8.2-a+sve (internal compiler error: in aarch64_layout_arg, at config/aarch64/aarch64.cc:7696) +FAIL: gcc.target/aarch64/sve/pcs/struct_3_256.c -march=armv8.2-a+sve (test for excess errors) +FAIL: gcc.target/aarch64/sve/pcs/struct_3_512.c -march=armv8.2-a+sve (internal compiler error: in aarch64_layout_arg, at config/aarch64/aarch64.cc:7696) +FAIL: gcc.target/aarch64/sve/pcs/struct_3_512.c -march=armv8.2-a+sve (test for excess errors) regressions with this change. Really deeply sorry for this :-( aarch64.cc:7696 is for me the newly added: + gcc_assert (alignment <= 16 * BITS_PER_UNIT + && (!alignment || abi_break < alignment) + && (!abi_break_packed || alignment < abi_break_packed)); assert. Details in https://kojipkgs.fedoraproject.org//work/tasks/2857/96062857/build.log (configure line etc.), plus if you wget https://kojipkgs.fedoraproject.org//work/tasks/2857/96062857/build.log sed -n '/^begin /,/^end/p' build.log | uuencode > you get a compressed tarball with the testsuite *.log files. Thanks I managed to download this (you meant uudecode rather than uuencode ;-) ) I see the scan-assembler-times are also failing in gcc.target, I guess you just forgot to paste them? From your other message, it seems you are building with stack-protector enabled by default, but I can't see that in the configure lines? Indeed I just checked the generated code with/without -fstack-protector-all, and it obviously changes a lot, thus breaking the fragile scan-assembler directives. As you said, it's easy to avoid with -fno-stack-protector. As a follow-up to this, I ran the full testsuite with -fstack-protector-all and this results in lots of failures (~65000 in gcc.sum alone). Since you also mentioned -fstack-protector-strong, I ran the full testsuite with it, which results in more failures too but the difference is much smaller than with -fstack-protector=all (from 126 FAIL to 309) For instance, I see many failures with -fstack-protector-strong in: gcc.target/aarch64/sve/pcs/ It looks like you have them too, according to the logs I downloaded from your link above. So is it worth adding -fno-stack-protector to my few new testcases? (I can, no problem, but just wondering why you appear to notice the problem with my new tests, and not with the ones in gcc.target/aarch64/sve/pcs/) Thanks, Christophe I'll check the problem with the assert. Thanks and sorry, Christophe Jakub
Re: [PATCH v3 2/2] aarch64: Fix bit-field alignment in param passing [PR105549]
Hi! On 1/13/23 16:38, Jakub Jelinek wrote: On Wed, Jan 11, 2023 at 03:18:06PM +0100, Christophe Lyon via Gcc-patches 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. Richard Sandiford further reduced this to a non-vararg function, added as a second testcase. This is a tough case mixing bit-fields and alignment, where aarch64_function_arg_alignment did not follow what its descriptive comment says: we want to use the natural alignment of the bit-field type only if the user didn't reduce the alignment for the bit-field itself. The patch also adds a comment and assert that would help someone who has to look at this area again. The fix would be very small, except that this introduces a new ABI break, and we have to warn about that. Since this actually fixes a problem introduced in GCC 9.1, we keep the old computation to detect when we now behave differently. This patch adds two new tests (va_arg-17.c and pr105549.c). va_arg-17.c contains the reduced offending testcase from struct-layout-1.exp for reference. We update some tests introduced by the previous patch, where parameters with bit-fields and packed attribute now emit a different warning. I'm seeing +FAIL: g++.target/aarch64/bitfield-abi-warning-align16-O2.C scan-assembler-times and\\tw0, w1, 1 10 +FAIL: g++.target/aarch64/bitfield-abi-warning-align32-O2.C scan-assembler-times and\\tw0, w1, 1 10 +FAIL: g++.target/aarch64/bitfield-abi-warning-align8-O2.C scan-assembler-times and\\tw0, w0, 1 11 +FAIL: g++.target/aarch64/bitfield-abi-warning-align8-O2.C scan-assembler-times and\\tw0, w1, 1 18 +FAIL: gcc.target/aarch64/sve/pcs/struct_3_128.c -march=armv8.2-a+sve (internal compiler error: in aarch64_layout_arg, at config/aarch64/aarch64.cc:7696) +FAIL: gcc.target/aarch64/sve/pcs/struct_3_128.c -march=armv8.2-a+sve (test for excess errors) +FAIL: gcc.target/aarch64/sve/pcs/struct_3_256.c -march=armv8.2-a+sve (internal compiler error: in aarch64_layout_arg, at config/aarch64/aarch64.cc:7696) +FAIL: gcc.target/aarch64/sve/pcs/struct_3_256.c -march=armv8.2-a+sve (test for excess errors) +FAIL: gcc.target/aarch64/sve/pcs/struct_3_512.c -march=armv8.2-a+sve (internal compiler error: in aarch64_layout_arg, at config/aarch64/aarch64.cc:7696) +FAIL: gcc.target/aarch64/sve/pcs/struct_3_512.c -march=armv8.2-a+sve (test for excess errors) regressions with this change. Really deeply sorry for this :-( aarch64.cc:7696 is for me the newly added: + gcc_assert (alignment <= 16 * BITS_PER_UNIT + && (!alignment || abi_break < alignment) + && (!abi_break_packed || alignment < abi_break_packed)); assert. Details in https://kojipkgs.fedoraproject.org//work/tasks/2857/96062857/build.log (configure line etc.), plus if you wget https://kojipkgs.fedoraproject.org//work/tasks/2857/96062857/build.log sed -n '/^begin /,/^end/p' build.log | uuencode > you get a compressed tarball with the testsuite *.log files. Thanks I managed to download this (you meant uudecode rather than uuencode ;-) ) I see the scan-assembler-times are also failing in gcc.target, I guess you just forgot to paste them? From your other message, it seems you are building with stack-protector enabled by default, but I can't see that in the configure lines? Indeed I just checked the generated code with/without -fstack-protector-all, and it obviously changes a lot, thus breaking the fragile scan-assembler directives. As you said, it's easy to avoid with -fno-stack-protector. I'll check the problem with the assert. Thanks and sorry, Christophe Jakub
Re: [PATCH v3 2/2] aarch64: Fix bit-field alignment in param passing [PR105549]
On Fri, Jan 13, 2023 at 08:25:01PM +0100, Jakub Jelinek via Gcc-patches wrote:
> alignment is 256, which is not <= 16 * BITS_PER_UNIT.
> type is pst_uniform4 with user alignment of 32 bytes:
> struct pst_uniform4
> {
> fixed_int32_t a __attribute__((aligned(SVE_BYTES * 2)));
> fixed_int32_t b[3] __attribute__((aligned(SVE_BYTES * 2)));
> fixed_int32_t c __attribute__((aligned(SVE_BYTES * 2)));
> };
> and with -march=armv8.2-a+sve -msve-vector-bits=128
> __ARM_FEATURE_SVE_BITS and therefore SVE_BYTES is 128 and so
> the alignment seems requested.
typedef __SVInt32_t fixed_int32_t __attribute__ ((arm_sve_vector_bits (128)));
struct A { fixed_int32_t a[3] __attribute__((aligned((128 / 8) * 2))); };
void foo (struct A x) {}
-march=armv8.2-a+sve -O -msve-vector-bits=128
reproduces it too.
Jakub
Re: [PATCH v3 2/2] aarch64: Fix bit-field alignment in param passing [PR105549]
On Fri, Jan 13, 2023 at 04:38:00PM +0100, Jakub Jelinek via Gcc-patches wrote:
> I'm seeing
> +FAIL: g++.target/aarch64/bitfield-abi-warning-align16-O2.C
> scan-assembler-times and\\tw0, w1, 1 10
> +FAIL: g++.target/aarch64/bitfield-abi-warning-align32-O2.C
> scan-assembler-times and\\tw0, w1, 1 10
> +FAIL: g++.target/aarch64/bitfield-abi-warning-align8-O2.C
> scan-assembler-times and\\tw0, w0, 1 11
> +FAIL: g++.target/aarch64/bitfield-abi-warning-align8-O2.C
> scan-assembler-times and\\tw0, w1, 1 18
The above seems only because I'm testing with
> +FAIL: gcc.target/aarch64/sve/pcs/struct_3_128.c -march=armv8.2-a+sve
> (internal compiler error: in aarch64_layout_arg, at
> config/aarch64/aarch64.cc:7696)
> +FAIL: gcc.target/aarch64/sve/pcs/struct_3_128.c -march=armv8.2-a+sve (test
> for excess errors)
> +FAIL: gcc.target/aarch64/sve/pcs/struct_3_256.c -march=armv8.2-a+sve
> (internal compiler error: in aarch64_layout_arg, at
> config/aarch64/aarch64.cc:7696)
> +FAIL: gcc.target/aarch64/sve/pcs/struct_3_256.c -march=armv8.2-a+sve (test
> for excess errors)
> +FAIL: gcc.target/aarch64/sve/pcs/struct_3_512.c -march=armv8.2-a+sve
> (internal compiler error: in aarch64_layout_arg, at
> config/aarch64/aarch64.cc:7696)
> +FAIL: gcc.target/aarch64/sve/pcs/struct_3_512.c -march=armv8.2-a+sve (test
> for excess errors)
> regressions with this change.
But these I can reproduce using a cross compiler on current trunk:
#0 fancy_abort (file=0x2da73c0 "../../gcc/config/aarch64/aarch64.cc",
line=7710, function=0x2da8917 "aarch64_layout_arg") at
../../gcc/diagnostic.cc:2219
#1 0x01a8756b in aarch64_layout_arg (pcum_v=..., arg=...) at
../../gcc/config/aarch64/aarch64.cc:7710
#2 0x01a88477 in aarch64_function_arg_advance (pcum_v=..., arg=...) at
../../gcc/config/aarch64/aarch64.cc:8023
#3 0x0107cb17 in gimplify_parameters (cleanup=0x7fffd8c0) at
../../gcc/function.cc:3929
#4 0x01156366 in gimplify_body (fndecl=, do_parms=true) at ../../gcc/gimplify.cc:17619
#5 0x01156ca0 in gimplify_function_tree (fndecl=) at ../../gcc/gimplify.cc:17822
#6 0x00ea2402 in cgraph_node::analyze (this=) at ../../gcc/cgraphunit.cc:676
#7 0x00ea4489 in analyze_functions (first_time=true) at
../../gcc/cgraphunit.cc:1240
#8 0x00ea7a06 in symbol_table::finalize_compilation_unit
(this=0x7fffea38b000) at ../../gcc/cgraphunit.cc:2547
#9 0x01572df1 in compile_file () at ../../gcc/toplev.cc:471
#10 0x01575caf in do_compile (no_backend=false) at
../../gcc/toplev.cc:2125
#11 0x01576078 in toplev::main (this=0x7fffdc6a, argc=14,
argv=0x7fffdd98) at ../../gcc/toplev.cc:2277
#12 0x02a81c6a in main (argc=14, argv=0x7fffdd98) at
../../gcc/main.cc:39
alignment is 256, which is not <= 16 * BITS_PER_UNIT.
type is pst_uniform4 with user alignment of 32 bytes:
struct pst_uniform4
{
fixed_int32_t a __attribute__((aligned(SVE_BYTES * 2)));
fixed_int32_t b[3] __attribute__((aligned(SVE_BYTES * 2)));
fixed_int32_t c __attribute__((aligned(SVE_BYTES * 2)));
};
and with -march=armv8.2-a+sve -msve-vector-bits=128
__ARM_FEATURE_SVE_BITS and therefore SVE_BYTES is 128 and so
the alignment seems requested.
Jakub
Re: [PATCH v3 2/2] aarch64: Fix bit-field alignment in param passing [PR105549]
On Wed, Jan 11, 2023 at 03:18:06PM +0100, Christophe Lyon via Gcc-patches 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. Richard Sandiford further > reduced this to a non-vararg function, added as a second testcase. > > This is a tough case mixing bit-fields and alignment, where > aarch64_function_arg_alignment did not follow what its descriptive > comment says: we want to use the natural alignment of the bit-field > type only if the user didn't reduce the alignment for the bit-field > itself. > > The patch also adds a comment and assert that would help someone who > has to look at this area again. > > The fix would be very small, except that this introduces a new ABI > break, and we have to warn about that. Since this actually fixes a > problem introduced in GCC 9.1, we keep the old computation to detect > when we now behave differently. > > This patch adds two new tests (va_arg-17.c and > pr105549.c). va_arg-17.c contains the reduced offending testcase from > struct-layout-1.exp for reference. We update some tests introduced by > the previous patch, where parameters with bit-fields and packed > attribute now emit a different warning. I'm seeing +FAIL: g++.target/aarch64/bitfield-abi-warning-align16-O2.C scan-assembler-times and\\tw0, w1, 1 10 +FAIL: g++.target/aarch64/bitfield-abi-warning-align32-O2.C scan-assembler-times and\\tw0, w1, 1 10 +FAIL: g++.target/aarch64/bitfield-abi-warning-align8-O2.C scan-assembler-times and\\tw0, w0, 1 11 +FAIL: g++.target/aarch64/bitfield-abi-warning-align8-O2.C scan-assembler-times and\\tw0, w1, 1 18 +FAIL: gcc.target/aarch64/sve/pcs/struct_3_128.c -march=armv8.2-a+sve (internal compiler error: in aarch64_layout_arg, at config/aarch64/aarch64.cc:7696) +FAIL: gcc.target/aarch64/sve/pcs/struct_3_128.c -march=armv8.2-a+sve (test for excess errors) +FAIL: gcc.target/aarch64/sve/pcs/struct_3_256.c -march=armv8.2-a+sve (internal compiler error: in aarch64_layout_arg, at config/aarch64/aarch64.cc:7696) +FAIL: gcc.target/aarch64/sve/pcs/struct_3_256.c -march=armv8.2-a+sve (test for excess errors) +FAIL: gcc.target/aarch64/sve/pcs/struct_3_512.c -march=armv8.2-a+sve (internal compiler error: in aarch64_layout_arg, at config/aarch64/aarch64.cc:7696) +FAIL: gcc.target/aarch64/sve/pcs/struct_3_512.c -march=armv8.2-a+sve (test for excess errors) regressions with this change. aarch64.cc:7696 is for me the newly added: > + gcc_assert (alignment <= 16 * BITS_PER_UNIT > + && (!alignment || abi_break < alignment) > + && (!abi_break_packed || alignment < abi_break_packed)); assert. Details in https://kojipkgs.fedoraproject.org//work/tasks/2857/96062857/build.log (configure line etc.), plus if you wget https://kojipkgs.fedoraproject.org//work/tasks/2857/96062857/build.log sed -n '/^begin /,/^end/p' build.log | uuencode you get a compressed tarball with the testsuite *.log files. Jakub
Re: [PATCH v3 2/2] aarch64: Fix bit-field alignment in param passing [PR105549]
On 1/12/23 14:19, Richard Sandiford wrote:
Christophe Lyon writes:
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. Richard Sandiford further
reduced this to a non-vararg function, added as a second testcase.
This is a tough case mixing bit-fields and alignment, where
aarch64_function_arg_alignment did not follow what its descriptive
comment says: we want to use the natural alignment of the bit-field
type only if the user didn't reduce the alignment for the bit-field
itself.
The patch also adds a comment and assert that would help someone who
has to look at this area again.
The fix would be very small, except that this introduces a new ABI
break, and we have to warn about that. Since this actually fixes a
problem introduced in GCC 9.1, we keep the old computation to detect
when we now behave differently.
This patch adds two new tests (va_arg-17.c and
pr105549.c). va_arg-17.c contains the reduced offending testcase from
struct-layout-1.exp for reference. We update some tests introduced by
the previous patch, where parameters with bit-fields and packed
attribute now emit a different warning.
v2->v3: testcase update
2022-11-28 Christophe Lyon
Richard Sandiford
gcc/
PR target/105549
* config/aarch64/aarch64.cc (aarch64_function_arg_alignment):
Check DECL_PACKED for bitfield.
(aarch64_layout_arg): Warn when parameter passing ABI changes.
(aarch64_function_arg_boundary): Do not warn here.
(aarch64_gimplify_va_arg_expr): Warn when parameter passing ABI
changes.
gcc/testsuite/
PR target/105549
* gcc.target/aarch64/bitfield-abi-warning-align16-O2.c: Update.
* gcc.target/aarch64/bitfield-abi-warning-align16-O2-extra.c: Update.
* gcc.target/aarch64/bitfield-abi-warning-align32-O2.c: Update.
* gcc.target/aarch64/bitfield-abi-warning-align32-O2-extra.c: Update.
* gcc.target/aarch64/aapcs64/va_arg-17.c: New test.
* gcc.target/aarch64/pr105549.c: New test.
* g++.target/aarch64/bitfield-abi-warning-align16-O2.C: Update.
* g++.target/aarch64/bitfield-abi-warning-align16-O2-extra.C: Update.
* g++.target/aarch64/bitfield-abi-warning-align32-O2.C: Update.
* g++.target/aarch64/bitfield-abi-warning-align32-O2-extra.C: Update.
---
gcc/config/aarch64/aarch64.cc | 148 ++
.../bitfield-abi-warning-align16-O2-extra.C | 64
.../aarch64/bitfield-abi-warning-align16-O2.C | 48 +++---
.../bitfield-abi-warning-align32-O2-extra.C | 131 +++-
.../aarch64/bitfield-abi-warning-align32-O2.C | 132
.../gcc.target/aarch64/aapcs64/va_arg-17.c| 105 +
.../bitfield-abi-warning-align16-O2-extra.c | 64
.../aarch64/bitfield-abi-warning-align16-O2.c | 48 +++---
.../bitfield-abi-warning-align32-O2-extra.c | 131 +++-
.../aarch64/bitfield-abi-warning-align32-O2.c | 132
gcc/testsuite/gcc.target/aarch64/pr105549.c | 12 ++
11 files changed, 587 insertions(+), 428 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/aarch64/aapcs64/va_arg-17.c
create mode 100644 gcc/testsuite/gcc.target/aarch64/pr105549.c
[...]
@@ -68,14 +62,14 @@
/* { dg-note {parameter passing for argument of type 'S4' changed in GCC 9.1}
"" { target *-*-* } 103 } f4_stdarg */
/* { dg-note {parameter passing for argument of type 'S8' changed in GCC 9.1}
"" { target *-*-* } 104 } f8_stdarg */
-/* Parameter passing for these should not have changed in GCC 9.1 (PR 105549).
+/* FIXME Parameter passing for these should not have changed in GCC 9.1 (PR
105549).
Fortunately we warn. Note the discrepancy with lines 120-124 below: we warn
in the callee, but not in the caller. */
Looks like a stray change. Same for the C test.
Ha yes, thanks for catching this!
OK otherwise, thanks.
Richard
Re: [PATCH v3 2/2] aarch64: Fix bit-field alignment in param passing [PR105549]
Christophe Lyon writes:
> 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. Richard Sandiford further
> reduced this to a non-vararg function, added as a second testcase.
>
> This is a tough case mixing bit-fields and alignment, where
> aarch64_function_arg_alignment did not follow what its descriptive
> comment says: we want to use the natural alignment of the bit-field
> type only if the user didn't reduce the alignment for the bit-field
> itself.
>
> The patch also adds a comment and assert that would help someone who
> has to look at this area again.
>
> The fix would be very small, except that this introduces a new ABI
> break, and we have to warn about that. Since this actually fixes a
> problem introduced in GCC 9.1, we keep the old computation to detect
> when we now behave differently.
>
> This patch adds two new tests (va_arg-17.c and
> pr105549.c). va_arg-17.c contains the reduced offending testcase from
> struct-layout-1.exp for reference. We update some tests introduced by
> the previous patch, where parameters with bit-fields and packed
> attribute now emit a different warning.
>
> v2->v3: testcase update
>
> 2022-11-28 Christophe Lyon
> Richard Sandiford
>
> gcc/
> PR target/105549
> * config/aarch64/aarch64.cc (aarch64_function_arg_alignment):
> Check DECL_PACKED for bitfield.
> (aarch64_layout_arg): Warn when parameter passing ABI changes.
> (aarch64_function_arg_boundary): Do not warn here.
> (aarch64_gimplify_va_arg_expr): Warn when parameter passing ABI
> changes.
>
> gcc/testsuite/
> PR target/105549
> * gcc.target/aarch64/bitfield-abi-warning-align16-O2.c: Update.
> * gcc.target/aarch64/bitfield-abi-warning-align16-O2-extra.c: Update.
> * gcc.target/aarch64/bitfield-abi-warning-align32-O2.c: Update.
> * gcc.target/aarch64/bitfield-abi-warning-align32-O2-extra.c: Update.
> * gcc.target/aarch64/aapcs64/va_arg-17.c: New test.
> * gcc.target/aarch64/pr105549.c: New test.
> * g++.target/aarch64/bitfield-abi-warning-align16-O2.C: Update.
> * g++.target/aarch64/bitfield-abi-warning-align16-O2-extra.C: Update.
> * g++.target/aarch64/bitfield-abi-warning-align32-O2.C: Update.
> * g++.target/aarch64/bitfield-abi-warning-align32-O2-extra.C: Update.
> ---
> gcc/config/aarch64/aarch64.cc | 148 ++
> .../bitfield-abi-warning-align16-O2-extra.C | 64
> .../aarch64/bitfield-abi-warning-align16-O2.C | 48 +++---
> .../bitfield-abi-warning-align32-O2-extra.C | 131 +++-
> .../aarch64/bitfield-abi-warning-align32-O2.C | 132
> .../gcc.target/aarch64/aapcs64/va_arg-17.c| 105 +
> .../bitfield-abi-warning-align16-O2-extra.c | 64
> .../aarch64/bitfield-abi-warning-align16-O2.c | 48 +++---
> .../bitfield-abi-warning-align32-O2-extra.c | 131 +++-
> .../aarch64/bitfield-abi-warning-align32-O2.c | 132
> gcc/testsuite/gcc.target/aarch64/pr105549.c | 12 ++
> 11 files changed, 587 insertions(+), 428 deletions(-)
> create mode 100644 gcc/testsuite/gcc.target/aarch64/aapcs64/va_arg-17.c
> create mode 100644 gcc/testsuite/gcc.target/aarch64/pr105549.c
> [...]
> @@ -68,14 +62,14 @@
> /* { dg-note {parameter passing for argument of type 'S4' changed in GCC
> 9.1} "" { target *-*-* } 103 } f4_stdarg */
> /* { dg-note {parameter passing for argument of type 'S8' changed in GCC
> 9.1} "" { target *-*-* } 104 } f8_stdarg */
>
> -/* Parameter passing for these should not have changed in GCC 9.1 (PR
> 105549).
> +/* FIXME Parameter passing for these should not have changed in GCC 9.1 (PR
> 105549).
> Fortunately we warn. Note the discrepancy with lines 120-124 below: we
> warn
> in the callee, but not in the caller. */
Looks like a stray change. Same for the C test.
OK otherwise, thanks.
Richard
