[PATCH GCC]Simplify interface for simplify_using_initial_conditions

[PATCH GCC]Simplify interface for simplify_using_initial_conditions

[Bin Cheng]

[-- Attachment #1: Type: text/plain, Size: 1345 bytes --]

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.cheng@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.

[-- Attachment #2: always-expand-cond-expr-20160801.txt --]
[-- Type: text/plain, Size: 6017 bytes --]

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 b7d7c32..7690f2f 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,7 +2002,7 @@ 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;
@@ -2015,6 +2013,8 @@ simplify_using_initial_conditions (struct loop *loop, tree expr, tree stop)
   if (TREE_CODE (expr) == INTEGER_CST)
     return expr;
 
+  expr = 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,7 +2036,7 @@ 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);
+      expr = tree_simplify_using_condition (cond, expr);
       /* Break if EXPR is simplified to const values.  */
       if (expr && (integer_zerop (expr) || integer_nonzerop (expr)))
 	break;
@@ -4138,8 +4138,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;
@@ -4197,7 +4200,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,

Re: [PATCH GCC]Simplify interface for simplify_using_initial_conditions

[Jeff Law]

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.cheng@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

Re: [PATCH GCC]Simplify interface for simplify_using_initial_conditions

[Bin.Cheng]

On Wed, Aug 3, 2016 at 11:17 PM, Jeff Law <law@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.cheng@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.

Thanks,
bin

Re: [PATCH GCC]Simplify interface for simplify_using_initial_conditions

[Richard Biener]

On Thu, Aug 4, 2016 at 10:40 AM, Bin.Cheng <amker.cheng@gmail.com> wrote:
> On Wed, Aug 3, 2016 at 11:17 PM, Jeff Law <law@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.cheng@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
the old behavior is worth preserving.

Richard.

> Thanks,
> bin

Re: [PATCH GCC]Simplify interface for simplify_using_initial_conditions

[Bin.Cheng]

[-- Attachment #1: Type: text/plain, Size: 2836 bytes --]

On Thu, Aug 4, 2016 at 1:48 PM, Richard Biener
<richard.guenther@gmail.com> wrote:
> On Thu, Aug 4, 2016 at 10:40 AM, Bin.Cheng <amker.cheng@gmail.com> wrote:
>> On Wed, Aug 3, 2016 at 11:17 PM, Jeff Law <law@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.cheng@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

[-- Attachment #2: always-expand-cond-expr-20160803.txt --]
[-- Type: text/plain, Size: 6385 bytes --]

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,

Re: [PATCH GCC]Simplify interface for simplify_using_initial_conditions

[Jeff Law]

On 08/05/2016 08:53 AM, Bin.Cheng wrote:
> On Thu, Aug 4, 2016 at 1:48 PM, Richard Biener
> <richard.guenther@gmail.com> wrote:
>> On Thu, Aug 4, 2016 at 10:40 AM, Bin.Cheng <amker.cheng@gmail.com> wrote:
>>> On Wed, Aug 3, 2016 at 11:17 PM, Jeff Law <law@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.cheng@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?
OK with a ChangeLog.

Thanks,
jeff