Re: [PATCH v3 1/2] aarch64: fix warning emission for ABI break since GCC 9.1
On 1/12/23 14:03, Richard Sandiford wrote: Christophe Lyon writes: While looking at PR 105549, which is about fixing the ABI break introduced in GCC 9.1 in parameter alignment with bit-fields, we noticed that the GCC 9.1 warning is not emitted in all the cases where it should be. This patch fixes that and the next patch in the series fixes the GCC 9.1 break. We split this into two patches since patch #2 introduces a new ABI break starting with GCC 13.1. This way, patch #1 can be back-ported to release branches if needed to fix the GCC 9.1 warning issue. The main idea is to add a new global boolean that indicates whether we're expanding the start of a function, so that aarch64_layout_arg can emit warnings for callees as well as callers. This removes the need for aarch64_function_arg_boundary to warn (with its incomplete information). However, in the first patch there are still cases where we emit warnings were we should not; this is fixed in patch #2 where we can distinguish between GCC 9.1 and GCC.13.1 ABI breaks properly. The fix in aarch64_function_arg_boundary (replacing & with &&) looks like an oversight of a previous commit in this area which changed 'abi_break' from a boolean to an integer. We also take the opportunity to fix the comment above aarch64_function_arg_alignment since the value of the abi_break parameter was changed in a previous commit, no longer matching the description. v2->v3: removed a bogus comment, added C++ tests (copied from the C ones) 2022-11-28 Christophe Lyon Richard Sandiford gcc/ChangeLog: * config/aarch64/aarch64.cc (aarch64_function_arg_alignment): Fix comment. (aarch64_layout_arg): Factorize warning conditions. (aarch64_function_arg_boundary): Fix typo. * function.cc (currently_expanding_function_start): New variable. (expand_function_start): Handle currently_expanding_function_start. * function.h (currently_expanding_function_start): Declare. gcc/testsuite/ChangeLog: * gcc.target/aarch64/bitfield-abi-warning-align16-O2.c: New test. * gcc.target/aarch64/bitfield-abi-warning-align16-O2-extra.c: New test. * gcc.target/aarch64/bitfield-abi-warning-align32-O2.c: New test. * gcc.target/aarch64/bitfield-abi-warning-align32-O2-extra.c: New test. * gcc.target/aarch64/bitfield-abi-warning-align8-O2.c: New test. * gcc.target/aarch64/bitfield-abi-warning.h: New test. * g++.target/aarch64/bitfield-abi-warning-align16-O2.C: New test. * g++.target/aarch64/bitfield-abi-warning-align16-O2-extra.C: New test. * g++.target/aarch64/bitfield-abi-warning-align32-O2.C: New test. * g++.target/aarch64/bitfield-abi-warning-align32-O2-extra.C: New test. * g++.target/aarch64/bitfield-abi-warning-align8-O2.C: New test. * g++.target/aarch64/bitfield-abi-warning.h: New test. OK for trunk, and for backports after a while. Thanks for doing this, and for your patience through it all. I've just backported this patch (only, not the other ones which were not intended for backport) - gcc-12: applied cleanly - gcc-11: applied cleany but I had to replace dg-note with dg-message in the tests - gcc-10: a small conflict, and I had to replace dg-note with dg-message in the tests Thanks, Christophe Richard --- gcc/config/aarch64/aarch64.cc | 28 +++- gcc/function.cc | 5 + gcc/function.h| 2 + .../bitfield-abi-warning-align16-O2-extra.C | 86 .../aarch64/bitfield-abi-warning-align16-O2.C | 87 .../bitfield-abi-warning-align32-O2-extra.C | 119 + .../aarch64/bitfield-abi-warning-align32-O2.C | 119 + .../aarch64/bitfield-abi-warning-align8-O2.C | 16 +++ .../g++.target/aarch64/bitfield-abi-warning.h | 125 ++ .../bitfield-abi-warning-align16-O2-extra.c | 86 .../aarch64/bitfield-abi-warning-align16-O2.c | 87 .../bitfield-abi-warning-align32-O2-extra.c | 119 + .../aarch64/bitfield-abi-warning-align32-O2.c | 119 + .../aarch64/bitfield-abi-warning-align8-O2.c | 16 +++ .../gcc.target/aarch64/bitfield-abi-warning.h | 125 ++ 15 files changed, 1132 insertions(+), 7 deletions(-) create mode 100644 gcc/testsuite/g++.target/aarch64/bitfield-abi-warning-align16-O2-extra.C create mode 100644 gcc/testsuite/g++.target/aarch64/bitfield-abi-warning-align16-O2.C create mode 100644 gcc/testsuite/g++.target/aarch64/bitfield-abi-warning-align32-O2-extra.C create mode 100644 gcc/testsuite/g++.target/aarch64/bitfield-abi-warning-align32-O2.C create mode 100644 gcc/testsuite/g++.target/aarch64/bitfield-abi-warning-align8-O2.C create mode 100644 gcc/testsuite/g++.target/aarch64/bitfield-abi-warning.h create mode 10
Re: [PATCH v3 1/2] aarch64: fix warning emission for ABI break since GCC 9.1
On 1/12/23 14:03, Richard Sandiford wrote: Christophe Lyon writes: While looking at PR 105549, which is about fixing the ABI break introduced in GCC 9.1 in parameter alignment with bit-fields, we noticed that the GCC 9.1 warning is not emitted in all the cases where it should be. This patch fixes that and the next patch in the series fixes the GCC 9.1 break. We split this into two patches since patch #2 introduces a new ABI break starting with GCC 13.1. This way, patch #1 can be back-ported to release branches if needed to fix the GCC 9.1 warning issue. The main idea is to add a new global boolean that indicates whether we're expanding the start of a function, so that aarch64_layout_arg can emit warnings for callees as well as callers. This removes the need for aarch64_function_arg_boundary to warn (with its incomplete information). However, in the first patch there are still cases where we emit warnings were we should not; this is fixed in patch #2 where we can distinguish between GCC 9.1 and GCC.13.1 ABI breaks properly. The fix in aarch64_function_arg_boundary (replacing & with &&) looks like an oversight of a previous commit in this area which changed 'abi_break' from a boolean to an integer. We also take the opportunity to fix the comment above aarch64_function_arg_alignment since the value of the abi_break parameter was changed in a previous commit, no longer matching the description. v2->v3: removed a bogus comment, added C++ tests (copied from the C ones) 2022-11-28 Christophe Lyon Richard Sandiford gcc/ChangeLog: * config/aarch64/aarch64.cc (aarch64_function_arg_alignment): Fix comment. (aarch64_layout_arg): Factorize warning conditions. (aarch64_function_arg_boundary): Fix typo. * function.cc (currently_expanding_function_start): New variable. (expand_function_start): Handle currently_expanding_function_start. * function.h (currently_expanding_function_start): Declare. gcc/testsuite/ChangeLog: * gcc.target/aarch64/bitfield-abi-warning-align16-O2.c: New test. * gcc.target/aarch64/bitfield-abi-warning-align16-O2-extra.c: New test. * gcc.target/aarch64/bitfield-abi-warning-align32-O2.c: New test. * gcc.target/aarch64/bitfield-abi-warning-align32-O2-extra.c: New test. * gcc.target/aarch64/bitfield-abi-warning-align8-O2.c: New test. * gcc.target/aarch64/bitfield-abi-warning.h: New test. * g++.target/aarch64/bitfield-abi-warning-align16-O2.C: New test. * g++.target/aarch64/bitfield-abi-warning-align16-O2-extra.C: New test. * g++.target/aarch64/bitfield-abi-warning-align32-O2.C: New test. * g++.target/aarch64/bitfield-abi-warning-align32-O2-extra.C: New test. * g++.target/aarch64/bitfield-abi-warning-align8-O2.C: New test. * g++.target/aarch64/bitfield-abi-warning.h: New test. OK for trunk, and for backports after a while. Thanks for doing this, and for your patience through it all. Great, and thanks for your help & patience too :-) Christophe Richard --- gcc/config/aarch64/aarch64.cc | 28 +++- gcc/function.cc | 5 + gcc/function.h| 2 + .../bitfield-abi-warning-align16-O2-extra.C | 86 .../aarch64/bitfield-abi-warning-align16-O2.C | 87 .../bitfield-abi-warning-align32-O2-extra.C | 119 + .../aarch64/bitfield-abi-warning-align32-O2.C | 119 + .../aarch64/bitfield-abi-warning-align8-O2.C | 16 +++ .../g++.target/aarch64/bitfield-abi-warning.h | 125 ++ .../bitfield-abi-warning-align16-O2-extra.c | 86 .../aarch64/bitfield-abi-warning-align16-O2.c | 87 .../bitfield-abi-warning-align32-O2-extra.c | 119 + .../aarch64/bitfield-abi-warning-align32-O2.c | 119 + .../aarch64/bitfield-abi-warning-align8-O2.c | 16 +++ .../gcc.target/aarch64/bitfield-abi-warning.h | 125 ++ 15 files changed, 1132 insertions(+), 7 deletions(-) create mode 100644 gcc/testsuite/g++.target/aarch64/bitfield-abi-warning-align16-O2-extra.C create mode 100644 gcc/testsuite/g++.target/aarch64/bitfield-abi-warning-align16-O2.C create mode 100644 gcc/testsuite/g++.target/aarch64/bitfield-abi-warning-align32-O2-extra.C create mode 100644 gcc/testsuite/g++.target/aarch64/bitfield-abi-warning-align32-O2.C create mode 100644 gcc/testsuite/g++.target/aarch64/bitfield-abi-warning-align8-O2.C create mode 100644 gcc/testsuite/g++.target/aarch64/bitfield-abi-warning.h create mode 100644 gcc/testsuite/gcc.target/aarch64/bitfield-abi-warning-align16-O2-extra.c create mode 100644 gcc/testsuite/gcc.target/aarch64/bitfield-abi-warning-align16-O2.c create mode 100644 gcc/testsuite/gcc.target/aarch64/bitfield-abi-warning-align32-O2
Re: [PATCH v3 1/2] aarch64: fix warning emission for ABI break since GCC 9.1
Christophe Lyon writes: > While looking at PR 105549, which is about fixing the ABI break > introduced in GCC 9.1 in parameter alignment with bit-fields, we > noticed that the GCC 9.1 warning is not emitted in all the cases where > it should be. This patch fixes that and the next patch in the series > fixes the GCC 9.1 break. > > We split this into two patches since patch #2 introduces a new ABI > break starting with GCC 13.1. This way, patch #1 can be back-ported > to release branches if needed to fix the GCC 9.1 warning issue. > > The main idea is to add a new global boolean that indicates whether > we're expanding the start of a function, so that aarch64_layout_arg > can emit warnings for callees as well as callers. This removes the > need for aarch64_function_arg_boundary to warn (with its incomplete > information). However, in the first patch there are still cases where > we emit warnings were we should not; this is fixed in patch #2 where > we can distinguish between GCC 9.1 and GCC.13.1 ABI breaks properly. > > The fix in aarch64_function_arg_boundary (replacing & with &&) looks > like an oversight of a previous commit in this area which changed > 'abi_break' from a boolean to an integer. > > We also take the opportunity to fix the comment above > aarch64_function_arg_alignment since the value of the abi_break > parameter was changed in a previous commit, no longer matching the > description. > > v2->v3: removed a bogus comment, added C++ tests (copied from the C > ones) > > 2022-11-28 Christophe Lyon > Richard Sandiford > > gcc/ChangeLog: > > * config/aarch64/aarch64.cc (aarch64_function_arg_alignment): Fix > comment. > (aarch64_layout_arg): Factorize warning conditions. > (aarch64_function_arg_boundary): Fix typo. > * function.cc (currently_expanding_function_start): New variable. > (expand_function_start): Handle > currently_expanding_function_start. > * function.h (currently_expanding_function_start): Declare. > > gcc/testsuite/ChangeLog: > > * gcc.target/aarch64/bitfield-abi-warning-align16-O2.c: New test. > * gcc.target/aarch64/bitfield-abi-warning-align16-O2-extra.c: New > test. > * gcc.target/aarch64/bitfield-abi-warning-align32-O2.c: New test. > * gcc.target/aarch64/bitfield-abi-warning-align32-O2-extra.c: New > test. > * gcc.target/aarch64/bitfield-abi-warning-align8-O2.c: New test. > * gcc.target/aarch64/bitfield-abi-warning.h: New test. > * g++.target/aarch64/bitfield-abi-warning-align16-O2.C: New test. > * g++.target/aarch64/bitfield-abi-warning-align16-O2-extra.C: New > test. > * g++.target/aarch64/bitfield-abi-warning-align32-O2.C: New test. > * g++.target/aarch64/bitfield-abi-warning-align32-O2-extra.C: New > test. > * g++.target/aarch64/bitfield-abi-warning-align8-O2.C: New test. > * g++.target/aarch64/bitfield-abi-warning.h: New test. OK for trunk, and for backports after a while. Thanks for doing this, and for your patience through it all. Richard > --- > gcc/config/aarch64/aarch64.cc | 28 +++- > gcc/function.cc | 5 + > gcc/function.h| 2 + > .../bitfield-abi-warning-align16-O2-extra.C | 86 > .../aarch64/bitfield-abi-warning-align16-O2.C | 87 > .../bitfield-abi-warning-align32-O2-extra.C | 119 + > .../aarch64/bitfield-abi-warning-align32-O2.C | 119 + > .../aarch64/bitfield-abi-warning-align8-O2.C | 16 +++ > .../g++.target/aarch64/bitfield-abi-warning.h | 125 ++ > .../bitfield-abi-warning-align16-O2-extra.c | 86 > .../aarch64/bitfield-abi-warning-align16-O2.c | 87 > .../bitfield-abi-warning-align32-O2-extra.c | 119 + > .../aarch64/bitfield-abi-warning-align32-O2.c | 119 + > .../aarch64/bitfield-abi-warning-align8-O2.c | 16 +++ > .../gcc.target/aarch64/bitfield-abi-warning.h | 125 ++ > 15 files changed, 1132 insertions(+), 7 deletions(-) > create mode 100644 > gcc/testsuite/g++.target/aarch64/bitfield-abi-warning-align16-O2-extra.C > create mode 100644 > gcc/testsuite/g++.target/aarch64/bitfield-abi-warning-align16-O2.C > create mode 100644 > gcc/testsuite/g++.target/aarch64/bitfield-abi-warning-align32-O2-extra.C > create mode 100644 > gcc/testsuite/g++.target/aarch64/bitfield-abi-warning-align32-O2.C > create mode 100644 > gcc/testsuite/g++.target/aarch64/bitfield-abi-warning-align8-O2.C > create mode 100644 gcc/testsuite/g++.target/aarch64/bitfield-abi-warning.h > create mode 100644 > gcc/testsuite/gcc.target/aarch64/bitfield-abi-warning-align16-O2-extra.c > create mode 100644 > gcc/testsuite/gcc.target/aarch64/bitfield-abi-warning-align16-O2.c > create mode 100644 > gcc/testsuite/gcc.target/aarch64/bitfield-abi-warning-align32-O2-extra.c >
[PATCH v3 1/2] aarch64: fix warning emission for ABI break since GCC 9.1
While looking at PR 105549, which is about fixing the ABI break introduced in GCC 9.1 in parameter alignment with bit-fields, we noticed that the GCC 9.1 warning is not emitted in all the cases where it should be. This patch fixes that and the next patch in the series fixes the GCC 9.1 break. We split this into two patches since patch #2 introduces a new ABI break starting with GCC 13.1. This way, patch #1 can be back-ported to release branches if needed to fix the GCC 9.1 warning issue. The main idea is to add a new global boolean that indicates whether we're expanding the start of a function, so that aarch64_layout_arg can emit warnings for callees as well as callers. This removes the need for aarch64_function_arg_boundary to warn (with its incomplete information). However, in the first patch there are still cases where we emit warnings were we should not; this is fixed in patch #2 where we can distinguish between GCC 9.1 and GCC.13.1 ABI breaks properly. The fix in aarch64_function_arg_boundary (replacing & with &&) looks like an oversight of a previous commit in this area which changed 'abi_break' from a boolean to an integer. We also take the opportunity to fix the comment above aarch64_function_arg_alignment since the value of the abi_break parameter was changed in a previous commit, no longer matching the description. v2->v3: removed a bogus comment, added C++ tests (copied from the C ones) 2022-11-28 Christophe Lyon Richard Sandiford gcc/ChangeLog: * config/aarch64/aarch64.cc (aarch64_function_arg_alignment): Fix comment. (aarch64_layout_arg): Factorize warning conditions. (aarch64_function_arg_boundary): Fix typo. * function.cc (currently_expanding_function_start): New variable. (expand_function_start): Handle currently_expanding_function_start. * function.h (currently_expanding_function_start): Declare. gcc/testsuite/ChangeLog: * gcc.target/aarch64/bitfield-abi-warning-align16-O2.c: New test. * gcc.target/aarch64/bitfield-abi-warning-align16-O2-extra.c: New test. * gcc.target/aarch64/bitfield-abi-warning-align32-O2.c: New test. * gcc.target/aarch64/bitfield-abi-warning-align32-O2-extra.c: New test. * gcc.target/aarch64/bitfield-abi-warning-align8-O2.c: New test. * gcc.target/aarch64/bitfield-abi-warning.h: New test. * g++.target/aarch64/bitfield-abi-warning-align16-O2.C: New test. * g++.target/aarch64/bitfield-abi-warning-align16-O2-extra.C: New test. * g++.target/aarch64/bitfield-abi-warning-align32-O2.C: New test. * g++.target/aarch64/bitfield-abi-warning-align32-O2-extra.C: New test. * g++.target/aarch64/bitfield-abi-warning-align8-O2.C: New test. * g++.target/aarch64/bitfield-abi-warning.h: New test. --- gcc/config/aarch64/aarch64.cc | 28 +++- gcc/function.cc | 5 + gcc/function.h| 2 + .../bitfield-abi-warning-align16-O2-extra.C | 86 .../aarch64/bitfield-abi-warning-align16-O2.C | 87 .../bitfield-abi-warning-align32-O2-extra.C | 119 + .../aarch64/bitfield-abi-warning-align32-O2.C | 119 + .../aarch64/bitfield-abi-warning-align8-O2.C | 16 +++ .../g++.target/aarch64/bitfield-abi-warning.h | 125 ++ .../bitfield-abi-warning-align16-O2-extra.c | 86 .../aarch64/bitfield-abi-warning-align16-O2.c | 87 .../bitfield-abi-warning-align32-O2-extra.c | 119 + .../aarch64/bitfield-abi-warning-align32-O2.c | 119 + .../aarch64/bitfield-abi-warning-align8-O2.c | 16 +++ .../gcc.target/aarch64/bitfield-abi-warning.h | 125 ++ 15 files changed, 1132 insertions(+), 7 deletions(-) create mode 100644 gcc/testsuite/g++.target/aarch64/bitfield-abi-warning-align16-O2-extra.C create mode 100644 gcc/testsuite/g++.target/aarch64/bitfield-abi-warning-align16-O2.C create mode 100644 gcc/testsuite/g++.target/aarch64/bitfield-abi-warning-align32-O2-extra.C create mode 100644 gcc/testsuite/g++.target/aarch64/bitfield-abi-warning-align32-O2.C create mode 100644 gcc/testsuite/g++.target/aarch64/bitfield-abi-warning-align8-O2.C create mode 100644 gcc/testsuite/g++.target/aarch64/bitfield-abi-warning.h create mode 100644 gcc/testsuite/gcc.target/aarch64/bitfield-abi-warning-align16-O2-extra.c create mode 100644 gcc/testsuite/gcc.target/aarch64/bitfield-abi-warning-align16-O2.c create mode 100644 gcc/testsuite/gcc.target/aarch64/bitfield-abi-warning-align32-O2-extra.c create mode 100644 gcc/testsuite/gcc.target/aarch64/bitfield-abi-warning-align32-O2.c create mode 100644 gcc/testsuite/gcc.target/aarch64/bitfield-abi-warning-align8-O2.c create mode 100644 gcc/testsuite/gcc.target/aarch64/bitfield-abi-warning.h diff --git a/gcc/config/aar
