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]

Re: [PATCH] Fix ICE in expand_cse_reciprocals (PR tree-optimization/42078)


On Jun  3, 2011, Alexandre Oliva <aoliva@redhat.com> wrote:

> According to http://gcc.gnu.org/ml/gcc-patches/2009-11/msg01082.html
> on Nov 20, 2009, Alexandre Oliva <aoliva@redhat.com> wrote:

>> On Nov 19, 2009, Richard Guenther <richard.guenther@gmail.com> wrote:
>>> In fact this exchanging of the LHS (or rather invalidating of the
>>> SSA name value) should be a helper function that knows
>>> the implementation details and avoids going through releasing
>>> and allocating the name.

>> Okie dokie, here's a sequence of patches that implements helper
>> functions for this sort of stuff.

>> The first patch introduces machinery to propagate âdyingâ DEFs into
>> debug stmts, while replacing them with other SSA_NAMEs.

> This is already in.

>> The second extends it so as to enable the old LHS to be redefined
>> e.g. in terms of the new LHS.  IIRC this may be useful in some other
>> transformations that, in the process of introducing VTA, I changed from
>> modifying stmts in place to inserting new stmts and removing others.  I
>> haven't looked for them yet.

>> The third uses this new feature for the case at hand, while avoiding
>> computing the reciprocal expression if we know it won't be used.

> Updated versions of these follow.  Regstrapped on x86_64-linux-gnu and
> i686-linux-gnu.  Ok to install?

I was asked to submit these again in stage1, so...  Ping?
(updated and re-tested)

for  gcc/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	PR tree-optimization/42078
	* gimple.h (gimple_replace_lhs_wants_value): New.
	(gimple_replace_lhs): Add value.
	* gimple.c (gimple_replace_lhs): Likewise.  Pass it on.
	* tree-flow.h (insert_debug_temp_for_var_def): Add value.
	* tree-ssa.c (insert_debug_temp_for_var_def): Likewise.  Use it.
	(insert_debug_temps_for_defs): Pass NULL value.
	* tree-ssanames.c (release_ssa_name): Likewise.
	* tree-ssa-math-opts.c (execute_cse_reciprocals): Likewise.
	* gcc/tree-ssa-reassoc.c (repropagate_negates): Likewise.

Index: gcc/gimple.h
===================================================================
--- gcc/gimple.h.orig	2012-04-07 20:24:41.796529955 -0300
+++ gcc/gimple.h	2012-04-08 02:06:44.417457641 -0300
@@ -943,7 +943,7 @@ void gimple_assign_set_rhs_with_ops_1 (g
 				       tree, tree, tree);
 tree gimple_get_lhs (const_gimple);
 void gimple_set_lhs (gimple, tree);
-void gimple_replace_lhs (gimple, tree);
+void gimple_replace_lhs (gimple, tree, tree);
 gimple gimple_copy (gimple);
 void gimple_set_modified (gimple, bool);
 void gimple_cond_get_ops_from_tree (tree, enum tree_code *, tree *, tree *);
@@ -2521,6 +2521,14 @@ gimple_has_lhs (gimple stmt)
 	      && gimple_call_lhs (stmt) != NULL_TREE));
 }
 
+/* Return true if it might be useful to pass a VALUE to
+   gimple_replace_lhs ().  */
+static inline bool
+gimple_replace_lhs_wants_value (void)
+{
+  return MAY_HAVE_DEBUG_STMTS;
+}
+
 
 /* Return the code of the predicate computed by conditional statement GS.  */
 
Index: gcc/gimple.c
===================================================================
--- gcc/gimple.c.orig	2012-04-07 20:24:41.796529955 -0300
+++ gcc/gimple.c	2012-04-08 02:06:44.606455321 -0300
@@ -2239,10 +2239,10 @@ gimple_set_lhs (gimple stmt, tree lhs)
    expression with a different value.
 
    This will update any annotations (say debug bind stmts) referring
