| Message ID | alpine.LFD.2.21.2101081133151.1637534@eddie.linux-mips.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | VAX/testsuite: Remove notsi comparison elimination regressions | expand |
On 1/8/21 5:49 AM, Maciej W. Rozycki wrote: > Remove fallout from commit 0bd675183d94 ("match.pd: Add ~(X - Y) -> ~X > + Y simplification [PR96685]") and paper over the regression caused as > it is not the matter of the test cases affected. > > Previously assembly like this: > > .text > .align 1 > .globl eq_notsi > .type eq_notsi, @function > eq_notsi: > .word 0 # 35 [c=0] procedure_entry_mask > subl2 $4,%sp # 46 [c=32] *addsi3 > mcoml 4(%ap),%r0 # 32 [c=16] *one_cmplsi2_ccz > jeql .L1 # 34 [c=26] *branch_ccz > addl2 $2,%r0 # 31 [c=32] *addsi3 > .L1: > ret # 40 [c=0] return > .size eq_notsi, .-eq_notsi > > was produced. Now this: > > .text > .align 1 > .globl eq_notsi > .type eq_notsi, @function > eq_notsi: > .word 0 # 36 [c=0] procedure_entry_mask > subl2 $4,%sp # 48 [c=32] *addsi3 > movl 4(%ap),%r0 # 33 [c=16] *movsi_2 > cmpl %r0,$-1 # 34 [c=8] *cmpsi_ccz/1 > jeql .L3 # 35 [c=26] *branch_ccz > subl3 %r0,$1,%r0 # 32 [c=32] *subsi3/1 > ret # 27 [c=0] return > .L3: > clrl %r0 # 31 [c=2] *movsi_2 > ret # 41 [c=0] return > .size eq_notsi, .-eq_notsi > > is, which cannot work with post-reload comparison elimination, due to > the comparison against -1 rather than 0. > > Use subtraction from a constant then rather than addition as the former > operation is not transformed, removing these regressions: > > FAIL: gcc.target/vax/cmpelim-eq-notsi.c -O1 scan-rtl-dump-times cmpelim "deleting insn with uid" 1 > FAIL: gcc.target/vax/cmpelim-eq-notsi.c -O1 scan-assembler-not \t(bit|cmpz?|tst). > FAIL: gcc.target/vax/cmpelim-eq-notsi.c -O1 scan-assembler one_cmplsi[^ ]*_ccz(/[0-9]+)?\n > FAIL: gcc.target/vax/cmpelim-lt-notsi.c -O1 scan-rtl-dump-times cmpelim "deleting insn with uid" 1 > FAIL: gcc.target/vax/cmpelim-lt-notsi.c -O1 scan-assembler-not \t(bit|cmpz?|tst). > FAIL: gcc.target/vax/cmpelim-lt-notsi.c -O1 scan-assembler one_cmplsi[^ ]*_ccn(/[0-9]+)?\n > > and likewise across some of the other the optimization levels verified. > > The LE variant appears unaffected as the new transformation produces > slightly different although still suboptimal code: > > .text > .align 1 > .globl le_notsi > .type le_notsi, @function > le_notsi: > .word 0 # 27 [c=0] procedure_entry_mask > subl2 $4,%sp # 34 [c=32] *addsi3 > movl 4(%ap),%r1 # 23 [c=16] *movsi_2 > mcoml %r1,%r0 # 24 [c=8] *one_cmplsi2_ccnz > jleq .L1 # 26 [c=26] *branch_ccnz > subl3 %r1,$1,%r0 # 22 [c=32] *subsi3/1 > .L1: > ret # 32 [c=0] return > .size le_notsi, .-le_notsi > > but update the test case too, for consistency with the other two. > > gcc/testsuite/ > * gcc.target/vax/cmpelim-eq-notsi.c: Use subtraction from a > constant then rather than addition. > * gcc.target/vax/cmpelim-le-notsi.c: Likewise. > * gcc.target/vax/cmpelim-lt-notsi.c: Likewise. OK jeff
On Fri, 8 Jan 2021, Jeff Law wrote: > > gcc/testsuite/ > > * gcc.target/vax/cmpelim-eq-notsi.c: Use subtraction from a > > constant then rather than addition. > > * gcc.target/vax/cmpelim-le-notsi.c: Likewise. > > * gcc.target/vax/cmpelim-lt-notsi.c: Likewise. > OK Thank you for your review. I have applied this change now and the remaining ones you have approved. I'll be watching out for any further concerns, but otherwise I consider VAX backend development complete for this release cycle. Also I have now scheduled full regression testing of the `vax-netbsdelf' target with the timeout extended to 7200 seconds. Hopefully this will let all cases complete that do not infinitely loop. I can post results to gcc-testresults if that would be desired (is there a dedicated format for that mailing list?), and overall they need to be triaged before anything can be decided about what to do next. I have seen some failures coming from individual test cases' assumption of the floating-point format being IEEE 754, so at least these can be easily excluded, or variants for the alternative format provided, as applicable. Maciej
On 1/10/21 7:45 AM, Maciej W. Rozycki wrote: > On Fri, 8 Jan 2021, Jeff Law wrote: > >>> gcc/testsuite/ >>> * gcc.target/vax/cmpelim-eq-notsi.c: Use subtraction from a >>> constant then rather than addition. >>> * gcc.target/vax/cmpelim-le-notsi.c: Likewise. >>> * gcc.target/vax/cmpelim-lt-notsi.c: Likewise. >> OK > Thank you for your review. I have applied this change now and the > remaining ones you have approved. I'll be watching out for any further > concerns, but otherwise I consider VAX backend development complete for > this release cycle. Sounds good. > > Also I have now scheduled full regression testing of the `vax-netbsdelf' > target with the timeout extended to 7200 seconds. Hopefully this will let > all cases complete that do not infinitely loop. I can post results to > gcc-testresults if that would be desired (is there a dedicated format for > that mailing list?), and overall they need to be triaged before anything > can be decided about what to do next. > > I have seen some failures coming from individual test cases' assumption > of the floating-point format being IEEE 754, so at least these can be > easily excluded, or variants for the alternative format provided, as > applicable. I think most are posting the stdout from the check run. So we don't generally get all the pass/xfail messages, but we do get fail/xpass messages. They don't need to be triaged or anything. jeff
Index: gcc/gcc/testsuite/gcc.target/vax/cmpelim-eq-notsi.c =================================================================== --- gcc.orig/gcc/testsuite/gcc.target/vax/cmpelim-eq-notsi.c +++ gcc/gcc/testsuite/gcc.target/vax/cmpelim-eq-notsi.c @@ -11,14 +11,14 @@ eq_notsi (int_t x) if (x == 0) return x; else - return x + 2; + return 2 - x; } /* Expect assembly like: mcoml 4(%ap),%r0 # 32 [c=16] *one_cmplsi2_ccz jeql .L1 # 34 [c=26] *branch_ccz - addl2 $2,%r0 # 31 [c=32] *addsi3 + subl3 %r0,$2,%r0 # 31 [c=32] *subsi3/1 .L1: */ Index: gcc/gcc/testsuite/gcc.target/vax/cmpelim-le-notsi.c =================================================================== --- gcc.orig/gcc/testsuite/gcc.target/vax/cmpelim-le-notsi.c +++ gcc/gcc/testsuite/gcc.target/vax/cmpelim-le-notsi.c @@ -11,14 +11,14 @@ le_notsi (int_t x) if (x <= 0) return x; else - return x + 2; + return 2 - x; } /* Expect assembly like: mcoml 4(%ap),%r0 # 28 [c=16] *one_cmplsi2_ccnz jleq .L1 # 30 [c=26] *branch_ccnz - addl2 $2,%r0 # 27 [c=32] *addsi3 + subl3 %r0,$2,%r0 # 27 [c=32] *subsi3/1 .L1: */ Index: gcc/gcc/testsuite/gcc.target/vax/cmpelim-lt-notsi.c =================================================================== --- gcc.orig/gcc/testsuite/gcc.target/vax/cmpelim-lt-notsi.c +++ gcc/gcc/testsuite/gcc.target/vax/cmpelim-lt-notsi.c @@ -11,14 +11,14 @@ lt_notsi (int_t x) if (x < 0) return x; else - return x + 2; + return 2 - x; } /* Expect assembly like: mcoml 4(%ap),%r0 # 28 [c=16] *one_cmplsi2_ccn jlss .L1 # 30 [c=26] *branch_ccn - addl2 $2,%r0 # 27 [c=32] *addsi3 + subl3 %r0,$2,%r0 # 27 [c=32] *subsi3/1 .L1: */
Remove fallout from commit 0bd675183d94 ("match.pd: Add ~(X - Y) -> ~X + Y simplification [PR96685]") and paper over the regression caused as it is not the matter of the test cases affected. Previously assembly like this: .text .align 1 .globl eq_notsi .type eq_notsi, @function eq_notsi: .word 0 # 35 [c=0] procedure_entry_mask subl2 $4,%sp # 46 [c=32] *addsi3 mcoml 4(%ap),%r0 # 32 [c=16] *one_cmplsi2_ccz jeql .L1 # 34 [c=26] *branch_ccz addl2 $2,%r0 # 31 [c=32] *addsi3 .L1: ret # 40 [c=0] return .size eq_notsi, .-eq_notsi was produced. Now this: .text .align 1 .globl eq_notsi .type eq_notsi, @function eq_notsi: .word 0 # 36 [c=0] procedure_entry_mask subl2 $4,%sp # 48 [c=32] *addsi3 movl 4(%ap),%r0 # 33 [c=16] *movsi_2 cmpl %r0,$-1 # 34 [c=8] *cmpsi_ccz/1 jeql .L3 # 35 [c=26] *branch_ccz subl3 %r0,$1,%r0 # 32 [c=32] *subsi3/1 ret # 27 [c=0] return .L3: clrl %r0 # 31 [c=2] *movsi_2 ret # 41 [c=0] return .size eq_notsi, .-eq_notsi is, which cannot work with post-reload comparison elimination, due to the comparison against -1 rather than 0. Use subtraction from a constant then rather than addition as the former operation is not transformed, removing these regressions: FAIL: gcc.target/vax/cmpelim-eq-notsi.c -O1 scan-rtl-dump-times cmpelim "deleting insn with uid" 1 FAIL: gcc.target/vax/cmpelim-eq-notsi.c -O1 scan-assembler-not \t(bit|cmpz?|tst). FAIL: gcc.target/vax/cmpelim-eq-notsi.c -O1 scan-assembler one_cmplsi[^ ]*_ccz(/[0-9]+)?\n FAIL: gcc.target/vax/cmpelim-lt-notsi.c -O1 scan-rtl-dump-times cmpelim "deleting insn with uid" 1 FAIL: gcc.target/vax/cmpelim-lt-notsi.c -O1 scan-assembler-not \t(bit|cmpz?|tst). FAIL: gcc.target/vax/cmpelim-lt-notsi.c -O1 scan-assembler one_cmplsi[^ ]*_ccn(/[0-9]+)?\n and likewise across some of the other the optimization levels verified. The LE variant appears unaffected as the new transformation produces slightly different although still suboptimal code: .text .align 1 .globl le_notsi .type le_notsi, @function le_notsi: .word 0 # 27 [c=0] procedure_entry_mask subl2 $4,%sp # 34 [c=32] *addsi3 movl 4(%ap),%r1 # 23 [c=16] *movsi_2 mcoml %r1,%r0 # 24 [c=8] *one_cmplsi2_ccnz jleq .L1 # 26 [c=26] *branch_ccnz subl3 %r1,$1,%r0 # 22 [c=32] *subsi3/1 .L1: ret # 32 [c=0] return .size le_notsi, .-le_notsi but update the test case too, for consistency with the other two. gcc/testsuite/ * gcc.target/vax/cmpelim-eq-notsi.c: Use subtraction from a constant then rather than addition. * gcc.target/vax/cmpelim-le-notsi.c: Likewise. * gcc.target/vax/cmpelim-lt-notsi.c: Likewise. --- gcc/testsuite/gcc.target/vax/cmpelim-eq-notsi.c | 4 ++-- gcc/testsuite/gcc.target/vax/cmpelim-le-notsi.c | 4 ++-- gcc/testsuite/gcc.target/vax/cmpelim-lt-notsi.c | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) gcc-test-vax-notsi-pr96685.diff