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,
