On Thu, Aug 4, 2016 at 1:48 PM, Richard Biener <richard.guent...@gmail.com> wrote: > On Thu, Aug 4, 2016 at 10:40 AM, Bin.Cheng <amker.ch...@gmail.com> wrote: >> On Wed, Aug 3, 2016 at 11:17 PM, Jeff Law <l...@redhat.com> wrote: >>> On 08/03/2016 10:35 AM, Bin Cheng wrote: >>>> >>>> Hi, >>>> When I introduced parameter STOP for expand_simple_operations, I also >>>> added it for simplify_using_initial_conditions. The STOP argument is also >>>> passed to simplify_using_initial_conditions in >>>> simple_iv_with_niters/loop_exits_before_overflow. After analyzing case >>>> reported by PR72772, I think STOP expanding is only needed for >>>> expand_simple_operations when handling IV.step in tree-ssa-loop-ivopts.c. >>>> For other cases like calls to simplify_using_initial_condition, both cond >>>> and expr should be expanded to check tree expression equality. This patch >>>> does so. It simplifies interface by removing parameter STOP, also moves >>>> expand_simple_operations from tree_simplify_using_condition_1 to its >>>> caller. >>>> >>>> Bootstrap and test on x86_64 and AArch64. Is it OK? >>>> >>>> Thanks, >>>> bin >>>> >>>> 2016-08-02 Bin Cheng <bin.ch...@arm.com> >>>> >>>> PR tree-optimization/72772 >>>> * tree-ssa-loop-niter.h (simplify_using_initial_conditions): >>>> Delete >>>> parameter STOP. >>>> * tree-ssa-loop-niter.c (tree_simplify_using_condition_1): Delete >>>> parameter STOP and update calls. Move expand_simple_operations >>>> function call from here... >>>> (simplify_using_initial_conditions): ...to here. Delete parameter >>>> STOP. >>>> (tree_simplify_using_condition): Delete parameter STOP. >>>> * tree-scalar-evolution.c (simple_iv_with_niters): Update call to >>>> simplify_using_initial_conditions. >>>> >>> OK. >>> jeff >> >> Thanks for reviewing. Now I have a question about behavior of the >> interface. Although by expanding both cond and expr, this patch >> catches more equality cases, it always returns expanded expr even it's >> not simplified, while the original behavior only returns simplified >> expr (not expanded). For most use cases, it doesn't matter because we >> only care if the simplified result is TRUE or FALSE, but in >> computation of niter->assumption and niter->may_be_zeor, we may result >> in different (expanded) expressions. Not sure how much this >> difference matters. I can work on another version patch keeping the >> old behavior if it worth keeping. > > It might result in additional redundant code to be generated when generating > versioning conditions from assumption or maybe_zero? So yes, I think Yes, that's the case it matters.
Hi Jeff, Richard, Attachment is updated patch preserving the old behavior. Bootstrap and test again. Is it OK? Thanks, bin
diff --git a/gcc/tree-scalar-evolution.c b/gcc/tree-scalar-evolution.c index 7c5cefd..b8bfe51 100644 --- a/gcc/tree-scalar-evolution.c +++ b/gcc/tree-scalar-evolution.c @@ -3484,7 +3484,7 @@ simple_iv_with_niters (struct loop *wrto_loop, struct loop *use_loop, bool allow_nonconstant_step) { enum tree_code code; - tree type, ev, base, e, stop; + tree type, ev, base, e; wide_int extreme; bool folded_casts, overflow; @@ -3601,8 +3601,7 @@ simple_iv_with_niters (struct loop *wrto_loop, struct loop *use_loop, return true; e = fold_build2 (code, boolean_type_node, base, wide_int_to_tree (type, extreme)); - stop = (TREE_CODE (base) == SSA_NAME) ? base : NULL; - e = simplify_using_initial_conditions (use_loop, e, stop); + e = simplify_using_initial_conditions (use_loop, e); if (!integer_zerop (e)) return true; diff --git a/gcc/tree-ssa-loop-niter.c b/gcc/tree-ssa-loop-niter.c index 191a071..5efba3b 100644 --- a/gcc/tree-ssa-loop-niter.c +++ b/gcc/tree-ssa-loop-niter.c @@ -1880,10 +1880,10 @@ expand_simple_operations (tree expr, tree stop) expression (or EXPR unchanged, if no simplification was possible). */ static tree -tree_simplify_using_condition_1 (tree cond, tree expr, tree stop) +tree_simplify_using_condition_1 (tree cond, tree expr) { bool changed; - tree e, te, e0, e1, e2, notcond; + tree e, e0, e1, e2, notcond; enum tree_code code = TREE_CODE (expr); if (code == INTEGER_CST) @@ -1895,17 +1895,17 @@ tree_simplify_using_condition_1 (tree cond, tree expr, tree stop) { changed = false; - e0 = tree_simplify_using_condition_1 (cond, TREE_OPERAND (expr, 0), stop); + e0 = tree_simplify_using_condition_1 (cond, TREE_OPERAND (expr, 0)); if (TREE_OPERAND (expr, 0) != e0) changed = true; - e1 = tree_simplify_using_condition_1 (cond, TREE_OPERAND (expr, 1), stop); + e1 = tree_simplify_using_condition_1 (cond, TREE_OPERAND (expr, 1)); if (TREE_OPERAND (expr, 1) != e1) changed = true; if (code == COND_EXPR) { - e2 = tree_simplify_using_condition_1 (cond, TREE_OPERAND (expr, 2), stop); + e2 = tree_simplify_using_condition_1 (cond, TREE_OPERAND (expr, 2)); if (TREE_OPERAND (expr, 2) != e2) changed = true; } @@ -1968,16 +1968,14 @@ tree_simplify_using_condition_1 (tree cond, tree expr, tree stop) return boolean_true_node; } - te = expand_simple_operations (expr, stop); - /* Check whether COND ==> EXPR. */ notcond = invert_truthvalue (cond); - e = fold_binary (TRUTH_OR_EXPR, boolean_type_node, notcond, te); + e = fold_binary (TRUTH_OR_EXPR, boolean_type_node, notcond, expr); if (e && integer_nonzerop (e)) return e; /* Check whether COND ==> not EXPR. */ - e = fold_binary (TRUTH_AND_EXPR, boolean_type_node, cond, te); + e = fold_binary (TRUTH_AND_EXPR, boolean_type_node, cond, expr); if (e && integer_zerop (e)) return e; @@ -1992,11 +1990,11 @@ tree_simplify_using_condition_1 (tree cond, tree expr, tree stop) the loop do not cause us to fail. */ static tree -tree_simplify_using_condition (tree cond, tree expr, tree stop) +tree_simplify_using_condition (tree cond, tree expr) { - cond = expand_simple_operations (cond, stop); + cond = expand_simple_operations (cond); - return tree_simplify_using_condition_1 (cond, expr, stop); + return tree_simplify_using_condition_1 (cond, expr); } /* Tries to simplify EXPR using the conditions on entry to LOOP. @@ -2004,17 +2002,19 @@ tree_simplify_using_condition (tree cond, tree expr, tree stop) simplification was possible). */ tree -simplify_using_initial_conditions (struct loop *loop, tree expr, tree stop) +simplify_using_initial_conditions (struct loop *loop, tree expr) { edge e; basic_block bb; gimple *stmt; - tree cond; + tree cond, expanded, backup; int cnt = 0; if (TREE_CODE (expr) == INTEGER_CST) return expr; + backup = expanded = expand_simple_operations (expr); + /* Limit walking the dominators to avoid quadraticness in the number of BBs times the number of loops in degenerate cases. */ @@ -2036,15 +2036,17 @@ simplify_using_initial_conditions (struct loop *loop, tree expr, tree stop) gimple_cond_rhs (stmt)); if (e->flags & EDGE_FALSE_VALUE) cond = invert_truthvalue (cond); - expr = tree_simplify_using_condition (cond, expr, stop); + expanded = tree_simplify_using_condition (cond, expanded); /* Break if EXPR is simplified to const values. */ - if (expr && (integer_zerop (expr) || integer_nonzerop (expr))) - break; + if (expanded + && (integer_zerop (expanded) || integer_nonzerop (expanded))) + return expanded; ++cnt; } - return expr; + /* Return the original expression if no simplification is done. */ + return operand_equal_p (backup, expanded, 0) ? expr : expanded; } /* Tries to simplify EXPR using the evolutions of the loop invariants @@ -4150,8 +4152,6 @@ loop_exits_before_overflow (tree base, tree step, constant step because otherwise we don't have the information. */ if (TREE_CODE (step) == INTEGER_CST) { - tree stop = (TREE_CODE (base) == SSA_NAME) ? base : NULL; - for (civ = loop->control_ivs; civ; civ = civ->next) { enum tree_code code; @@ -4209,7 +4209,7 @@ loop_exits_before_overflow (tree base, tree step, } extreme = fold_build2 (MINUS_EXPR, type, extreme, step); e = fold_build2 (code, boolean_type_node, base, extreme); - e = simplify_using_initial_conditions (loop, e, stop); + e = simplify_using_initial_conditions (loop, e); if (integer_zerop (e)) return true; } diff --git a/gcc/tree-ssa-loop-niter.h b/gcc/tree-ssa-loop-niter.h index f5e2259..e6eebd9 100644 --- a/gcc/tree-ssa-loop-niter.h +++ b/gcc/tree-ssa-loop-niter.h @@ -21,8 +21,7 @@ along with GCC; see the file COPYING3. If not see #define GCC_TREE_SSA_LOOP_NITER_H extern tree expand_simple_operations (tree, tree = NULL); -extern tree simplify_using_initial_conditions (struct loop *, - tree, tree = NULL); +extern tree simplify_using_initial_conditions (struct loop *, tree); extern bool loop_only_exit_p (const struct loop *, const_edge); extern bool number_of_iterations_exit (struct loop *, edge, struct tree_niter_desc *niter, bool,