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);
+
+  }
+
+}
+
+

Reply via email to