-   to the original LHS, so that they use the RHS instead.  This is
-   done even if NLHS and LHS are the same, for it is understood that
-   the RHS will be modified afterwards, and NLHS will not be assigned
-   an equivalent value.
+   to the original LHS, so that they use VALUE or the RHS instead.
+   This is done even if NLHS and LHS are the same, for it is
+   understood that the RHS will be modified afterwards, and NLHS will
+   not be assigned an equivalent value.
 
    Adjusting any non-annotation uses of the LHS, if needed, is a
    responsibility of the caller.
@@ -2254,15 +2254,20 @@ gimple_set_lhs (gimple stmt, tree lhs)
    copying and removing.  */
 
 void
-gimple_replace_lhs (gimple stmt, tree nlhs)
+gimple_replace_lhs (gimple stmt, tree nlhs, tree value)
 {
+  /* If the conditions in which this function uses VALUE change,
+     adjust gimple_replace_lhs_wants_value().  */
+  gcc_assert (gimple_replace_lhs_wants_value ()
+	      == MAY_HAVE_DEBUG_STMTS);
+
   if (MAY_HAVE_DEBUG_STMTS)
     {
       tree lhs = gimple_get_lhs (stmt);
 
       gcc_assert (SSA_NAME_DEF_STMT (lhs) == stmt);
 
-      insert_debug_temp_for_var_def (NULL, lhs);
+      insert_debug_temp_for_var_def (NULL, lhs, value);
     }
 
   gimple_set_lhs (stmt, nlhs);
Index: gcc/tree-flow.h
===================================================================
--- gcc/tree-flow.h.orig	2012-04-07 20:24:45.350481393 -0300
+++ gcc/tree-flow.h	2012-04-08 02:06:44.658454684 -0300
@@ -563,7 +563,7 @@ typedef bool (*walk_use_def_chains_fn) (
 extern void walk_use_def_chains (tree, walk_use_def_chains_fn, void *, bool);
 
 void insert_debug_temps_for_defs (gimple_stmt_iterator *);
-void insert_debug_temp_for_var_def (gimple_stmt_iterator *, tree);
+void insert_debug_temp_for_var_def (gimple_stmt_iterator *, tree, tree);
 void reset_debug_uses (gimple);
 void release_defs_bitset (bitmap toremove);
 
Index: gcc/tree-ssa.c
===================================================================
--- gcc/tree-ssa.c.orig	2012-02-25 09:45:32.788781511 -0200
+++ gcc/tree-ssa.c	2012-04-08 02:06:44.941451213 -0300
@@ -300,17 +300,25 @@ find_released_ssa_name (tree *tp, int *w
 
 /* Insert a DEBUG BIND stmt before the DEF of VAR if VAR is referenced
    by other DEBUG stmts, and replace uses of the DEF with the
-   newly-created debug temp.  */
+   newly-created debug temp, or with the DEF's value, if the
+   substitution is valid.  The value is taken from VALUE, if given, or
+   from the DEF stmt, taken from GSI, if given, or from the DEF of
+   VAR.  If VALUE and VAR are the same, no DEBUG BIND stmt will be
+   created, and all uses of VAR will be reset.  If a DEBUG BIND stmt
+   has to be created, it will be inserted before or after GSI or the
+   DEF, depending */
 
 void
-insert_debug_temp_for_var_def (gimple_stmt_iterator *gsi, tree var)
+insert_debug_temp_for_var_def (gimple_stmt_iterator *gsi, tree var, tree value)
 {
   imm_use_iterator imm_iter;
   use_operand_p use_p;
   gimple stmt;
   gimple def_stmt = NULL;
   int usecount = 0;
-  tree value = NULL;
+  bool given_value;
+  gimple def_temp = NULL;
+  gimple_stmt_iterator ngsi;
 
   if (!MAY_HAVE_DEBUG_STMTS)
     return;
@@ -350,10 +358,13 @@ insert_debug_temp_for_var_def (gimple_st
   else
     def_stmt = SSA_NAME_DEF_STMT (var);
 
+  given_value = false;
+  if (value)
+    given_value = true;
   /* If we didn't get an insertion point, and the stmt has already
      been removed, we won't be able to insert the debug bind stmt, so
      we'll have to drop debug information.  */
-  if (gimple_code (def_stmt) == GIMPLE_PHI)
+  else if (gimple_code (def_stmt) == GIMPLE_PHI)
     {
       value = degenerate_phi_result (def_stmt);
       if (value && walk_tree (&value, find_released_ssa_name, NULL, NULL))
@@ -408,6 +419,9 @@ insert_debug_temp_for_var_def (gimple_st
 	value = gimple_assign_rhs_to_tree (def_stmt);
     }
 
+  if (value == var)
+    value = NULL;
+
   if (value)
     {
       /* If there's a single use of VAR, and VAR is the entire debug
@@ -425,7 +439,7 @@ insert_debug_temp_for_var_def (gimple_st
 	 at the expense of duplication of expressions.  */
 
       if (CONSTANT_CLASS_P (value)
-	  || gimple_code (def_stmt) == GIMPLE_PHI
+	  || (gimple_code (def_stmt) == GIMPLE_PHI && !given_value)
 	  || (usecount == 1
 	      && (!gimple_assign_single_p (def_stmt)
 		  || is_gimple_min_invariant (value)))
@@ -433,7 +447,6 @@ insert_debug_temp_for_var_def (gimple_st
 	value = unshare_expr (value);
       else
 	{
-	  gimple def_temp;
 	  tree vexpr = make_node (DEBUG_EXPR_DECL);
 
 	  def_temp = gimple_build_debug_bind (vexpr,
@@ -447,14 +460,27 @@ insert_debug_temp_for_var_def (gimple_st
 	  else
 	    DECL_MODE (vexpr) = TYPE_MODE (TREE_TYPE (value));
 
-	  if (gsi)
-	    gsi_insert_before (gsi, def_temp, GSI_SAME_STMT);
-	  else
+	  if (gimple_code (def_stmt) == GIMPLE_PHI)
+	    {
+	      basic_block bb;
+
+	      if (gsi)
+		bb = gsi_bb (*gsi);
+	      else
+		bb = gimple_bb (def_stmt);
+
+	      ngsi = gsi_after_labels (bb);
+	      gsi = &ngsi;
+	    }
+	  else if (!gsi)
 	    {
-	      gimple_stmt_iterator ngsi = gsi_for_stmt (def_stmt);
-	      gsi_insert_before (&ngsi, def_temp, GSI_SAME_STMT);
+	      ngsi = gsi_for_stmt (def_stmt);
+	      gsi = &ngsi;
 	    }
 
+	  if (!given_value)
+	    gsi_insert_before (gsi, def_temp, GSI_SAME_STMT);
+
 	  value = vexpr;
 	}
     }
@@ -486,6 +512,11 @@ insert_debug_temp_for_var_def (gimple_st
 
       update_stmt (stmt);
     }
+
+  /* Insert the definition last, so that we don't modify it in case
+     VALUE references VAR.  Assume the caller intends it to be so.  */
+  if (def_temp && given_value)
+    gsi_insert_after (gsi, def_temp, GSI_SAME_STMT);
 }
 
 
@@ -512,7 +543,7 @@ insert_debug_temps_for_defs (gimple_stmt
       if (TREE_CODE (var) != SSA_NAME)
 	continue;
 
-      insert_debug_temp_for_var_def (gsi, var);
+      insert_debug_temp_for_var_def (gsi, var, NULL);
     }
 }
 
Index: gcc/tree-ssanames.c
===================================================================
--- gcc/tree-ssanames.c.orig	2012-04-07 20:24:45.383480943 -0300
+++ gcc/tree-ssanames.c	2012-04-08 02:06:45.215447852 -0300
@@ -203,7 +203,7 @@ release_ssa_name (tree var)
       use_operand_p imm = &(SSA_NAME_IMM_USE_NODE (var));
 
       if (MAY_HAVE_DEBUG_STMTS)
-	insert_debug_temp_for_var_def (NULL, var);
+	insert_debug_temp_for_var_def (NULL, var, NULL);
 
 #ifdef ENABLE_CHECKING
       verify_imm_links (stderr, var);
Index: gcc/tree-ssa-math-opts.c
===================================================================
--- gcc/tree-ssa-math-opts.c.orig	2012-04-07 20:24:45.354481340 -0300
+++ gcc/tree-ssa-math-opts.c	2012-04-08 02:06:45.383445792 -0300
@@ -604,7 +604,7 @@ execute_cse_reciprocals (void)
 		  if (fail)
 		    continue;
 
-		  gimple_replace_lhs (stmt1, arg1);
+		  gimple_replace_lhs (stmt1, arg1, NULL);
 		  gimple_call_set_fndecl (stmt1, fndecl);
 		  update_stmt (stmt1);
 		  reciprocal_stats.rfuncs_inserted++;
Index: gcc/tree-ssa-reassoc.c
===================================================================
--- gcc/tree-ssa-reassoc.c.orig	2011-10-04 02:32:44.000000000 -0300
+++ gcc/tree-ssa-reassoc.c	2012-04-08 02:06:45.923439168 -0300
@@ -2719,7 +2719,7 @@ repropagate_negates (void)
 	      tree a = gimple_assign_rhs1 (feed);
 	      tree rhs2 = gimple_assign_rhs2 (user);
 	      gimple_stmt_iterator gsi = gsi_for_stmt (feed), gsi2;
-	      gimple_replace_lhs (feed, negate);
+	      gimple_replace_lhs (feed, negate, NULL);
 	      gimple_assign_set_rhs_with_ops (&gsi, PLUS_EXPR, a, rhs2);
 	      update_stmt (gsi_stmt (gsi));
 	      gsi2 = gsi_for_stmt (user);
for  gcc/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	PR tree-optimization/42078
	* tree-ssa-math-opts.c (execute_cse_reciprocals): Compute reciprocal
	value for debug stmts.  

Index: gcc/tree-ssa-math-opts.c
===================================================================
--- gcc/tree-ssa-math-opts.c.orig	2012-04-08 02:06:45.383445792 -0300
+++ gcc/tree-ssa-math-opts.c	2012-04-08 02:09:53.000000000 -0300
@@ -575,6 +575,7 @@ execute_cse_reciprocals (void)
 		  bool md_code, fail;
 		  imm_use_iterator ui;
 		  use_operand_p use_p;
+		  tree value;
 
 		  code = DECL_FUNCTION_CODE (fndecl);
 		  md_code = DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_MD;
@@ -604,14 +605,26 @@ execute_cse_reciprocals (void)
 		  if (fail)
 		    continue;
 
-		  gimple_replace_lhs (stmt1, arg1, NULL);
+		  if (gimple_replace_lhs_wants_value ())
+		    {
+		      tree t = TREE_TYPE (arg1);
+
+		      value = build2 (RDIV_EXPR, t, build_one_cst (t), arg1);
+		    }
+		  else
+		    value = NULL;
+
+		  gimple_replace_lhs (stmt1, arg1, value);
 		  gimple_call_set_fndecl (stmt1, fndecl);
 		  update_stmt (stmt1);
 		  reciprocal_stats.rfuncs_inserted++;
 
 		  FOR_EACH_IMM_USE_STMT (stmt, ui, arg1)
 		    {
-		      gimple_stmt_iterator gsi = gsi_for_stmt (stmt);
+		      gimple_stmt_iterator gsi;
+		      if (is_gimple_debug (stmt))
+			continue;
+		      gsi = gsi_for_stmt (stmt);
 		      gimple_assign_set_rhs_code (stmt, MULT_EXPR);
 		      fold_stmt_inplace (&gsi);
 		      update_stmt (stmt);

-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist      Red Hat Brazil Compiler Engineer

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