The testcase and a few others now ICEs on x86_64 with -m32 since LOCAL_DECL_ALIGNMENT with -mpreferred-stack-boundary=2 shrinks alignment of 'long long' from 8 to 4. /home/rguenther/src/trunk/gcc/testsuite/gcc.target/i386/pr69454-2.c: In function 'fn1':^M /home/rguenther/src/trunk/gcc/testsuite/gcc.target/i386/pr69454-2.c:7:6: internal compiler error: in execute, at adjust-alignment.c:73^M 0x216245b execute^M ../../src/trunk/gcc/adjust-alignment.c:73^M Please submit a full bug report,^M this was a latent wrong-code bug before if you consider typedef __UINTPTR_TYPE__ uintptr_t; void __attribute__((noipa)) foo (long long *p, uintptr_t a) { if ((uintptr_t)p & (a-1)) __builtin_abort (); } int main() { long long x; uintptr_t a = __alignof__(x); foo(&x, a); return 0; } and the frame of main not being aligned to 8 bytes.
FAIL: c-c++-common/gomp/declare-variant-5.c (internal compiler error) FAIL: c-c++-common/gomp/declare-variant-5.c (internal compiler error) FAIL: c-c++-common/gomp/declare-variant-5.c (internal compiler error) FAIL: c-c++-common/gomp/declare-variant-5.c (test for excess errors) FAIL: c-c++-common/gomp/declare-variant-5.c (test for excess errors) FAIL: c-c++-common/gomp/declare-variant-5.c (test for excess errors) FAIL: gcc.target/i386/pr69454-2.c (internal compiler error) FAIL: gcc.target/i386/pr69454-2.c (test for excess errors) FAIL: gcc.target/i386/stackalign/longlong-1.c -mno-stackrealign (internal compiler error) FAIL: gcc.target/i386/stackalign/longlong-1.c -mno-stackrealign (test for excess errors) FAIL: gcc.target/i386/stackalign/longlong-1.c -mstackrealign (internal compiler error) FAIL: gcc.target/i386/stackalign/longlong-1.c -mstackrealign (test for excess errors)
*** Bug 95370 has been marked as a duplicate of this bug. ***
ICEs are "fixed" by the first hunk, the testcase in Comment #0 by the second: --cut here-- diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c index 060e2df62ea..cd7abaf7e04 100644 --- a/gcc/config/i386/i386.c +++ b/gcc/config/i386/i386.c @@ -16752,6 +16752,7 @@ ix86_local_alignment (tree exp, machine_mode mode, decl = NULL; } +#if 0 /* Don't do dynamic stack realignment for long long objects with -mpreferred-stack-boundary=2. */ if (!TARGET_64BIT @@ -16761,6 +16762,7 @@ ix86_local_alignment (tree exp, machine_mode mode, && (!type || !TYPE_USER_ALIGN (type)) && (!decl || !DECL_USER_ALIGN (decl))) align = 32; +#endif /* If TYPE is NULL, we are allocating a stack slot for caller-save register in MODE. We will return the largest alignment of XF @@ -16868,6 +16870,7 @@ ix86_minimum_alignment (tree exp, machine_mode mode, if (TARGET_64BIT || align != 64 || ix86_preferred_stack_boundary >= 64) return align; +#if 0 /* Don't do dynamic stack realignment for long long objects with -mpreferred-stack-boundary=2. */ if ((mode == DImode || (type && TYPE_MODE (type) == DImode)) @@ -16877,6 +16880,7 @@ ix86_minimum_alignment (tree exp, machine_mode mode, gcc_checking_assert (!TARGET_STV); return 32; } +#endif return align; } --cut here--
This test case and many other regression on x86 caused by following change set r11-508 Good ==== $ /local/skpandey/gccwork/pr95237/tools-build/gcc-debug-r11-507/release/usr/gcc-11.0.0-x86-64/bin/gcc -m32 -mpreferred-stack-boundary=2 foo.c bash-5.0$ echo $? 0 Bad === bash-5.0$ /local/skpandey/gccwork/pr95237/tools-build/gcc-debug-r11-508/release/usr/gcc-11.0.0-x86-64/bin/gcc -m32 -mpreferred-stack-boundary=2 foo.c during GIMPLE pass: adjust_alignment foo.c: In function ??main??: foo.c:7:5: internal compiler error: in execute, at adjust-alignment.c:73 7 | int main() | ^~~~ 0x206a091 execute /local/skpandey/gccwork/pr95237/gcc/gcc/adjust-alignment.c:73 Please submit a full bug report, with preprocessed source if appropriate. Please include the complete backtrace with any bug report. See <https://gcc.gnu.org/bugs/> for instructions. $git show dfa4fcdba374ed44d4aa1a22b2738f3f5c5b37af commit dfa4fcdba374ed44d4aa1a22b2738f3f5c5b37af Author: Kito Cheng <kito.cheng@sifive.com> Date: Tue Apr 14 14:53:19 2020 +0800 Fix alignment for local variable [PR90811] - The alignment for local variable was adjust during estimate_stack_frame_size, however it seems wrong spot to adjust that, expand phase will adjust that but it little too late to some gimple optimization, which rely on certain target hooks need to check alignment, forwprop is an example for that, result of simplify_builtin_call rely on the alignment on some target like ARM or RISC-V. - Exclude static local var and hard register var in the process of alignment adjustment. - This patch fix gfortran.dg/pr45636.f90 for arm and riscv. - Regression test on riscv32/riscv64 and x86_64-linux-gnu, no new fail introduced. gcc/ChangeLog PR target/90811 * Makefile.in (OBJS): Add adjust-alignment.o. * adjust-alignment.c (pass_data_adjust_alignment): New. (pass_adjust_alignment): New. (pass_adjust_alignment::execute): New. (make_pass_adjust_alignment): New. * tree-pass.h (make_pass_adjust_alignment): New. * passes.def: Add pass_adjust_alignment. diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 9eee988e12c..fdaa94ae8b9 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,14 @@ +2020-05-20 Kito Cheng <kito.cheng@sifive.com> + + PR target/90811 + * Makefile.in (OBJS): Add adjust-alignment.o. + * adjust-alignment.c (pass_data_adjust_alignment): New. + (pass_adjust_alignment): New. + (pass_adjust_alignment::execute): New. + (make_pass_adjust_alignment): New. + * tree-pass.h (make_pass_adjust_alignment): New. + * passes.def: Add pass_adjust_alignment. GDB dump: ========= The program being debugged has been started already. Start it from the beginning? (y or n) y Starting program: /local/skpandey/gccwork/pr95237/tools-build/gcc-debug-r11-508/release/usr/gcc-11.0.0-x86-64/libexec/gcc/x86_64-pc-linux-gnu/11.0.0/cc1 -quiet -imultilib 32 -iprefix /local/skpandey/gccwork/pr95237/tools-build/gcc-debug-r11-508/release/usr/gcc-11.0.0-x86-64/bin/../lib/gcc/x86_64-pc-linux-gnu/11.0.0/ foo.c -quiet -dumpbase foo.c -m32 -mpreferred-stack-boundary=2 -mtune=generic -march=x86-64 -auxbase foo -o /tmp/ccEnTcu1.s File "/usr/share/gdb/auto-load/usr/lib64/libisl.so.15.1.1-gdb.py", line 67 print "No isl printer for this type" ^ SyntaxError: Missing parentheses in call to 'print'. Did you mean print("No isl printer for this type")? Breakpoint 1, (anonymous namespace)::pass_adjust_alignment::execute (this=0x32fbb10, fun=0x7fffea9340b0) at /local/skpandey/gccwork/pr95237/gcc/gcc/adjust-alignment.c:73 73 gcc_assert (align >= DECL_ALIGN (var)); (gdb) c Continuing. Breakpoint 1, (anonymous namespace)::pass_adjust_alignment::execute (this=0x32fbb10, fun=0x7fffea9340b0) at /local/skpandey/gccwork/pr95237/gcc/gcc/adjust-alignment.c:73 73 gcc_assert (align >= DECL_ALIGN (var)); (gdb) n during GIMPLE pass: adjust_alignment foo.c: In function ??main??: foo.c:7:5: internal compiler error: in execute, at adjust-alignment.c:73 7 | int main() | ^~~~ 0x206a091 execute /local/skpandey/gccwork/pr95237/gcc/gcc/adjust-alignment.c:73 Please submit a full bug report, with preprocessed source if appropriate. Please include the complete backtrace with any bug report. See <https://gcc.gnu.org/bugs/> for instructions. [Inferior 1 (process 4132225) exited with code 04] (gdb) Can we just exclude adjust-alignment pass for x86?
On i386, we don't do dynamic stack realignment for long long objects with -mpreferred-stack-boundary=2 on purpose. A backend should be allowed to opt-out gcc_assert (align >= DECL_ALIGN (var)); based one variable type.
Created attachment 48658 [details] i386-Implement-ROUND_TYPE_ALIGN-to-make-sure-alignme.patch Some optimization might made decision depend on the alignment, and alignment shrinking might made the decision become wrong. So I prefer keep the assertion checking and implement ROUND_TYPE_ALIGN for x86, so that it will set the alignment properly from the beginning. PoC/untested patch attached.
*** Bug 95460 has been marked as a duplicate of this bug. ***
(In reply to Kito Cheng from comment #6) > Created attachment 48658 [details] > i386-Implement-ROUND_TYPE_ALIGN-to-make-sure-alignme.patch > > Some optimization might made decision depend on the alignment, and alignment > shrinking might made the decision become wrong. Nothing will happen on i386 if alignment of long long isn't 8 bytes. > So I prefer keep the assertion checking and implement ROUND_TYPE_ALIGN for > x86, so that it will set the alignment properly from the beginning. > > PoC/untested patch attached. The i386 psABI specifies 4 byte alignment for long long. But we want to use 8 byte alignment if there is no ABI implication and no stack realignment is needed. Will ROUND_TYPE_ALIGN always align long long to 8 bytes?
On Tue, 2 Jun 2020, hjl.tools at gmail dot com wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95237 > > --- Comment #8 from H.J. Lu <hjl.tools at gmail dot com> --- > (In reply to Kito Cheng from comment #6) > > Created attachment 48658 [details] > > i386-Implement-ROUND_TYPE_ALIGN-to-make-sure-alignme.patch > > > > Some optimization might made decision depend on the alignment, and alignment > > shrinking might made the decision become wrong. > > Nothing will happen on i386 if alignment of long long isn't 8 bytes. GIMPLE passes see DECL_ALIGN and can for example elide runtime checks like if(_Alignof(var) != 8) which is bogus when the actual alignment is not ensured. For this reason we cannot permit a larger aligment early and resort to a lower one later. > > So I prefer keep the assertion checking and implement ROUND_TYPE_ALIGN for > > x86, so that it will set the alignment properly from the beginning. > > > > PoC/untested patch attached. > > The i386 psABI specifies 4 byte alignment for long long. But we want to > use 8 byte alignment if there is no ABI implication and no stack realignment > is needed. Will ROUND_TYPE_ALIGN always align long long to 8 bytes? So what we want here is RTL expansion pad out stack slots if they know to be aligned but not adjust DECL_ALIGN. Not sure if there's already a target hook to pad out variables, if not this may be the way to get what you desire here. I guess the actual reason is crossing of cache-lines?
(In reply to rguenther@suse.de from comment #9) > > The i386 psABI specifies 4 byte alignment for long long. But we want to > > use 8 byte alignment if there is no ABI implication and no stack realignment > > is needed. Will ROUND_TYPE_ALIGN always align long long to 8 bytes? > > So what we want here is RTL expansion pad out stack slots if they know > to be aligned but not adjust DECL_ALIGN. Not sure if there's already > a target hook to pad out variables, if not this may be the way to get > what you desire here. > > I guess the actual reason is crossing of cache-lines? If the incoming stack is 4 byte aligned, we can't align long long to 8 bytes without stack realignment.
On Tue, 2 Jun 2020, hjl.tools at gmail dot com wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95237 > > --- Comment #10 from H.J. Lu <hjl.tools at gmail dot com> --- > (In reply to rguenther@suse.de from comment #9) > > > > The i386 psABI specifies 4 byte alignment for long long. But we want to > > > use 8 byte alignment if there is no ABI implication and no stack realignment > > > is needed. Will ROUND_TYPE_ALIGN always align long long to 8 bytes? > > > > So what we want here is RTL expansion pad out stack slots if they know > > to be aligned but not adjust DECL_ALIGN. Not sure if there's already > > a target hook to pad out variables, if not this may be the way to get > > what you desire here. > > > > I guess the actual reason is crossing of cache-lines? > > If the incoming stack is 4 byte aligned, we can't align long long to > 8 bytes without stack realignment. I understand. But if the stack is 4 byte aligned only and we don't to stack realignment we may not claim long long is 8 bytes aligned.
(In reply to H.J. Lu from comment #10) > (In reply to rguenther@suse.de from comment #9) > > > > The i386 psABI specifies 4 byte alignment for long long. But we want to > > > use 8 byte alignment if there is no ABI implication and no stack realignment > > > is needed. Will ROUND_TYPE_ALIGN always align long long to 8 bytes? > > > > So what we want here is RTL expansion pad out stack slots if they know > > to be aligned but not adjust DECL_ALIGN. Not sure if there's already > > a target hook to pad out variables, if not this may be the way to get > > what you desire here. > > > > I guess the actual reason is crossing of cache-lines? > > If the incoming stack is 4 byte aligned, we can't align long long to > 8 bytes without stack realignment. Only for local variable on stack. We still want to align global/static long long to 8 bytes.
On Tue, 2 Jun 2020, hjl.tools at gmail dot com wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95237 > > --- Comment #12 from H.J. Lu <hjl.tools at gmail dot com> --- > (In reply to H.J. Lu from comment #10) > > (In reply to rguenther@suse.de from comment #9) > > > > > > The i386 psABI specifies 4 byte alignment for long long. But we want to > > > > use 8 byte alignment if there is no ABI implication and no stack realignment > > > > is needed. Will ROUND_TYPE_ALIGN always align long long to 8 bytes? > > > > > > So what we want here is RTL expansion pad out stack slots if they know > > > to be aligned but not adjust DECL_ALIGN. Not sure if there's already > > > a target hook to pad out variables, if not this may be the way to get > > > what you desire here. > > > > > > I guess the actual reason is crossing of cache-lines? > > > > If the incoming stack is 4 byte aligned, we can't align long long to > > 8 bytes without stack realignment. > > Only for local variable on stack. We still want to align global/static long > long to 8 bytes. Sure. And yes, ROUND_TYPE_ALIGN doesn't work for that since it only gets the type and not the decl. We'd need a ROUND_DECL_ALIGN for this. I alternatively suggested that LOCAL_DECL_ALIGNMENT should be used there and I guess we can do that for alignment lowering.
Created attachment 48662 [details] Add target hook to skip alignment check for long long on x86 with -m32 and -mpreferred-stack-boundary=2 Bootstrap and regression tested on x86_64.
On June 2, 2020 6:55:21 PM GMT+02:00, skpgkp2 at gmail dot com <gcc-bugzilla@gcc.gnu.org> wrote: >https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95237 > >--- Comment #14 from Sunil Pandey <skpgkp2 at gmail dot com> --- >Created attachment 48662 [details] > --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=48662&action=edit >Add target hook to skip alignment check for long long on x86 with -m32 >and >-mpreferred-stack-boundary=2 > >Bootstrap and regression tested on x86_64. That doesn't make sense. The alignment is already too large, so the patch will avoid the ICE but not fix the wrong-code issue.
*** Bug 95645 has been marked as a duplicate of this bug. ***
$ cat foo.c long long c(long long x) {} int a() { long long b = c(b); } $ gcc -m32 -mpreferred-stack-boundary=2 -c foo.c during GIMPLE pass: adjust_alignment foo.c: In function ??a??: foo.c:2:5: internal compiler error: in execute, at adjust-alignment.c:74 2 | int a() { long long b = c(b); } | ^ 0x79d34f execute /local/skpandey/gccwork/pr95237/gitlab/gcc.orig/gcc/adjust-alignment.c:74 Please submit a full bug report, with preprocessed source if appropriate. Please include the complete backtrace with any bug report. See <https://gcc.gnu.org/bugs/> for instructions.
Another test, trigger with -Os option. $ cat foo.i int a; long long b() {} int c() { if (b()) a = 1; } $gcc -m32 -mpreferred-stack-boundary=2 -Os -c foo.i during GIMPLE pass: adjust_alignment foo.i: In function ??c??: foo.i:3:5: internal compiler error: in execute, at adjust-alignment.c:74 3 | int c() { | ^ 0x2091411 execute /local/skpandey/gccwork/pr95237/gitlab/gcc.orig/gcc/adjust-alignment.c:74 Please submit a full bug report, with preprocessed source if appropriate. Please include the complete backtrace with any bug report. See <https://gcc.gnu.org/bugs/> for instructions.
*** Bug 95912 has been marked as a duplicate of this bug. ***
This bug has prevented the successful compilation of the local Linux kernel for just over a month now. If I can assist with any testing, please let me know.
The master branch has been updated by H.J. Lu <hjl@gcc.gnu.org>: https://gcc.gnu.org/g:0a9d711df36b42b6494b73a90c7ebf050e904493 commit r11-2259-g0a9d711df36b42b6494b73a90c7ebf050e904493 Author: Sunil K Pandey <skpgkp2@gmail.com> Date: Fri Jul 17 19:42:09 2020 -0700 Add TARGET_LOWER_LOCAL_DECL_ALIGNMENT [PR95237] Default for this hook is NOP. For x86, in 32 bit mode, this hook sets alignment of long long on stack to 32 bits if preferred stack boundary is 32 bits. - This patch prevents lowering of alignment from following macros. LOCAL_ALIGNMENT STACK_SLOT_ALIGNMENT LOCAL_DECL_ALIGNMENT - This patch fixes gcc.target/i386/pr69454-2.c gcc.target/i386/stackalign/longlong-1.c - Regression test on x86-64, no new fail introduced. Tested on x86-64. gcc/c/ChangeLog: PR target/95237 * c-decl.c (finish_decl): Call target hook lower_local_decl_alignment to lower local decl alignment. gcc/ChangeLog: PR target/95237 * config/i386/i386-protos.h (ix86_local_alignment): Add another function parameter may_lower alignment. Default is false. * config/i386/i386.c (ix86_lower_local_decl_alignment): New function. (ix86_local_alignment): Amend ix86_local_alignment to accept another parameter may_lower. If may_lower is true, new align may be lower than incoming alignment. If may_lower is false, new align will be greater or equal to incoming alignment. (TARGET_LOWER_LOCAL_DECL_ALIGNMENT): Define. * doc/tm.texi: Regenerate. * doc/tm.texi.in (TARGET_LOWER_LOCAL_DECL_ALIGNMENT): New hook. * target.def (lower_local_decl_alignment): New hook. gcc/cp/ChangeLog: PR target/95237 * decl.c (cp_finish_decl): Call target hook lower_local_decl_alignment to lower local decl alignment. gcc/testsuite/ChangeLog: PR target/95237 * c-c++-common/pr95237-1.c: New test. * c-c++-common/pr95237-2.c: New test. * c-c++-common/pr95237-3.c: New test. * c-c++-common/pr95237-4.c: New test. * c-c++-common/pr95237-5.c: New test. * c-c++-common/pr95237-6.c: New test. * c-c++-common/pr95237-7.c: New test. * c-c++-common/pr95237-8.c: New test. * c-c++-common/pr95237-9.c: New test.
Fixed.
(In reply to David Binderman from comment #20) > This bug has prevented the successful compilation of the local > Linux kernel for just over a month now. > > If I can assist with any testing, please let me know. Please look into bug report 96192. I also posted a proposed patch for PR96192. It will be great, if you can apply the patch on latest gcc source to see if kernel can build/boot. Also, it would be helpful if you can post a test case, in case of failure.
The master branch has been updated by H.J. Lu <hjl@gcc.gnu.org>: https://gcc.gnu.org/g:748ada0acb6fd746207aaff23a468717eee06555 commit r11-2270-g748ada0acb6fd746207aaff23a468717eee06555 Author: H.J. Lu <hjl.tools@gmail.com> Date: Wed Jul 22 07:33:54 2020 -0700 Limit pr95237-6.c to x86 targets Since c-c++-common/pr95237-6.c is x86 specific, limit it to x86 targets. PR target/95237 * c-c++-common/pr95237-6.c: Only run for x86 targets.
The master branch has been updated by H.J. Lu <hjl@gcc.gnu.org>: https://gcc.gnu.org/g:07c70c29affe7c6d01ab37cc7bc725fd1351f668 commit r11-2315-g07c70c29affe7c6d01ab37cc7bc725fd1351f668 Author: Sunil K Pandey <skpgkp2@gmail.com> Date: Fri Jul 24 14:08:41 2020 -0700 Add testcase to show kernel issue got fixed by pr95237 [PR96192] This test case, extracted from PR 95645, was failing because alignment of local long long variable got lowered from 8 bytes to 4 bytes in adjust alignment pass, which triggered assert failure. This test case passes now because PR 95237 fix only allows lowering of alignment of local variables in the front end. As a result, alignment of local long long variable no longer gets lowered in adjust alignment pass. gcc/testsuite/ChangeLog: PR target/96192 * c-c++-common/pr96192-1.c: New test.