On Tue, 14 Oct 2014, Richard Biener wrote:
> On Tue, 14 Oct 2014, Richard Biener wrote:
>
> >
> > This changes default behavior of fold_stmt back to _not_ following
> > SSA use-def chains when trying to simplify things. I had to force
> > that already for one caller and for the merge to trunk I'd rather
> > not track down issues in every other existing caller.
> >
> > This means that fold_stmt will not become more powerful, at least for now.
> > I still hope to get rid of its use of fold() during the merge process.
> >
> > Bootstrap and regtest ongoing on x86_64-unknown-linux-gnu.
> >
> > (yeah, I'm preparing a first batch of changes to merge from the
> > branch)
>
> Unfortunately this exposes an issue with combining our SSA propagators
> with pattern matching which makes us miscompile tree-vect-generic.c
> from VRP. Consider
>
> Visiting PHI node: i_137 = PHI <0(51), i_48(63)>
> Argument #0 (51 -> 52 executable)
> 0: [0, 0]
> Argument #1 (63 -> 52 not executable)
> Found new range for i_137: [0, 0]
> ...
> i_48 = delta_25 + i_137;
> Found new range for i_48: VARYING
> _67 = (unsigned int) delta_25;
> Found new range for _67: [0, +INF]
> _78 = (unsigned int) i_48;
> Found new range for _78: [0, +INF]
> _257 = _78 - _67;
> (unsigned int) (delta_25 + i_137) - (unsigned int) delta_25
> Match-and-simplified _78 - _67 to 0
> Found new range for _257: [0, 0]
>
> now after i_137 is revisited and it becomes VARYING the SSA propagator
> stops at i_48 because its value does not change. Thus it fails to
> re-visit _257 where a pattern was applied that used the optimistic
> value of i_137 to its advantage.
>
> The following patch makes sure SSA propagators (CCP and VRP) do
> not get any benefit during their propagation phase from
> match-and-simplify by disabling the following of SSA use-def edges.
>
> Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
>
> Richard.
And here is the patch.
Richard.
2014-10-14 Richard Biener <rguent...@suse.de>
* gimple-fold.h (no_follow_ssa_edges): Declare.
(gimple_fold_stmt_to_constant_1): Add separate valueize hook for
gimple_simplify, defaulted to no_follow_ssa_edges.
* gimple-fold.c (fold_stmt): Make old API never follow SSA edges
when simplifying.
(no_follow_ssa_edges): New function.
(gimple_fold_stmt_to_constant_1): Adjust.
* tree-cfg.c (no_follow_ssa_edges): Remove.
(replace_uses_by): Use plain fold_stmt again.
* gimple-match-head.c (gimple_simplify): When simplifying
a statement do not stop when valueizing its operands yields NULL.
Index: gcc/gimple-fold.h
===================================================================
--- gcc/gimple-fold.h (revision 216146)
+++ gcc/gimple-fold.h (working copy)
@@ -32,7 +32,9 @@ extern tree maybe_fold_and_comparisons (
enum tree_code, tree, tree);
extern tree maybe_fold_or_comparisons (enum tree_code, tree, tree,
enum tree_code, tree, tree);
-extern tree gimple_fold_stmt_to_constant_1 (gimple, tree (*) (tree));
+extern tree no_follow_ssa_edges (tree);
+extern tree gimple_fold_stmt_to_constant_1 (gimple, tree (*) (tree),
+ tree (*) (tree) =
no_follow_ssa_edges);
extern tree gimple_fold_stmt_to_constant (gimple, tree (*) (tree));
extern tree fold_const_aggregate_ref_1 (tree, tree (*) (tree));
extern tree fold_const_aggregate_ref (tree);
Index: gcc/gimple-fold.c
===================================================================
--- gcc/gimple-fold.c (revision 216146)
+++ gcc/gimple-fold.c (working copy)
@@ -3136,6 +3136,14 @@ fail:
return changed;
}
+/* Valueziation callback that ends up not following SSA edges. */
+
+tree
+no_follow_ssa_edges (tree)
+{
+ return NULL_TREE;
+}
+
/* Fold the statement pointed to by GSI. In some cases, this function may
replace the whole statement with a new one. Returns true iff folding
makes any changes.
@@ -3146,7 +3154,7 @@ fail:
bool
fold_stmt (gimple_stmt_iterator *gsi)
{
- return fold_stmt_1 (gsi, false, NULL);
+ return fold_stmt_1 (gsi, false, no_follow_ssa_edges);
}
bool
@@ -3167,7 +3175,7 @@ bool
fold_stmt_inplace (gimple_stmt_iterator *gsi)
{
gimple stmt = gsi_stmt (*gsi);
- bool changed = fold_stmt_1 (gsi, true, NULL);
+ bool changed = fold_stmt_1 (gsi, true, no_follow_ssa_edges);
gcc_assert (gsi_stmt (*gsi) == stmt);
return changed;
}
@@ -4527,12 +4535,19 @@ gimple_fold_stmt_to_constant_2 (gimple s
}
}
+/* ??? The SSA propagators do not correctly deal with following SSA use-def
+ edges if there are intermediate VARYING defs. For this reason
+ there are two valueization hooks here, one for the legacy code
+ in gimple_fold_stmt_to_constant_2 and one for gimple_simplify
+ which is defaulted to no_follow_ssa_edges. */
+
tree
-gimple_fold_stmt_to_constant_1 (gimple stmt, tree (*valueize) (tree))
+gimple_fold_stmt_to_constant_1 (gimple stmt, tree (*valueize) (tree),
+ tree (*gvalueize) (tree))
{
code_helper rcode;
tree ops[3] = {};
- if (gimple_simplify (stmt, &rcode, ops, NULL, valueize)
+ if (gimple_simplify (stmt, &rcode, ops, NULL, gvalueize)
&& rcode.is_tree_code ()
&& (TREE_CODE_LENGTH ((tree_code) rcode) == 0
|| ((tree_code) rcode) == ADDR_EXPR)
Index: gcc/tree-cfg.c
===================================================================
--- gcc/tree-cfg.c (revision 216146)
+++ gcc/tree-cfg.c (working copy)
@@ -1709,14 +1709,6 @@ gimple_can_merge_blocks_p (basic_block a
return true;
}
-/* ??? Maybe this should be a generic overload of fold_stmt. */
-
-static tree
-no_follow_ssa_edges (tree)
-{
- return NULL_TREE;
-}
-
/* Replaces all uses of NAME by VAL. */
void
@@ -1773,17 +1765,7 @@ replace_uses_by (tree name, tree val)
recompute_tree_invariant_for_addr_expr (op);
}
- /* If we have sth like
- neighbor_29 = <name> + -1;
- _33 = <name> + neighbor_29;
- and substitute 1 for <name> then when visiting
- _33 first then folding will simplify the stmt
- to _33 = <name>; and the new immediate use will
- be inserted before the stmt iterator marker and
- thus we fail to visit it again, ICEing within the
- has_zero_uses assert.
- Avoid that by never following SSA edges. */
- if (fold_stmt (&gsi, no_follow_ssa_edges))
+ if (fold_stmt (&gsi))
stmt = gsi_stmt (gsi);
if (maybe_clean_or_replace_eh_stmt (orig_stmt, stmt))
Index: gcc/gimple-match-head.c
===================================================================
--- gcc/gimple-match-head.c (revision 216146)
+++ gcc/gimple-match-head.c (working copy)
@@ -595,9 +595,9 @@ gimple_simplify (gimple stmt,
tree op0 = TREE_OPERAND (gimple_assign_rhs1 (stmt), 0);
if (valueize && TREE_CODE (op0) == SSA_NAME)
{
- op0 = valueize (op0);
- if (!op0)
- return false;
+ tree tem = valueize (op0);
+ if (tem)
+ op0 = tem;
}
*rcode = code;
ops[0] = op0;
@@ -609,9 +609,9 @@ gimple_simplify (gimple stmt,
tree op0 = TREE_OPERAND (rhs1, 0);
if (valueize && TREE_CODE (op0) == SSA_NAME)
{
- op0 = valueize (op0);
- if (!op0)
- return false;
+ tree tem = valueize (op0);
+ if (tem)
+ op0 = tem;
}
*rcode = code;
ops[0] = op0;
@@ -636,9 +636,9 @@ gimple_simplify (gimple stmt,
tree rhs1 = gimple_assign_rhs1 (stmt);
if (valueize && TREE_CODE (rhs1) == SSA_NAME)
{
- rhs1 = valueize (rhs1);
- if (!rhs1)
- return false;
+ tree tem = valueize (rhs1);
+ if (tem)
+ rhs1 = tem;
}
*rcode = code;
ops[0] = rhs1;
@@ -649,16 +649,16 @@ gimple_simplify (gimple stmt,
tree rhs1 = gimple_assign_rhs1 (stmt);
if (valueize && TREE_CODE (rhs1) == SSA_NAME)
{
- rhs1 = valueize (rhs1);
- if (!rhs1)
- return false;
+ tree tem = valueize (rhs1);
+ if (tem)
+ rhs1 = tem;
}
tree rhs2 = gimple_assign_rhs2 (stmt);
if (valueize && TREE_CODE (rhs2) == SSA_NAME)
{
- rhs2 = valueize (rhs2);
- if (!rhs2)
- return false;
+ tree tem = valueize (rhs2);
+ if (tem)
+ rhs2 = tem;
}
*rcode = code;
ops[0] = rhs1;
@@ -670,23 +670,23 @@ gimple_simplify (gimple stmt,
tree rhs1 = gimple_assign_rhs1 (stmt);
if (valueize && TREE_CODE (rhs1) == SSA_NAME)
{
- rhs1 = valueize (rhs1);
- if (!rhs1)
- return false;
+ tree tem = valueize (rhs1);
+ if (tem)
+ rhs1 = tem;
}
tree rhs2 = gimple_assign_rhs2 (stmt);
if (valueize && TREE_CODE (rhs2) == SSA_NAME)
{
- rhs2 = valueize (rhs2);
- if (!rhs2)
- return false;
+ tree tem = valueize (rhs2);
+ if (tem)
+ rhs2 = tem;
}
tree rhs3 = gimple_assign_rhs3 (stmt);
if (valueize && TREE_CODE (rhs3) == SSA_NAME)
{
- rhs3 = valueize (rhs3);
- if (!rhs3)
- return false;
+ tree tem = valueize (rhs3);
+ if (tem)
+ rhs3 = tem;
}
*rcode = code;
ops[0] = rhs1;
@@ -708,9 +708,12 @@ gimple_simplify (gimple stmt,
/* ??? Internal function support missing. */
if (!fn)
return false;
- if (TREE_CODE (fn) == SSA_NAME
- && valueize)
- fn = valueize (fn);
+ if (valueize && TREE_CODE (fn) == SSA_NAME)
+ {
+ tree tem = valueize (fn);
+ if (tem)
+ fn = tem;
+ }
if (!fn
|| TREE_CODE (fn) != ADDR_EXPR
|| TREE_CODE (TREE_OPERAND (fn, 0)) != FUNCTION_DECL
@@ -729,9 +732,9 @@ gimple_simplify (gimple stmt,
tree arg1 = gimple_call_arg (stmt, 0);
if (valueize && TREE_CODE (arg1) == SSA_NAME)
{
- arg1 = valueize (arg1);
- if (!arg1)
- return false;
+ tree tem = valueize (arg1);
+ if (tem)
+ arg1 = tem;
}
*rcode = DECL_FUNCTION_CODE (decl);
ops[0] = arg1;
@@ -742,16 +745,16 @@ gimple_simplify (gimple stmt,
tree arg1 = gimple_call_arg (stmt, 0);
if (valueize && TREE_CODE (arg1) == SSA_NAME)
{
- arg1 = valueize (arg1);
- if (!arg1)
- return false;
+ tree tem = valueize (arg1);
+ if (tem)
+ arg1 = tem;
}
tree arg2 = gimple_call_arg (stmt, 1);
if (valueize && TREE_CODE (arg2) == SSA_NAME)
{
- arg2 = valueize (arg2);
- if (!arg2)
- return false;
+ tree tem = valueize (arg2);
+ if (tem)
+ arg2 = tem;
}
*rcode = DECL_FUNCTION_CODE (decl);
ops[0] = arg1;
@@ -763,23 +766,23 @@ gimple_simplify (gimple stmt,
tree arg1 = gimple_call_arg (stmt, 0);
if (valueize && TREE_CODE (arg1) == SSA_NAME)
{
- arg1 = valueize (arg1);
- if (!arg1)
- return false;
+ tree tem = valueize (arg1);
+ if (tem)
+ arg1 = tem;
}
tree arg2 = gimple_call_arg (stmt, 1);
if (valueize && TREE_CODE (arg2) == SSA_NAME)
{
- arg2 = valueize (arg2);
- if (!arg2)
- return false;
+ tree tem = valueize (arg2);
+ if (tem)
+ arg2 = tem;
}
tree arg3 = gimple_call_arg (stmt, 2);
if (valueize && TREE_CODE (arg3) == SSA_NAME)
{
- arg3 = valueize (arg3);
- if (!arg3)
- return false;
+ tree tem = valueize (arg3);
+ if (tem)
+ arg3 = tem;
}
*rcode = DECL_FUNCTION_CODE (decl);
ops[0] = arg1;
@@ -798,16 +801,16 @@ gimple_simplify (gimple stmt,
tree lhs = gimple_cond_lhs (stmt);
if (valueize && TREE_CODE (lhs) == SSA_NAME)
{
- lhs = valueize (lhs);
- if (!lhs)
- return false;
+ tree tem = valueize (lhs);
+ if (tem)
+ lhs = tem;
}
tree rhs = gimple_cond_rhs (stmt);
if (valueize && TREE_CODE (rhs) == SSA_NAME)
{
- rhs = valueize (rhs);
- if (!rhs)
- return false;
+ tree tem = valueize (rhs);
+ if (tem)
+ rhs = tem;
}
*rcode = gimple_cond_code (stmt);
ops[0] = lhs;
