This is something I noticed some time ago and that I remembered when
you added that looping SSA_NAME_VALUE to simplify_control_stmt_condition.
Currently DOM doesn't make sure that when setting
SSA_NAME_VALUE (x) = y that SSA_NAME_VALUE (y) == y, thus you could
get SSA_NAME_VALUE forming a chain until you reach the final value.

Thus the following patch which fixes all occurances and removes the
looping from simplify_control_stmt_condition.  Did you have any
testcase when you added that looping?

Note that this is purely by code inspection and I don't have any
testcase where a SSA_NAME_VALUE chain causes missed optimizations
(you'd have to disable a lot of other optimizations before dom1
to be able to create a reasonably simple one).

Anyway - the tree-ssa-dom.c part bootstrapped and tested ok on
x86_64-unknown-linux-gnu, the tree-ssa-threadedge.c part bootstrapped
ok ontop of that and testing is still in progress.

Ok?

Thanks,
Richard.

2015-02-17  Richard Biener  <rguent...@suse.de>

        * tree-ssa-dom.c (record_equivalences_from_phis): Valueize PHI arg.
        (record_const_or_copy_1): Valueize y.
        (record_equivalences_from_stmt): Valueize rhs.
        * tree-ssa-threadedge.c (simplify_control_stmt_condition):
        Remove repeated valueization.

Index: gcc/tree-ssa-dom.c
===================================================================
--- gcc/tree-ssa-dom.c  (revision 220751)
+++ gcc/tree-ssa-dom.c  (working copy)
@@ -1223,6 +1223,13 @@ record_equivalences_from_phis (basic_blo
          if (lhs == t)
            continue;
 
+         /* Valueize t.  */
+         if (TREE_CODE (t) == SSA_NAME)
+           {
+             tree tmp = SSA_NAME_VALUE (t);
+             t = tmp ? tmp : t;
+           }
+
          /* If we have not processed an alternative yet, then set
             RHS to this alternative.  */
          if (rhs == NULL)
@@ -1566,6 +1573,13 @@ record_conditions (struct edge_info *edg
 static void
 record_const_or_copy_1 (tree x, tree y, tree prev_x)
 {
+  /* Valueize y.  */
+  if (TREE_CODE (y) == SSA_NAME)
+    {
+      tree tmp = SSA_NAME_VALUE (y);
+      y = tmp ? tmp : y;
+    }
+
   set_ssa_name_value (x, y);
 
   if (dump_file && (dump_flags & TDF_DETAILS))
@@ -2184,18 +2198,25 @@ record_equivalences_from_stmt (gimple st
       if (may_optimize_p
          && (TREE_CODE (rhs) == SSA_NAME
              || is_gimple_min_invariant (rhs)))
-      {
-       if (dump_file && (dump_flags & TDF_DETAILS))
-         {
-           fprintf (dump_file, "==== ASGN ");
-           print_generic_expr (dump_file, lhs, 0);
-           fprintf (dump_file, " = ");
-           print_generic_expr (dump_file, rhs, 0);
-           fprintf (dump_file, "\n");
-         }
+       {
+         /* Valueize rhs.  */
+         if (TREE_CODE (rhs) == SSA_NAME)
+           {
+             tree tmp = SSA_NAME_VALUE (rhs);
+             rhs = tmp ? tmp : rhs;
+           }
 
-       set_ssa_name_value (lhs, rhs);
-      }
+         if (dump_file && (dump_flags & TDF_DETAILS))
+           {
+             fprintf (dump_file, "==== ASGN ");
+             print_generic_expr (dump_file, lhs, 0);
+             fprintf (dump_file, " = ");
+             print_generic_expr (dump_file, rhs, 0);
+             fprintf (dump_file, "\n");
+           }
+
+         set_ssa_name_value (lhs, rhs);
+       }
     }
 
   /* Make sure we can propagate &x + CST.  */
Index: gcc/tree-ssa-threadedge.c
===================================================================
--- gcc/tree-ssa-threadedge.c   (revision 220751)
+++ gcc/tree-ssa-threadedge.c   (working copy)
@@ -591,29 +591,13 @@ simplify_control_stmt_condition (edge e,
       cond_code = gimple_cond_code (stmt);
 
       /* Get the current value of both operands.  */
-      if (TREE_CODE (op0) == SSA_NAME)
-       {
-         for (int i = 0; i < 2; i++)
-           {
-             if (TREE_CODE (op0) == SSA_NAME
-                 && SSA_NAME_VALUE (op0))
-               op0 = SSA_NAME_VALUE (op0);
-             else
-               break;
-           }
-       }
-
-      if (TREE_CODE (op1) == SSA_NAME)
-       {
-         for (int i = 0; i < 2; i++)
-           {
-             if (TREE_CODE (op1) == SSA_NAME
-                 && SSA_NAME_VALUE (op1))
-               op1 = SSA_NAME_VALUE (op1);
-             else
-               break;
-           }
-       }
+      if (TREE_CODE (op0) == SSA_NAME
+         && SSA_NAME_VALUE (op0))
+       op0 = SSA_NAME_VALUE (op0);
+
+      if (TREE_CODE (op1) == SSA_NAME
+         && SSA_NAME_VALUE (op1))
+       op1 = SSA_NAME_VALUE (op1);
 
       if (handle_dominating_asserts)
        {
@@ -682,22 +666,11 @@ simplify_control_stmt_condition (edge e,
       tree original_lhs = cond;
       cached_lhs = cond;
 
-      /* Get the variable's current value from the equivalence chains.
-
-        It is possible to get loops in the SSA_NAME_VALUE chains
-        (consider threading the backedge of a loop where we have
-        a loop invariant SSA_NAME used in the condition.  */
-      if (cached_lhs)
-       {
-         for (int i = 0; i < 2; i++)
-           {
-             if (TREE_CODE (cached_lhs) == SSA_NAME
-                 && SSA_NAME_VALUE (cached_lhs))
-               cached_lhs = SSA_NAME_VALUE (cached_lhs);
-             else
-               break;
-           }
-       }
+      /* Get the variable's current value from the equivalence chains.  */
+      if (cached_lhs
+         && TREE_CODE (cached_lhs) == SSA_NAME
+         && SSA_NAME_VALUE (cached_lhs))
+       cached_lhs = SSA_NAME_VALUE (cached_lhs);
 
       /* If we're dominated by a suitable ASSERT_EXPR, then
         update CACHED_LHS appropriately.  */

Reply via email to