This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Fix ICE in expand_cse_reciprocals (PR tree-optimization/42078)
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.
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.
Are any of them ok to install, if they pass regstrap?
for gcc/ChangeLog
from Alexandre Oliva <aoliva@redhat.com>
PR tree-optimization/42078
* gimple.h (gimple_replace_lhs): New declaration.
* gimple.c (gimple_replace_lhs): New function.
* tree-ssa-math-opts.c (execute_cse_reciprocals): Call it before
modifying the call.
for gcc/testsuite/ChangeLog
from Jakub Jelinek <jakub@redhat.com>
PR tree-optimization/42078
* gcc.dg/pr42078.c: New test.
Index: gcc/gimple.h
===================================================================
--- gcc/gimple.h.orig 2009-11-20 06:15:41.000000000 -0200
+++ gcc/gimple.h 2009-11-20 06:18:14.000000000 -0200
@@ -843,6 +843,7 @@ void gimple_assign_set_rhs_with_ops (gim
tree, tree);
tree gimple_get_lhs (const_gimple);
void gimple_set_lhs (gimple, tree);
+void gimple_replace_lhs (gimple, tree);
gimple gimple_copy (gimple);
bool is_gimple_operand (const_tree);
void gimple_set_modified (gimple, bool);
Index: gcc/gimple.c
===================================================================
--- gcc/gimple.c.orig 2009-11-20 06:15:42.000000000 -0200
+++ gcc/gimple.c 2009-11-20 06:18:25.000000000 -0200
@@ -1981,6 +1981,38 @@ gimple_set_lhs (gimple stmt, tree lhs)
gcc_unreachable();
}
+/* Replace the LHS of STMT, an assignment, either a GIMPLE_ASSIGN or a
+ GIMPLE_CALL, with NLHS, in preparation for modifying the RHS to an
+ 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.
+
+ Adjusting any non-annotation uses of the LHS, if needed, is a
+ responsibility of the caller.
+
+ The effect of this call should be pretty much the same as that of
+ inserting a copy of STMT before STMT, and then removing the
+ original stmt, at which time gsi_remove() would have update
+ annotations, but using this function saves all the inserting,
+ copying and removing. */
+
+void
+gimple_replace_lhs (gimple stmt, tree nlhs)
+{
+ if (MAY_HAVE_DEBUG_STMTS)
+ {
+ gimple_stmt_iterator gsi = gsi_for_stmt (stmt);
+ tree lhs = gimple_get_lhs (stmt);
+
+ insert_debug_temp_for_var_def (&gsi, lhs);
+ }
+
+ gimple_set_lhs (stmt, nlhs);
+}
/* Return a deep copy of statement STMT. All the operands from STMT
are reallocated and copied using unshare_expr. The DEF, USE, VDEF
Index: gcc/tree-ssa-math-opts.c
===================================================================
--- gcc/tree-ssa-math-opts.c.orig 2009-11-20 06:15:41.000000000 -0200
+++ gcc/tree-ssa-math-opts.c 2009-11-20 06:18:14.000000000 -0200
@@ -563,6 +563,7 @@ execute_cse_reciprocals (void)
if (fail)
continue;
+ gimple_replace_lhs (stmt1, arg1);
gimple_call_set_fndecl (stmt1, fndecl);
update_stmt (stmt1);
Index: gcc/testsuite/gcc.dg/pr42078.c
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ gcc/testsuite/gcc.dg/pr42078.c 2009-11-20 06:16:30.000000000 -0200
@@ -0,0 +1,22 @@
+/* PR tree-optimization/42078 */
+/* { dg-do compile } */
+/* { dg-options "-g -O -ffast-math" } */
+
+double sqrt (double x);
+
+float
+foo (float x)
+{
+ float y = sqrt (x);
+ return x / y;
+}
+
+inline float
+bar (float x)
+{
+ float y = sqrt (x);
+ float a = y;
+ float b = y;
+ float c = y;
+ return x / y;
+}
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.
Index: gcc/gimple.h
===================================================================
--- gcc/gimple.h.orig 2009-11-20 06:18:14.000000000 -0200
+++ gcc/gimple.h 2009-11-20 06:18:35.000000000 -0200
@@ -843,7 +843,7 @@ void gimple_assign_set_rhs_with_ops (gim
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);
bool is_gimple_operand (const_tree);
void gimple_set_modified (gimple, bool);
@@ -2226,6 +2226,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 2009-11-20 06:18:25.000000000 -0200
+++ gcc/gimple.c 2009-11-20 06:20:35.000000000 -0200
@@ -1986,10 +1986,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.
@@ -2001,14 +2001,19 @@ 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)
{
gimple_stmt_iterator gsi = gsi_for_stmt (stmt);
tree lhs = gimple_get_lhs (stmt);
- insert_debug_temp_for_var_def (&gsi, lhs);
+ insert_debug_temp_for_var_def (&gsi, lhs, value);
}
gimple_set_lhs (stmt, nlhs);
Index: gcc/tree-flow.h
===================================================================
--- gcc/tree-flow.h.orig 2009-11-20 06:18:15.000000000 -0200
+++ gcc/tree-flow.h 2009-11-20 06:18:35.000000000 -0200
@@ -638,7 +638,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 release_defs_bitset (bitmap toremove);
/* In tree-into-ssa.c */
Index: gcc/tree-ssa.c
===================================================================
--- gcc/tree-ssa.c.orig 2009-11-20 06:18:14.000000000 -0200
+++ gcc/tree-ssa.c 2009-11-20 07:03:50.000000000 -0200
@@ -297,17 +297,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;
@@ -343,10 +351,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))
@@ -397,6 +408,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
@@ -414,7 +428,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)))
@@ -422,7 +436,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,
@@ -436,14 +449,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;
}
}
@@ -466,6 +492,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);
}
@@ -492,7 +523,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 2009-11-20 06:18:14.000000000 -0200
+++ gcc/tree-ssanames.c 2009-11-20 06:18:35.000000000 -0200
@@ -206,7 +206,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 2009-11-20 06:18:14.000000000 -0200
+++ gcc/tree-ssa-math-opts.c 2009-11-20 07:03:50.000000000 -0200
@@ -563,7 +563,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);
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 2009-11-20 06:43:14.000000000 -0200
+++ gcc/tree-ssa-math-opts.c 2009-11-20 06:43:19.000000000 -0200
@@ -534,6 +534,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;
@@ -563,12 +564,23 @@ 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);
FOR_EACH_IMM_USE_STMT (stmt, ui, arg1)
{
+ if (is_gimple_debug (stmt))
+ continue;
gimple_assign_set_rhs_code (stmt, MULT_EXPR);
fold_stmt_inplace (stmt);
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