[Bug target/66215] [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66215 Richard Biener rguenth at gcc dot gnu.org changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #34 from Richard Biener rguenth at gcc dot gnu.org --- Fixed.
[Bug target/66215] [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66215 --- Comment #32 from Andreas Krebbel krebbel at gcc dot gnu.org --- Author: krebbel Date: Mon Jun 1 11:28:09 2015 New Revision: 223931 URL: https://gcc.gnu.org/viewcvs?rev=223931root=gccview=rev Log: PR 66215: S390: Fix placement of post-label NOPs with -mhotpatch gcc/ChangeLog: -- 2015-06-01 Dominik Vogt v...@linux.vnet.ibm.com Backport from mainline 2015-05-29 Dominik Vogt v...@linux.vnet.ibm.com PR target/66215 * config/s390/s390.c (s390_reorg): Fix placement of post-label NOPs with -mhotpatch=. gcc/testsuite/ChangeLog: 2015-06-01 Dominik Vogt v...@linux.vnet.ibm.com Backport from mainline 2015-05-29 Dominik Vogt v...@linux.vnet.ibm.com PR target/66215 * gcc.target/s390/hotpatch-1.c: Remove optimization options from dg-options. * gcc.target/s390/hotpatch-10.c: Likewise. * gcc.target/s390/hotpatch-11.c: Likewise. * gcc.target/s390/hotpatch-12.c: Likewise. * gcc.target/s390/hotpatch-17.c: Likewise. * gcc.target/s390/hotpatch-18.c: Likewise. * gcc.target/s390/hotpatch-20.c: Likewise. * gcc.target/s390/hotpatch-21.c: Likewise. * gcc.target/s390/hotpatch-22.c: Likewise. * gcc.target/s390/hotpatch-23.c: Likewise. * gcc.target/s390/hotpatch-24.c: Likewise. * gcc.target/s390/hotpatch-2.c: Likewise. Adjust scan-assembler to check for the exact nops too. * gcc.target/s390/hotpatch-3.c: Likewise. * gcc.target/s390/hotpatch-4.c: Likewise. * gcc.target/s390/hotpatch-5.c: Likewise. * gcc.target/s390/hotpatch-6.c: Likewise. * gcc.target/s390/hotpatch-7.c: Likewise. * gcc.target/s390/hotpatch-8.c: Likewise. * gcc.target/s390/hotpatch-9.c: Likewise. * gcc.target/s390/hotpatch-14.c: Likewise. * gcc.target/s390/hotpatch-15.c: Likewise. * gcc.target/s390/hotpatch-16.c: Likewise. * gcc.target/s390/hotpatch-19.c: Likewise. * gcc.target/s390/hotpatch-25.c: Likewise. Remove scan-assembler-times counting number of .align directives. * gcc.target/s390/hotpatch-13.c: Remove optimization options from dg-options. Remove scan-assembler-times counting number of .align directives. * gcc.target/s390/hotpatch-26.c: New file. * gcc.target/s390/hotpatch-27.c: New file. * gcc.target/s390/hotpatch-28.c: New file. * gcc.target/s390/s390.exp: Run hotpatch-*.c tests as torture tests using -Os -O0 -O1 -O2 -O3 options. Added: branches/gcc-4_9-branch/gcc/testsuite/gcc.target/s390/hotpatch-26.c branches/gcc-4_9-branch/gcc/testsuite/gcc.target/s390/hotpatch-27.c branches/gcc-4_9-branch/gcc/testsuite/gcc.target/s390/hotpatch-28.c Modified: branches/gcc-4_9-branch/gcc/ChangeLog branches/gcc-4_9-branch/gcc/config/s390/s390.c branches/gcc-4_9-branch/gcc/testsuite/ChangeLog branches/gcc-4_9-branch/gcc/testsuite/gcc.target/s390/hotpatch-1.c branches/gcc-4_9-branch/gcc/testsuite/gcc.target/s390/hotpatch-10.c branches/gcc-4_9-branch/gcc/testsuite/gcc.target/s390/hotpatch-11.c branches/gcc-4_9-branch/gcc/testsuite/gcc.target/s390/hotpatch-12.c branches/gcc-4_9-branch/gcc/testsuite/gcc.target/s390/hotpatch-13.c branches/gcc-4_9-branch/gcc/testsuite/gcc.target/s390/hotpatch-14.c branches/gcc-4_9-branch/gcc/testsuite/gcc.target/s390/hotpatch-15.c branches/gcc-4_9-branch/gcc/testsuite/gcc.target/s390/hotpatch-16.c branches/gcc-4_9-branch/gcc/testsuite/gcc.target/s390/hotpatch-17.c branches/gcc-4_9-branch/gcc/testsuite/gcc.target/s390/hotpatch-18.c branches/gcc-4_9-branch/gcc/testsuite/gcc.target/s390/hotpatch-19.c branches/gcc-4_9-branch/gcc/testsuite/gcc.target/s390/hotpatch-2.c branches/gcc-4_9-branch/gcc/testsuite/gcc.target/s390/hotpatch-20.c branches/gcc-4_9-branch/gcc/testsuite/gcc.target/s390/hotpatch-21.c branches/gcc-4_9-branch/gcc/testsuite/gcc.target/s390/hotpatch-22.c branches/gcc-4_9-branch/gcc/testsuite/gcc.target/s390/hotpatch-23.c branches/gcc-4_9-branch/gcc/testsuite/gcc.target/s390/hotpatch-24.c branches/gcc-4_9-branch/gcc/testsuite/gcc.target/s390/hotpatch-25.c branches/gcc-4_9-branch/gcc/testsuite/gcc.target/s390/hotpatch-3.c branches/gcc-4_9-branch/gcc/testsuite/gcc.target/s390/hotpatch-4.c branches/gcc-4_9-branch/gcc/testsuite/gcc.target/s390/hotpatch-5.c branches/gcc-4_9-branch/gcc/testsuite/gcc.target/s390/hotpatch-6.c branches/gcc-4_9-branch/gcc/testsuite/gcc.target/s390/hotpatch-7.c branches/gcc-4_9-branch/gcc/testsuite/gcc.target/s390/hotpatch-8.c
[Bug target/66215] [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66215 --- Comment #29 from Dominik Vogt vogt at linux dot vnet.ibm.com --- As this still seems to work in 4.8, 4.9 and 5.1, is it acceptable to only fix this on the master?
[Bug target/66215] [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66215 --- Comment #33 from Andreas Krebbel krebbel at gcc dot gnu.org --- Author: krebbel Date: Mon Jun 1 11:29:46 2015 New Revision: 223932 URL: https://gcc.gnu.org/viewcvs?rev=223932root=gccview=rev Log: PR 66215: S390: Fix placement of post-label NOPs with -mhotpatch gcc/ChangeLog: -- 2015-06-01 Dominik Vogt v...@linux.vnet.ibm.com Backport from mainline 2015-05-29 Dominik Vogt v...@linux.vnet.ibm.com PR target/66215 * config/s390/s390.c (s390_reorg): Fix placement of post-label NOPs with -mhotpatch=. gcc/testsuite/ChangeLog: 2015-06-01 Dominik Vogt v...@linux.vnet.ibm.com Backport from mainline 2015-05-29 Dominik Vogt v...@linux.vnet.ibm.com PR target/66215 * gcc.target/s390/hotpatch-1.c: Remove optimization options from dg-options. * gcc.target/s390/hotpatch-10.c: Likewise. * gcc.target/s390/hotpatch-11.c: Likewise. * gcc.target/s390/hotpatch-12.c: Likewise. * gcc.target/s390/hotpatch-17.c: Likewise. * gcc.target/s390/hotpatch-18.c: Likewise. * gcc.target/s390/hotpatch-20.c: Likewise. * gcc.target/s390/hotpatch-21.c: Likewise. * gcc.target/s390/hotpatch-22.c: Likewise. * gcc.target/s390/hotpatch-23.c: Likewise. * gcc.target/s390/hotpatch-24.c: Likewise. * gcc.target/s390/hotpatch-2.c: Likewise. Adjust scan-assembler to check for the exact nops too. * gcc.target/s390/hotpatch-3.c: Likewise. * gcc.target/s390/hotpatch-4.c: Likewise. * gcc.target/s390/hotpatch-5.c: Likewise. * gcc.target/s390/hotpatch-6.c: Likewise. * gcc.target/s390/hotpatch-7.c: Likewise. * gcc.target/s390/hotpatch-8.c: Likewise. * gcc.target/s390/hotpatch-9.c: Likewise. * gcc.target/s390/hotpatch-14.c: Likewise. * gcc.target/s390/hotpatch-15.c: Likewise. * gcc.target/s390/hotpatch-16.c: Likewise. * gcc.target/s390/hotpatch-19.c: Likewise. * gcc.target/s390/hotpatch-25.c: Likewise. Remove scan-assembler-times counting number of .align directives. * gcc.target/s390/hotpatch-13.c: Remove optimization options from dg-options. Remove scan-assembler-times counting number of .align directives. * gcc.target/s390/hotpatch-26.c: New file. * gcc.target/s390/hotpatch-27.c: New file. * gcc.target/s390/hotpatch-28.c: New file. * gcc.target/s390/s390.exp: Run hotpatch-*.c tests as torture tests using -Os -O0 -O1 -O2 -O3 options. Added: branches/gcc-5-branch/gcc/testsuite/gcc.target/s390/hotpatch-26.c branches/gcc-5-branch/gcc/testsuite/gcc.target/s390/hotpatch-27.c branches/gcc-5-branch/gcc/testsuite/gcc.target/s390/hotpatch-28.c Modified: branches/gcc-5-branch/gcc/ChangeLog branches/gcc-5-branch/gcc/config/s390/s390.c branches/gcc-5-branch/gcc/testsuite/ChangeLog branches/gcc-5-branch/gcc/testsuite/gcc.target/s390/hotpatch-1.c branches/gcc-5-branch/gcc/testsuite/gcc.target/s390/hotpatch-10.c branches/gcc-5-branch/gcc/testsuite/gcc.target/s390/hotpatch-11.c branches/gcc-5-branch/gcc/testsuite/gcc.target/s390/hotpatch-12.c branches/gcc-5-branch/gcc/testsuite/gcc.target/s390/hotpatch-13.c branches/gcc-5-branch/gcc/testsuite/gcc.target/s390/hotpatch-14.c branches/gcc-5-branch/gcc/testsuite/gcc.target/s390/hotpatch-15.c branches/gcc-5-branch/gcc/testsuite/gcc.target/s390/hotpatch-16.c branches/gcc-5-branch/gcc/testsuite/gcc.target/s390/hotpatch-17.c branches/gcc-5-branch/gcc/testsuite/gcc.target/s390/hotpatch-18.c branches/gcc-5-branch/gcc/testsuite/gcc.target/s390/hotpatch-19.c branches/gcc-5-branch/gcc/testsuite/gcc.target/s390/hotpatch-2.c branches/gcc-5-branch/gcc/testsuite/gcc.target/s390/hotpatch-20.c branches/gcc-5-branch/gcc/testsuite/gcc.target/s390/hotpatch-21.c branches/gcc-5-branch/gcc/testsuite/gcc.target/s390/hotpatch-22.c branches/gcc-5-branch/gcc/testsuite/gcc.target/s390/hotpatch-23.c branches/gcc-5-branch/gcc/testsuite/gcc.target/s390/hotpatch-24.c branches/gcc-5-branch/gcc/testsuite/gcc.target/s390/hotpatch-25.c branches/gcc-5-branch/gcc/testsuite/gcc.target/s390/hotpatch-3.c branches/gcc-5-branch/gcc/testsuite/gcc.target/s390/hotpatch-4.c branches/gcc-5-branch/gcc/testsuite/gcc.target/s390/hotpatch-5.c branches/gcc-5-branch/gcc/testsuite/gcc.target/s390/hotpatch-6.c branches/gcc-5-branch/gcc/testsuite/gcc.target/s390/hotpatch-7.c branches/gcc-5-branch/gcc/testsuite/gcc.target/s390/hotpatch-8.c branches/gcc-5-branch/gcc/testsuite/gcc.target/s390/hotpatch-9.c
[Bug target/66215] [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66215 --- Comment #31 from Andreas Krebbel krebbel at gcc dot gnu.org --- Author: krebbel Date: Mon Jun 1 11:25:56 2015 New Revision: 223930 URL: https://gcc.gnu.org/viewcvs?rev=223930root=gccview=rev Log: PR 66215: S390: Fix placement of post-label NOPs with -mhotpatch gcc/ChangeLog: -- 2015-06-01 Dominik Vogt v...@linux.vnet.ibm.com Backport from mainline 2015-05-29 Dominik Vogt v...@linux.vnet.ibm.com PR target/66215 * config/s390/s390.c (s390_reorg): Fix placement of post-label NOPs with -mhotpatch=. gcc/testsuite/ChangeLog: 2015-06-01 Dominik Vogt v...@linux.vnet.ibm.com Backport from mainline 2015-05-29 Dominik Vogt v...@linux.vnet.ibm.com PR target/66215 * gcc.target/s390/hotpatch-1.c: Remove optimization options from dg-options. * gcc.target/s390/hotpatch-10.c: Likewise. * gcc.target/s390/hotpatch-11.c: Likewise. * gcc.target/s390/hotpatch-12.c: Likewise. * gcc.target/s390/hotpatch-17.c: Likewise. * gcc.target/s390/hotpatch-18.c: Likewise. * gcc.target/s390/hotpatch-20.c: Likewise. * gcc.target/s390/hotpatch-21.c: Likewise. * gcc.target/s390/hotpatch-22.c: Likewise. * gcc.target/s390/hotpatch-23.c: Likewise. * gcc.target/s390/hotpatch-24.c: Likewise. * gcc.target/s390/hotpatch-2.c: Likewise. Adjust scan-assembler to check for the exact nops too. * gcc.target/s390/hotpatch-3.c: Likewise. * gcc.target/s390/hotpatch-4.c: Likewise. * gcc.target/s390/hotpatch-5.c: Likewise. * gcc.target/s390/hotpatch-6.c: Likewise. * gcc.target/s390/hotpatch-7.c: Likewise. * gcc.target/s390/hotpatch-8.c: Likewise. * gcc.target/s390/hotpatch-9.c: Likewise. * gcc.target/s390/hotpatch-14.c: Likewise. * gcc.target/s390/hotpatch-15.c: Likewise. * gcc.target/s390/hotpatch-16.c: Likewise. * gcc.target/s390/hotpatch-19.c: Likewise. * gcc.target/s390/hotpatch-25.c: Likewise. Remove scan-assembler-times counting number of .align directives. * gcc.target/s390/hotpatch-13.c: Remove optimization options from dg-options. Remove scan-assembler-times counting number of .align directives. * gcc.target/s390/hotpatch-26.c: New file. * gcc.target/s390/hotpatch-27.c: New file. * gcc.target/s390/hotpatch-28.c: New file. * gcc.target/s390/s390.exp: Run hotpatch-*.c tests as torture tests using -Os -O0 -O1 -O2 -O3 options. Added: branches/gcc-4_8-branch/gcc/testsuite/gcc.target/s390/hotpatch-26.c branches/gcc-4_8-branch/gcc/testsuite/gcc.target/s390/hotpatch-27.c branches/gcc-4_8-branch/gcc/testsuite/gcc.target/s390/hotpatch-28.c Modified: branches/gcc-4_8-branch/gcc/ChangeLog branches/gcc-4_8-branch/gcc/config/s390/s390.c branches/gcc-4_8-branch/gcc/testsuite/ChangeLog branches/gcc-4_8-branch/gcc/testsuite/gcc.target/s390/hotpatch-1.c branches/gcc-4_8-branch/gcc/testsuite/gcc.target/s390/hotpatch-10.c branches/gcc-4_8-branch/gcc/testsuite/gcc.target/s390/hotpatch-11.c branches/gcc-4_8-branch/gcc/testsuite/gcc.target/s390/hotpatch-12.c branches/gcc-4_8-branch/gcc/testsuite/gcc.target/s390/hotpatch-13.c branches/gcc-4_8-branch/gcc/testsuite/gcc.target/s390/hotpatch-14.c branches/gcc-4_8-branch/gcc/testsuite/gcc.target/s390/hotpatch-15.c branches/gcc-4_8-branch/gcc/testsuite/gcc.target/s390/hotpatch-16.c branches/gcc-4_8-branch/gcc/testsuite/gcc.target/s390/hotpatch-17.c branches/gcc-4_8-branch/gcc/testsuite/gcc.target/s390/hotpatch-18.c branches/gcc-4_8-branch/gcc/testsuite/gcc.target/s390/hotpatch-19.c branches/gcc-4_8-branch/gcc/testsuite/gcc.target/s390/hotpatch-2.c branches/gcc-4_8-branch/gcc/testsuite/gcc.target/s390/hotpatch-20.c branches/gcc-4_8-branch/gcc/testsuite/gcc.target/s390/hotpatch-21.c branches/gcc-4_8-branch/gcc/testsuite/gcc.target/s390/hotpatch-22.c branches/gcc-4_8-branch/gcc/testsuite/gcc.target/s390/hotpatch-23.c branches/gcc-4_8-branch/gcc/testsuite/gcc.target/s390/hotpatch-24.c branches/gcc-4_8-branch/gcc/testsuite/gcc.target/s390/hotpatch-25.c branches/gcc-4_8-branch/gcc/testsuite/gcc.target/s390/hotpatch-3.c branches/gcc-4_8-branch/gcc/testsuite/gcc.target/s390/hotpatch-4.c branches/gcc-4_8-branch/gcc/testsuite/gcc.target/s390/hotpatch-5.c branches/gcc-4_8-branch/gcc/testsuite/gcc.target/s390/hotpatch-6.c branches/gcc-4_8-branch/gcc/testsuite/gcc.target/s390/hotpatch-7.c branches/gcc-4_8-branch/gcc/testsuite/gcc.target/s390/hotpatch-8.c
[Bug target/66215] [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66215 --- Comment #30 from Jakub Jelinek jakub at gcc dot gnu.org --- IMHO it is better to keep the tests in sync between branches.
[Bug target/66215] [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66215 --- Comment #24 from Dominik Vogt vogt at linux dot vnet.ibm.com --- Okay, I've made a new patch that fixes the problem with only the hotpatch tests running in the s390 test suite. This change passes all tests on 64 and 31 bit. After this change we have done some cleanup work: * Added a comment to the if (active_insn_p (insn) ...), * removed the now redundant -fno-dwarf2-cfi-asm from the torture tests, * removed the changes from the hopatch-compile*.c tests as they are not going through the torture tests anyway, * updated the ChangeLog. The new set of changes is being tested right now, but just with the upstream code, without bootstrapping and without the Ada testsuite. This appears to be acceptable to me given that the latest changes don't affect the code.
[Bug target/66215] [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66215 --- Comment #26 from Dominik Vogt vogt at linux dot vnet.ibm.com --- The patch for upstream gcc is available here: https://gcc.gnu.org/ml/gcc-patches/2015-05/msg02739.html
[Bug target/66215] [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66215 --- Comment #25 from Andreas Krebbel krebbel at gcc dot gnu.org --- The patch looks good to me. Please post it on the mailing list and I'll commit it. Jakub, thanks for your valuable comments! Does Dominik's last patch address your concerns?
[Bug target/66215] [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66215 --- Comment #28 from Jakub Jelinek jakub at gcc dot gnu.org --- BTW, the hotpatch-19.c, hotpatch-20.c and hotpatch-compile-15.c testcases look wrong, always_inline attribute shouldn't be used on functions not declared inline.
[Bug target/66215] [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66215 --- Comment #27 from Andreas Krebbel krebbel at gcc dot gnu.org --- Author: krebbel Date: Fri May 29 14:26:56 2015 New Revision: 223867 URL: https://gcc.gnu.org/viewcvs?rev=223867root=gccview=rev Log: PR 66215: S390: Fix placement of post-label NOPs with -mhotpatch gcc/ChangeLog: PR target/66215 * config/s390/s390.c (s390_reorg): Fix placement of post-label NOPs with -mhotpatch=. gcc/testsuite/ChangeLog: PR target/66215 * gcc.target/s390/hotpatch-1.c: Remove optimization options from dg-options. * gcc.target/s390/hotpatch-10.c: Likewise. * gcc.target/s390/hotpatch-11.c: Likewise. * gcc.target/s390/hotpatch-12.c: Likewise. * gcc.target/s390/hotpatch-17.c: Likewise. * gcc.target/s390/hotpatch-18.c: Likewise. * gcc.target/s390/hotpatch-20.c: Likewise. * gcc.target/s390/hotpatch-21.c: Likewise. * gcc.target/s390/hotpatch-22.c: Likewise. * gcc.target/s390/hotpatch-23.c: Likewise. * gcc.target/s390/hotpatch-24.c: Likewise. * gcc.target/s390/hotpatch-2.c: Likewise. Adjust scan-assembler to check for the exact nops too. * gcc.target/s390/hotpatch-3.c: Likewise. * gcc.target/s390/hotpatch-4.c: Likewise. * gcc.target/s390/hotpatch-5.c: Likewise. * gcc.target/s390/hotpatch-6.c: Likewise. * gcc.target/s390/hotpatch-7.c: Likewise. * gcc.target/s390/hotpatch-8.c: Likewise. * gcc.target/s390/hotpatch-9.c: Likewise. * gcc.target/s390/hotpatch-14.c: Likewise. * gcc.target/s390/hotpatch-15.c: Likewise. * gcc.target/s390/hotpatch-16.c: Likewise. * gcc.target/s390/hotpatch-19.c: Likewise. * gcc.target/s390/hotpatch-25.c: Likewise. Remove scan-assembler-times counting number of .align directives. * gcc.target/s390/hotpatch-13.c: Remove optimization options from dg-options. Remove scan-assembler-times counting number of .align directives. * gcc.target/s390/hotpatch-26.c: New file. * gcc.target/s390/hotpatch-27.c: New file. * gcc.target/s390/hotpatch-28.c: New file. * gcc.target/s390/s390.exp: Run hotpatch-*.c tests as torture tests using -Os -O0 -O1 -O2 -O3 options. Added: trunk/gcc/testsuite/gcc.target/s390/hotpatch-26.c trunk/gcc/testsuite/gcc.target/s390/hotpatch-27.c trunk/gcc/testsuite/gcc.target/s390/hotpatch-28.c Modified: trunk/gcc/ChangeLog trunk/gcc/config/s390/s390.c trunk/gcc/testsuite/ChangeLog trunk/gcc/testsuite/gcc.target/s390/hotpatch-1.c trunk/gcc/testsuite/gcc.target/s390/hotpatch-10.c trunk/gcc/testsuite/gcc.target/s390/hotpatch-11.c trunk/gcc/testsuite/gcc.target/s390/hotpatch-12.c trunk/gcc/testsuite/gcc.target/s390/hotpatch-13.c trunk/gcc/testsuite/gcc.target/s390/hotpatch-14.c trunk/gcc/testsuite/gcc.target/s390/hotpatch-15.c trunk/gcc/testsuite/gcc.target/s390/hotpatch-16.c trunk/gcc/testsuite/gcc.target/s390/hotpatch-17.c trunk/gcc/testsuite/gcc.target/s390/hotpatch-18.c trunk/gcc/testsuite/gcc.target/s390/hotpatch-19.c trunk/gcc/testsuite/gcc.target/s390/hotpatch-2.c trunk/gcc/testsuite/gcc.target/s390/hotpatch-20.c trunk/gcc/testsuite/gcc.target/s390/hotpatch-21.c trunk/gcc/testsuite/gcc.target/s390/hotpatch-22.c trunk/gcc/testsuite/gcc.target/s390/hotpatch-23.c trunk/gcc/testsuite/gcc.target/s390/hotpatch-24.c trunk/gcc/testsuite/gcc.target/s390/hotpatch-25.c trunk/gcc/testsuite/gcc.target/s390/hotpatch-3.c trunk/gcc/testsuite/gcc.target/s390/hotpatch-4.c trunk/gcc/testsuite/gcc.target/s390/hotpatch-5.c trunk/gcc/testsuite/gcc.target/s390/hotpatch-6.c trunk/gcc/testsuite/gcc.target/s390/hotpatch-7.c trunk/gcc/testsuite/gcc.target/s390/hotpatch-8.c trunk/gcc/testsuite/gcc.target/s390/hotpatch-9.c trunk/gcc/testsuite/gcc.target/s390/s390.exp
[Bug target/66215] [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66215 --- Comment #21 from Jakub Jelinek jakub at gcc dot gnu.org --- In case somebody forces any of the torture options into the default through --target_board=unix/WHATEVER that will result in testing some option combination multiple times and -O0 never. If you want to avoid duplication, you can just set hotpatch_tests $srcdir/$subdir/hotpatch-\[0-9\]*.c and use $hotpatch_tests in both cases (untested).
[Bug target/66215] [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66215 --- Comment #23 from Jakub Jelinek jakub at gcc dot gnu.org --- (In reply to Dominik Vogt from comment #22) Created attachment 35628 [details] Experimental fix 5 Version 5 with the suggested changes and a new test case. Hopefully the last version before submitting the changes on gcc-patches. +dg-runtest [lsort [prune [glob -nocomplain $srcdir/$subdir/hotpatch-*.\[cS\]] \ +$hotpatch_tests]] $DEFAULT_CFLAGS is wrong, that will run all hotpatch-*.[cS] tests that are not hotpatch-[0-9].[cS] (i.e. just hotpatch-compile*, but not all the other s390 tests). You want (IMHO): +dg-runtest [lsort [prune [glob -nocomplain $srcdir/$subdir/*.\[cS\]] \ +$hotpatch_tests]] $DEFAULT_CFLAGS instead.
[Bug target/66215] [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66215 Dominik Vogt vogt at linux dot vnet.ibm.com changed: What|Removed |Added Attachment #35599|0 |1 is obsolete|| --- Comment #22 from Dominik Vogt vogt at linux dot vnet.ibm.com --- Created attachment 35628 -- https://gcc.gnu.org/bugzilla/attachment.cgi?id=35628action=edit Experimental fix 5 Version 5 with the suggested changes and a new test case. Hopefully the last version before submitting the changes on gcc-patches.
[Bug target/66215] [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66215 --- Comment #18 from Miroslav Benes mbenes at suse dot cz --- I confirm that v4 of the proposed patch works for me. Tested on simple userspace program similar to the one in the bug description and on the kernel module where I stumbled upon the bug originally. Thanks a lot for the fix, Miroslav
[Bug target/66215] [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66215 --- Comment #19 from Jakub Jelinek jakub at gcc dot gnu.org --- I'm still missing (untested): -dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/*.\[cS\]]] \ +dg-runtest [lsort [prune [glob -nocomplain $srcdir/$subdir/*.\[cS\]] \ + $srcdir/$subdir/hotpatch-\[0-9\]*.c]] \ $DEFAULT_CFLAGS or similar change in the s390.exp, otherwise you run all the hotpatch-[0-9]*.c tests once outside of the torture (with the implicit -O0) and many times in the torture mode (where it includes -O0 too).
[Bug target/66215] [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66215 --- Comment #20 from Dominik Vogt vogt at linux dot vnet.ibm.com --- I'll remove -O0 from the list of torture test options so that the list of hotpatch torture tests is only defined in one place.
[Bug target/66215] [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66215 --- Comment #7 from Dominik Vogt vogt at linux dot vnet.ibm.com --- Created attachment 35595 -- https://gcc.gnu.org/bugzilla/attachment.cgi?id=35595action=edit Experimental fix Can you please check whether this patch works for you? The patch also contains improved test cases that should hopefully detect anything like this happening again.
[Bug target/66215] [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66215 --- Comment #8 from Jakub Jelinek jakub at gcc dot gnu.org --- Doesn't this disable all the other s390 tests? Supposedly you need to prune them from the non-torture lists, see e.g. g++.dg/dg.exp on how it prunes tests.
[Bug target/66215] [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66215 --- Comment #11 from Dominik Vogt vogt at linux dot vnet.ibm.com --- (In reply to Jakub Jelinek from comment #10) As for the hotpatch testcases, have you tested them also with -fno-dwarf2-cfi-asm (as they seem to contain .cfi directives in the regexps)? Not yet. I'll add another test case for that. The regexp is supposed to skip all lines with .cfi_... or labels matching .L*:, so -fno-dwarf2-cfi-asm should not matter, but testing is besser than guessing. And for the main patch, the change will revert the order of 2, 4 and 6 byte nops post-label The order is the same because the patch never updates insn. The order would be reversed if I had written insn = emit_before_insn (...), keeping the assignment. The tests indirectly verify that the order is correct. For example, hotpatch-5.c tests that with -mhotpatch=...,4 the first instruction after the comment is a 6-byte-NOP: hp1: # post-label NOPs for hotpatch (4 halfwords) .LFB0: .cfi_startproc brcl0, 0 nopr%r7 stmg%r11,%r15,88(%r15) is that ok? No, that would be incorrect. It is important that the largest NOP comes first
[Bug target/66215] [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66215 --- Comment #9 from Dominik Vogt vogt at linux dot vnet.ibm.com --- Yeah, I accidentally left the other test cases commented out. I'll clean that up in the next version of the patch. That doesn't have an impact on the correctness of the code change, though.
[Bug target/66215] [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66215 --- Comment #10 from Jakub Jelinek jakub at gcc dot gnu.org --- As for the hotpatch testcases, have you tested them also with -fno-dwarf2-cfi-asm (as they seem to contain .cfi directives in the regexps)? And for the main patch, the change will revert the order of 2, 4 and 6 byte nops post-label, is that ok? Say if some hotpatching code were to verify it overwrites the nops and required the old order.
[Bug target/66215] [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66215 --- Comment #12 from Dominik Vogt vogt at linux dot vnet.ibm.com --- Created attachment 35597 -- https://gcc.gnu.org/bugzilla/attachment.cgi?id=35597action=edit Experimental fix 2 Version 2 of the patch with the suggested changes and test fixes. Note: Functions without active insns will get the pre-label NOPs, the comment # post-label NOPs but no NOPs after that. Not sure whether it's worth the effort to clean this up. Please test if this fixes the observed problem (it does for me).
[Bug target/66215] [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66215 --- Comment #14 from Dominik Vogt vogt at linux dot vnet.ibm.com --- Is it possible that a function has no insns at all?
[Bug target/66215] [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66215 Dominik Vogt vogt at linux dot vnet.ibm.com changed: What|Removed |Added Attachment #35597|0 |1 is obsolete|| --- Comment #15 from Dominik Vogt vogt at linux dot vnet.ibm.com --- Created attachment 35598 -- https://gcc.gnu.org/bugzilla/attachment.cgi?id=35598action=edit Experimental fix 3 Version 3 of the patch with the suggested change; also fixes th egcc_unreachable test.
[Bug target/66215] [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66215
--- Comment #13 from Jakub Jelinek jakub at gcc dot gnu.org ---
The only function without active insns I'm aware of is:
void
foo (void)
{
__builtin_unreachable ();
}
which only contains notes and a BARRIER, nothing else (at least on s390x).
It would be desirable to handle even that, otherwise a hotpatcher would
overwrite random bytes from some other function.
One option would be to avoid using next_active_insn:
for (insn = get_insns (); insn; insn = NEXT_INSN (insn))
if (active_insn_p (insn) || BARRIER_P (insn))
break;
Adding the nops after a BARRIER is generally undesirable I'd say.
[Bug target/66215] [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66215 Dominik Vogt vogt at linux dot vnet.ibm.com changed: What|Removed |Added Attachment #35598|0 |1 is obsolete|| --- Comment #17 from Dominik Vogt vogt at linux dot vnet.ibm.com --- Created attachment 35599 -- https://gcc.gnu.org/bugzilla/attachment.cgi?id=35599action=edit Experimental fix 4 Version 4 of the patch with the suggested change and a test case for it.
[Bug target/66215] [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66215
--- Comment #16 from Jakub Jelinek jakub at gcc dot gnu.org ---
The function I've mentioned has:
(note 1 0 3 NOTE_INSN_DELETED)
(note 3 1 12 [bb 2] NOTE_INSN_BASIC_BLOCK)
(note 12 3 2 NOTE_INSN_PROLOGUE_END)
(note 2 12 5 NOTE_INSN_FUNCTION_BEG)
(barrier 5 2 8)
(note 8 5 0 NOTE_INSN_DELETED)
(at least in my somewhat older trunk version).
I can't think of anything simpler than that, because if the function isn't
noreturn, it shall have at least a return insn at the end, and if it is
noreturn, but doesn't have __builtin_unreachable () in it, there should be
either an endless loop (so some jump_insn), or noreturn call (call_insn).
Though, thinking about it, if you only have notes at the beginning and then
some CODE_LABEL, adding the nops after the CODE_LABEL would result in running
the nops not just when entering the function the first time, but more often.
Thus, I'd suggest maybe:
for (insn = get_insns (); insn; insn = NEXT_INSN (insn))
if (active_insn_p (insn)
|| BARRIER_P (insn)
|| LABEL_P (insn))
break;
Thus, put the nops before first of active insns, barrier (that one for the
__builtin_unreachable () case) and the last one for e.g.:
void
baz (volatile unsigned int *i)
{
for (;;)
(*i)++;
}
where I guess hotpatching wouldn't really like to see this as running the
function many times.
[Bug target/66215] [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66215
--- Comment #6 from Jakub Jelinek jakub at gcc dot gnu.org ---
No, IMHO you can have many debug insns after that and before first real insn.
I'd go for something like:
rtx_insn *insn = get_insns ();
if (!active_insn_p (insn))
insn = next_active_insn (insn);
and insert before, rather than after (otherwise you don't handle the
hypothetical case of an active insn being the first one).
That would require rewriting the nop insertion code after it, because you want
to insert the 6 byte nops first. Or just gcc_assert the first insn is not
active, or if the first insn is active, emit a NOTE_INSN_DELETED note before
that
first active insn, emit the nops after that note and perhaps kill the note at
the end.
Please test
void foo (void) { __builtin_unreachable (); }
actually generates any active insns.
[Bug target/66215] [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66215 --- Comment #5 from Dominik Vogt vogt at linux dot vnet.ibm.com --- Wouldn't the correct and easy to identify place be right after the first NOTE_INSN_BASIC_BLOCK?
[Bug target/66215] [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66215 Richard Biener rguenth at gcc dot gnu.org changed: What|Removed |Added Target Milestone|--- |4.8.5
[Bug target/66215] [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66215
--- Comment #1 from Martin Liška marxin at gcc dot gnu.org ---
Following testcase is wrongly compiled event with -O2 optimization level.
$ cat o2-test-case.c
static int a;
int t(int tt)
{
switch (tt)
{
case 1: return a;
}
return 0;
}
$ ./xgcc -B. -mhotpatch=2,3 -O2 -fno-inline -o /tmp/test.s -S o2-test-case.c
$ head -n20 /tmp/test.s
.file o2-test-case.c
.text
.align 8
.globl t
.type t, @function
nopr%r7 # pre-label NOPs for hotpatch (2 halfwords)
nopr%r7
# alignment for hotpatch
.align 8
t:
marxin@marxinbox:~/Programming/gcc2/objdir/gcc head -n20 /tmp/test.s
.file o2-test-case.c
.text
.align 8
.globl t
.type t, @function
nopr%r7 # pre-label NOPs for hotpatch (2 halfwords)
nopr%r7
# alignment for hotpatch
.align 8
t:
# post-label NOPs for hotpatch (3 halfwords)
.LFB0:
st %r14,56(%r15)
nop 0
nopr%r7
.LCFI0:
l %r4,56(%r15)
lhi %r2,0
l %r14,56(%r15)
.LCFI1:
[Bug target/66215] [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66215
--- Comment #2 from Martin Liška marxin at gcc dot gnu.org ---
(In reply to Martin Liška from comment #1)
Following testcase is wrongly compiled event with -O2 optimization level.
$ cat o2-test-case.c
static int a;
int t(int tt)
{
switch (tt)
{
case 1: return a;
}
return 0;
}
$ ./xgcc -B. -mhotpatch=2,3 -O2 -fno-inline -o /tmp/test.s -S o2-test-case.c
$ head -n20 /tmp/test.s
.file o2-test-case.c
.text
.align 8
.globl t
.type t, @function
nopr%r7 # pre-label NOPs for hotpatch (2 halfwords)
nopr%r7
# alignment for hotpatch
.align 8
t:
marxin@marxinbox:~/Programming/gcc2/objdir/gcc head -n20 /tmp/test.s
.file o2-test-case.c
.text
.align 8
.globl t
.type t, @function
nopr%r7 # pre-label NOPs for hotpatch (2 halfwords)
nopr%r7
# alignment for hotpatch
.align 8
t:
# post-label NOPs for hotpatch (3 halfwords)
.LFB0:
st %r14,56(%r15)
nop 0
nopr%r7
.LCFI0:
l %r4,56(%r15)
lhi %r2,0
l %r14,56(%r15)
.LCFI1:
Sorry, this testcase is invalid as I modified the compiler.
Martin
[Bug target/66215] [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66215 Andreas Krebbel krebbel at gcc dot gnu.org changed: What|Removed |Added Status|UNCONFIRMED |NEW Last reconfirmed||2015-05-20 Ever confirmed|0 |1
[Bug target/66215] [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66215 Jakub Jelinek jakub at gcc dot gnu.org changed: What|Removed |Added CC||jakub at gcc dot gnu.org --- Comment #3 from Jakub Jelinek jakub at gcc dot gnu.org --- IMHO the nops should go immediately before the first real instruction in the function. The point of not emitting it earlier is so that the nops are already covered by debug and unwind info. So, we certainly should skip all NOTEs at the start of the function, and perhaps selected other insns (dunno if e.g. UNSPECV_MAIN_POOL insn emits any code or not). NOTE_INSN_FUNCTION_BEG is not a good place, as it can be preceeded by various prologue instructions, and the nops really have to go before all real instructions (that emit some bytes into the function text section).
[Bug target/66215] [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66215 --- Comment #4 from Andreas Krebbel krebbel at gcc dot gnu.org --- (In reply to Jakub Jelinek from comment #3) IMHO the nops should go immediately before the first real instruction in the function. The point of not emitting it earlier is so that the nops are already covered by debug and unwind info. So, we certainly should skip all NOTEs at the start of the function, and perhaps selected other insns (dunno if e.g. UNSPECV_MAIN_POOL insn emits any code or not). NOTE_INSN_FUNCTION_BEG is not a good place, as it can be preceeded by various prologue instructions, and the nops really have to go before all real instructions (that emit some bytes into the function text section). UNSPECV_MAIN_POOL does not itself emit code. However, it is used as an anchor to emit insns right after it so we should not skip it here I think. We will try with skipping all the NOTEs as you suggested and we definitely should run all the hotpatch testcases at different optimization levels. Thanks!
