This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] Properly valueize values we value-number to


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  <rguenther@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.  */


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]