Richard & Segher, if y'all could check my analysis here, it'd be appreciated.
pr90275 is a P2 regression that is only triggering on ARM. David's testcase in c#1 is the best for this problem as it doesn't require magic flags like -fno-dce to trigger. The block in question: > (code_label 89 88 90 24 15 (nil) [0 uses]) > (note 90 89 97 24 [bb 24] NOTE_INSN_BASIC_BLOCK) > (insn 97 90 98 24 (parallel [ > (set (reg:CC 100 cc) > (compare:CC (reg:SI 131 [ d_lsm.21 ]) > (const_int 0 [0]))) > (set (reg:SI 135 [ d_lsm.21 ]) > (reg:SI 131 [ d_lsm.21 ])) > ]) "pr90275.c":21:45 248 {*movsi_compare0} > (expr_list:REG_DEAD (reg:SI 131 [ d_lsm.21 ]) > (nil))) > (insn 98 97 151 24 (set (reg:SI 136 [+4 ]) > (reg:SI 132 [ d_lsm.21+4 ])) "pr90275.c":21:45 241 {*arm_movsi_insn} > (expr_list:REG_DEAD (reg:SI 132 [ d_lsm.21+4 ]) > (expr_list:REG_DEAD (reg:CC 100 cc) > (nil)))) > (insn 151 98 152 24 (set (reg:SI 131 [ d_lsm.21 ]) > (reg:SI 131 [ d_lsm.21 ])) "pr90275.c":21:45 241 {*arm_movsi_insn} > (expr_list:REG_DEAD (reg:SI 135 [ d_lsm.21 ]) > (nil))) > (insn 152 151 103 24 (set (reg:SI 132 [ d_lsm.21+4 ]) > (reg:SI 136 [+4 ])) "pr90275.c":21:45 241 {*arm_movsi_insn} > (expr_list:REG_DEAD (reg:SI 136 [+4 ]) > (nil))) > insns 97 and 151 are the most important. We process insn 97 which creates an equivalency between r135 and r131. This is expressed by putting both on on the "same_value" chain (table_elt->{next,prev}_same_value). When we put the REGs on the chain we'll set REG_QTY to a positive number which indicates their values are valid. We continue processing insns forward and run into insn 151 which is a self-copy. First CSE will invalidate r131 (because its set). Invalidation is accomplished by setting REG_QTY for r131 to a negative value. It does not remove r131 from the same value chains. Then CSE will call insert_regs for r131. The qty is not valid, so we get into this code: > if (modified || ! qty_valid) > { > if (classp) > for (classp = classp->first_same_value; > classp != 0; > classp = classp->next_same_value) > if (REG_P (classp->exp) > && GET_MODE (classp->exp) == GET_MODE (x)) > { > unsigned c_regno = REGNO (classp->exp); > > gcc_assert (REGNO_QTY_VALID_P (c_regno)); > [ ... ] So we walk the chain of same values for r131. WHen walking we run into r131 itself. Since r131 has been invalidated we trip the assert. The fix is pretty simple. We just arrange to stop processing insns that are nop reg->reg copies much like we already do for mem->mem copies and (set (pc) (pc)). This has bootstrapped and regression tested on x86_64. I've also verified it fixes the testcase in c#1 of pr90275, the test in pr93125 and pr92388. Interestingly enough I couldn't trigger the original testcase in 90275, but I'm confident this is ultimately all the same problem. OK for the trunk? Thanks, Jeff
diff --git a/gcc/ChangeLog b/gcc/ChangeLog index d6b5ded32a4..90d9f9d92d3 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,8 @@ +2020-02-04 Jeff Law <l...@redhat.com> + + PR rtl-optimization/90275 + * cse.c (cse_insn): Stop processing reg->reg nop moves early. + 2020-02-04 Richard Biener <rguent...@suse.de> PR tree-optimization/93538 diff --git a/gcc/cse.c b/gcc/cse.c index 79ee0ce80e3..6e18bdae85f 100644 --- a/gcc/cse.c +++ b/gcc/cse.c @@ -5572,6 +5572,16 @@ cse_insn (rtx_insn *insn) sets[i].rtl = 0; } + /* Similarly for no-op moves. */ + if (n_sets == 1 + && GET_CODE (src) == REG + && src == dest) + { + cse_cfg_altered |= delete_insn_and_edges (insn); + /* No more processing for this set. */ + sets[i].rtl = 0; + } + /* If this SET is now setting PC to a label, we know it used to be a conditional or computed branch. */ else if (dest == pc_rtx && GET_CODE (src) == LABEL_REF diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index d2dc6648bc4..7be52bd6d2a 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,8 @@ +2020-02-04 Jeff Law <l...@redhat.com> + + PR rtl-optimization/90275 + * gcc.c-torture/compile/pr90275.c: New test + 2020-02-04 David Malcolm <dmalc...@redhat.com> * gcc.dg/analyzer/data-model-1.c (struct coord): Convert fields diff --git a/gcc/testsuite/gcc.c-torture/compile/pr90275.c b/gcc/testsuite/gcc.c-torture/compile/pr90275.c new file mode 100644 index 00000000000..83e0df77226 --- /dev/null +++ b/gcc/testsuite/gcc.c-torture/compile/pr90275.c @@ -0,0 +1,27 @@ +a, b, c; + +long long d; + +e() { + + char f; + + for (;;) { + + c = a = c ? 5 : 0; + + if (f) { + + b = a; + + f = d; + + } + + (d || b) < (a > e) ?: (b ? 0 : f) || (d -= f); + + } + +} + +