Re: [committed][PATCH] Fix bogus propagation in DOM

[Jeff Law <law at redhat dot com>]

On 11/21/2017 07:56 AM, Richard Biener wrote:
>>
>>
>> I considered trying to key behavior on EDGE_DFS_BACK (6->8).  It'd be
>> something like don't record equivalences from a degenerate PHI where the
>> remaining edge(s) are EDGE_DFS_BACK.  But I just couldn't convince
>> myself that was actually reasonable or sufficient in general.
> 
> The key point is that you can never record "symbolic" equivalences on
> backedges.  Because in SSA you rely on PHIs to fence SSA names from
> leaking cross iterations.  That is, _1 in iteration 1 is obviously not equal
> to _1 in iteration 2.
> 
> Other passes like VRP and value-numbering need to take care of this
> as well.
> 
> You can of course record that _1 is 2 (constant) even across a backedge.
> 
> So I think your consideration was exactly correct.
So finally getting back to this.  Thankfully I was able to find an old
branch, revert the tree-ssa-dom.c bits and via some on-the-fly hackery
trigger the issue again.

This also enabled me to verify that triggering on EDGE_DFS_BACK would in
fact resolve this issue.

So attached are two patches.  The first is the reversion.  The second is
the change to avoid recording the equivalence for a backedge in the CFG.

Bootstrapped and regression tested on x86_64.  Also verified by hand
(using that old branch) that we wouldn't get into the infinite loop if
we reverted and used EDGE_DFS_BACK to trigger ignoring the equivalence.

Installing on the trunk.

Jeff

commit 46a2536a74d7f6981a02da05041885d097d9e23c
Author: Jeff Law <law@torsion.usersys.redhat.com>
Date:   Mon Dec 18 19:18:47 2017 -0500

            Revert
            2017-11-19  Jeff Law  <law@redhat.com>
    
            * tree-ssa-dom.c (record_equivalences_from_phis): Fix handling
            of degenerates resulting from ignoring an edge.

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index d165b5ccf41..b5854f7a67c 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,11 @@
+2017-12-18  Jeff Law  <law@redhat.com>
+
+	Revert
+	2017-11-19  Jeff Law  <law@redhat.com>
+
+	* tree-ssa-dom.c (record_equivalences_from_phis): Fix handling
+	of degenerates resulting from ignoring an edge.
+
 2017-12-18  Martin Sebor  <msebor@redhat.com>
 
 	PR middle-end/83373
diff --git a/gcc/tree-ssa-dom.c b/gcc/tree-ssa-dom.c
index 93c992a9215..05bc807071f 100644
--- a/gcc/tree-ssa-dom.c
+++ b/gcc/tree-ssa-dom.c
@@ -1109,7 +1109,6 @@ record_equivalences_from_phis (basic_block bb)
       tree rhs = NULL;
       size_t i;
 
-      bool ignored_phi_arg = false;
       for (i = 0; i < gimple_phi_num_args (phi); i++)
 	{
 	  tree t = gimple_phi_arg_def (phi, i);
@@ -1120,14 +1119,10 @@ record_equivalences_from_phis (basic_block bb)
 	  if (lhs == t)
 	    continue;
 
-	  /* We want to track if we ignored any PHI arguments because
-	     their associated edges were not executable.  This impacts
-	     whether or not we can use any equivalence we might discover.  */
+	  /* If the associated edge is not marked as executable, then it
+	     can be ignored.  */
 	  if ((gimple_phi_arg_edge (phi, i)->flags & EDGE_EXECUTABLE) == 0)
-	    {
-	      ignored_phi_arg = true;
-	      continue;
-	    }
+	    continue;
 
 	  t = dom_valueize (t);
 
@@ -1152,15 +1147,9 @@ record_equivalences_from_phis (basic_block bb)
 	 a useful equivalence.  We do not need to record unwind data for
 	 this, since this is a true assignment and not an equivalence
 	 inferred from a comparison.  All uses of this ssa name are dominated
-	 by this assignment, so unwinding just costs time and space.
-
-	 Note that if we ignored a PHI argument and the resulting equivalence
-	 is SSA_NAME = SSA_NAME.  Then we can not use the equivalence as the
-	 uses of the LHS SSA_NAME are not necessarily dominated by the
-	 assignment of the RHS SSA_NAME.  */
+	 by this assignment, so unwinding just costs time and space.  */
       if (i == gimple_phi_num_args (phi)
-	  && may_propagate_copy (lhs, rhs)
-	  && (!ignored_phi_arg || TREE_CODE (rhs) != SSA_NAME))
+	  && may_propagate_copy (lhs, rhs))
 	set_ssa_name_value (lhs, rhs);
     }
 }

commit 61046656ff4e832bd1019e90c251b1c4c2c43883
Author: Jeff Law <law@torsion.usersys.redhat.com>
Date:   Mon Dec 18 23:28:37 2017 -0500

            * tree-ssa-dom.c (record_equivalences_from_phis): Do not
            record symbolic equivalences from backedges in the CFG.

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index b5854f7a67c..96146e8934f 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,5 +1,8 @@
 2017-12-18  Jeff Law  <law@redhat.com>
 
+	* tree-ssa-dom.c (record_equivalences_from_phis): Do not
+	record symbolic equivalences from backedges in the CFG.
+
 	Revert
 	2017-11-19  Jeff Law  <law@redhat.com>
 
diff --git a/gcc/tree-ssa-dom.c b/gcc/tree-ssa-dom.c
index 05bc807071f..663d07b6fe9 100644
--- a/gcc/tree-ssa-dom.c
+++ b/gcc/tree-ssa-dom.c
@@ -1126,6 +1126,12 @@ record_equivalences_from_phis (basic_block bb)
 
 	  t = dom_valueize (t);
 
+	  /* If T is an SSA_NAME and its associated edge is a backedge,
+	     then quit as we can not utilize this equivalence.  */
+	  if (TREE_CODE (t) == SSA_NAME
+	      && (gimple_phi_arg_edge (phi, i)->flags & EDGE_DFS_BACK))
+	    break;
+
 	  /* If we have not processed an alternative yet, then set
 	     RHS to this alternative.  */
 	  if (rhs == NULL)