diff mbox series

Use fold_build2 instead fold_binary for TRUTH_AND

Message ID 20211020022916.131643-1-guojiufu@linux.ibm.com
State New
Headers
Series Use fold_build2 instead fold_binary for TRUTH_AND |

Commit Message

Jiufu Guo Oct. 20, 2021, 2:29 a.m. UTC 
  In tree_simplify_using_condition_1, there is code which should be logic:
"op0 || op1"/"op0 && op1".  When creating expression for TRUTH_OR_EXPR
and TRUTH_AND_EXPR, fold_build2 would be used instead fold_binary which
always return NULL_TREE for this kind of expr.

Bootstrap and regtest pass on ppc and ppc64le.  Is this ok for trunk?

BR,
Jiufu

gcc/ChangeLog:

	* tree-ssa-loop-niter.c (tree_simplify_using_condition_1): Replace
	fold_binary with fold_build2 fir logical OR/AND.

---
 gcc/tree-ssa-loop-niter.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Andrew Pinski Oct. 20, 2021, 2:44 a.m. UTC | #1 
On Tue, Oct 19, 2021 at 7:30 PM Jiufu Guo via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> In tree_simplify_using_condition_1, there is code which should be logic:
> "op0 || op1"/"op0 && op1".  When creating expression for TRUTH_OR_EXPR
> and TRUTH_AND_EXPR, fold_build2 would be used instead fold_binary which
> always return NULL_TREE for this kind of expr.
>
> Bootstrap and regtest pass on ppc and ppc64le.  Is this ok for trunk?

No, because I think it is the wrong thing to do as we will be throwing
away the result if the fold_binary is not an integer cst anyways so
creating an extra tree is a waste.

>
> BR,
> Jiufu
>
> gcc/ChangeLog:
>
>         * tree-ssa-loop-niter.c (tree_simplify_using_condition_1): Replace
>         fold_binary with fold_build2 fir logical OR/AND.
>
> ---
>  gcc/tree-ssa-loop-niter.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/gcc/tree-ssa-loop-niter.c b/gcc/tree-ssa-loop-niter.c
> index 75109407124..27e11a29707 100644
> --- a/gcc/tree-ssa-loop-niter.c
> +++ b/gcc/tree-ssa-loop-niter.c
> @@ -2290,12 +2290,12 @@ tree_simplify_using_condition_1 (tree cond, tree expr)
>
>    /* Check whether COND ==> EXPR.  */
>    notcond = invert_truthvalue (cond);
> -  e = fold_binary (TRUTH_OR_EXPR, boolean_type_node, notcond, expr);
> +  e = fold_build2 (TRUTH_OR_EXPR, boolean_type_node, notcond, expr);
>    if (e && integer_nonzerop (e))
>      return e;

We already check for non-nullness and we also check to see it is an
integer which is nonzero. So building a tree which will be thrown away
is just a waste and all.

>
>    /* Check whether COND ==> not EXPR.  */
> -  e = fold_binary (TRUTH_AND_EXPR, boolean_type_node, cond, expr);
> +  e = fold_build2 (TRUTH_AND_EXPR, boolean_type_node, cond, expr);
>    if (e && integer_zerop (e))
>      return e;

Likewise.

Thanks,
Andrew Pinski

>
> --
> 2.17.1
>
Jiufu Guo Oct. 20, 2021, 5:24 a.m. UTC | #2 
On 2021-10-20 10:44, Andrew Pinski wrote:
> On Tue, Oct 19, 2021 at 7:30 PM Jiufu Guo via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
>> 
>> In tree_simplify_using_condition_1, there is code which should be 
>> logic:
>> "op0 || op1"/"op0 && op1".  When creating expression for TRUTH_OR_EXPR
>> and TRUTH_AND_EXPR, fold_build2 would be used instead fold_binary 
>> which
>> always return NULL_TREE for this kind of expr.
>> 
>> Bootstrap and regtest pass on ppc and ppc64le.  Is this ok for trunk?
> 
> No, because I think it is the wrong thing to do as we will be throwing
> away the result if the fold_binary is not an integer cst anyways so
> creating an extra tree is a waste.

Hi Andrew,

Thanks for your great comments!  I understand your explanation.  And 
there
is already non-nullness checking and zero/nonzero cst checking as you 
said.
I agree with you now :)  because if "op0 && op1"/"op0 || op1" is able to
be folded (especially folded into a cst nonzero/zero), fold_binary is 
enough.
And then, when fold_build2 creates a tree expr on code _AND_/_OR_, it 
should
not be a cst anymore.

BR,
Jiufu

> 
>> 
>> BR,
>> Jiufu
>> 
>> gcc/ChangeLog:
>> 
>>         * tree-ssa-loop-niter.c (tree_simplify_using_condition_1): 
>> Replace
>>         fold_binary with fold_build2 fir logical OR/AND.
>> 
>> ---
>>  gcc/tree-ssa-loop-niter.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>> 
>> diff --git a/gcc/tree-ssa-loop-niter.c b/gcc/tree-ssa-loop-niter.c
>> index 75109407124..27e11a29707 100644
>> --- a/gcc/tree-ssa-loop-niter.c
>> +++ b/gcc/tree-ssa-loop-niter.c
>> @@ -2290,12 +2290,12 @@ tree_simplify_using_condition_1 (tree cond, 
>> tree expr)
>> 
>>    /* Check whether COND ==> EXPR.  */
>>    notcond = invert_truthvalue (cond);
>> -  e = fold_binary (TRUTH_OR_EXPR, boolean_type_node, notcond, expr);
>> +  e = fold_build2 (TRUTH_OR_EXPR, boolean_type_node, notcond, expr);
>>    if (e && integer_nonzerop (e))
>>      return e;
> 
> We already check for non-nullness and we also check to see it is an
> integer which is nonzero. So building a tree which will be thrown away
> is just a waste and all.
> 
>> 
>>    /* Check whether COND ==> not EXPR.  */
>> -  e = fold_binary (TRUTH_AND_EXPR, boolean_type_node, cond, expr);
>> +  e = fold_build2 (TRUTH_AND_EXPR, boolean_type_node, cond, expr);
>>    if (e && integer_zerop (e))
>>      return e;
> 
> Likewise.
> 
> Thanks,
> Andrew Pinski
> 
>> 
>> --
>> 2.17.1
>>
diff mbox series

Patch

diff --git a/gcc/tree-ssa-loop-niter.c b/gcc/tree-ssa-loop-niter.c
index 75109407124..27e11a29707 100644
--- a/gcc/tree-ssa-loop-niter.c
+++ b/gcc/tree-ssa-loop-niter.c
@@ -2290,12 +2290,12 @@  tree_simplify_using_condition_1 (tree cond, tree expr)
 
   /* Check whether COND ==> EXPR.  */
   notcond = invert_truthvalue (cond);
-  e = fold_binary (TRUTH_OR_EXPR, boolean_type_node, notcond, expr);
+  e = fold_build2 (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, expr);
+  e = fold_build2 (TRUTH_AND_EXPR, boolean_type_node, cond, expr);
   if (e && integer_zerop (e))
     return e;