| Message ID | alpine.LSU.2.11.1410140948110.20733@zhemvz.fhfr.qr |
|---|---|
| State | New |
| Headers | show |
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.
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. */ + +static 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; } 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))