Re: [PATCH 2/2] Fix PR88784, middle end is missing some optimizations about unsigned
On 9/11/19 3:08 PM, Richard Biener wrote:
> On Fri, 6 Sep 2019, Martin Liška wrote:
>
>> On 9/5/19 3:17 PM, Richard Biener wrote:
>>> On Tue, 16 Jul 2019, Li Jia He wrote:
>>>
>>>> Hi,
>>>>
>>>> I made some changes based on the recommendations. Would you like to
>>>> help me to see it again ? Sorry, it took so long time to provide the
>>>> patch.
>>>>
>>>> Note: 1. I keep the code for and_comparisons_1 and or_comparisons_1.
>>>>The reason is that I did not found a good way to handle the
>>>>optimization of '((x CODE1 y) AND (x CODE2 y))' in match.pd.
>>>>Maybe I missing some important information about match.pd.
>>>>2. The gimple_resimplify2 function is not used. Since stmt1,
>>>>stmt2, lhs1 and lhs2 are allocated on the stack, Is there a
>>>>question with the value on the stack as the return value ?
>>>>I may have misunderstood Richard's intention.
>>>
>>> And now for the match.pd patch.
>>>
>>> +/* x > y && x != XXX_MIN --> x > y */
>>> +(for and (truth_and bit_and)
>>> + (simplify
>>> + (and:c (gt:c@3 @0 @1) (ne @0 INTEGER_CST@2))
>>> + (if (INTEGRAL_TYPE_P (TREE_TYPE (@0)) && INTEGRAL_TYPE_P
>>> (TREE_TYPE(@1))
>>> + && (wi::eq_p (wi::to_wide (@2), wi::min_value (TREE_TYPE (@2)
>>> +@3)))
>>> +
>>> +/* x > y && x == XXX_MIN --> false */
>>> +(for and (truth_and bit_and)
>>> + (simplify
>>> + (and:c (gt:c @0 @1) (eq @0 INTEGER_CST@2))
>>> + (if (INTEGRAL_TYPE_P (TREE_TYPE (@0)) && INTEGRAL_TYPE_P
>>> (TREE_TYPE(@1))
>>> + && (wi::eq_p (wi::to_wide (@2), wi::min_value (TREE_TYPE (@2)
>>> +{ boolean_false_node; })))
>>>
>>> you could merge those two via
>>>
>>> (for eqne (eq ne)
>>> (for and (
>>>(simplify
>>> (and:c (gt:c @0 @1) (eqne @0 INTEGER_CST@2))
>>> (if (...)
>>> (switch
>>> (if (eqne == NE_EXPR)
>>>@3)
>>> (if (eqne == EQ_EXPR)
>>>{ constant_boolean_node (false, type); }
>>>
>>> notice using constant_boolean_node (false, type); instead of
>>> boolean_false_node. I suspect more unification is possible.
>>>
>>> Also you could do
>>>
>>> (match min_value
>>> INTEGER_CST
>>> (if (INTEGRAL_TYPE_P (type)
>>> && wi::eq_p (wi::to_wide (t), wi::min_value (type)
>>>
>>> and then write
>>>
>>> (simplify
>>> (and:c (gt:c @0 @1) (eq @0 min_value))
>>> (...
>>>
>>> Your
>>>
>>> (if (INTEGRAL_TYPE_P (TREE_TYPE (@0)) && INTEGRAL_TYPE_P
>>> (TREE_TYPE(@1))
>>>
>>> is redundant, it's enough to check either @0 or @1 given they
>>> have to be compatible for the gt operation. Note you probably
>>> want to use
>>>
>>> (and:c (gt:c @0 @1) (eq @@0 min_value))
>>>
>>> and verify that types_match (@1, @0) because when @0 are a constant
>>> (and (eq @0 min_value) is not folded which can happen) then they
>>> might have different types and thus you could have
>>> (SHORT_MAX > intvar) && (SHORT_MAX == SHORT_MAX)
>>>
>>> That said, the patterns can be quite a bit simplified I think.
>>>
>>> Richard.
>>>
>>
>> Likewise, I applied the suggested simplification.
>>
>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>>
>> Ready to be installed?
>
> +/* { dg-options "-O2 -fdump-tree-ifcombine --param
> logical-op-non-short-circuit=0" } */
> +
> +#include
> +
> +_Bool and1(unsigned x, unsigned y)
> +{
> + /* x > y && x != 0 --> x > y */
> + return x > y && x != 0;
> +}
> ...
> +/* { dg-final { scan-tree-dump-not " != " "ifcombine" } } */
>
> since you iterate over bit_and and truth_and GENERIC folding should
> already simplify this, no?
>
> I suggest to remove the truth_and/or iteration.
Done in updated patch.
Martin
>
> Otherwise looks OK now.
>
> Richard.
>
>From acb5e62c2babf749b60c5475925ac6afb059e459 Mon Sep 17 00:00:00 2001
From: Li Jia He
Date: Mon, 15 Jul 2019 03:50:34 -0500
Subject: [PATCH 2/5] Fix PR88784, middle end is missing some optimizations
about unsigne
Re: [PATCH 2/2] Fix PR88784, middle end is missing some optimizations about unsigned
On Fri, 6 Sep 2019, Martin Liška wrote:
> On 9/5/19 3:17 PM, Richard Biener wrote:
> > On Tue, 16 Jul 2019, Li Jia He wrote:
> >
> >> Hi,
> >>
> >> I made some changes based on the recommendations. Would you like to
> >> help me to see it again ? Sorry, it took so long time to provide the
> >> patch.
> >>
> >> Note: 1. I keep the code for and_comparisons_1 and or_comparisons_1.
> >>The reason is that I did not found a good way to handle the
> >>optimization of '((x CODE1 y) AND (x CODE2 y))' in match.pd.
> >>Maybe I missing some important information about match.pd.
> >>2. The gimple_resimplify2 function is not used. Since stmt1,
> >>stmt2, lhs1 and lhs2 are allocated on the stack, Is there a
> >>question with the value on the stack as the return value ?
> >>I may have misunderstood Richard's intention.
> >
> > And now for the match.pd patch.
> >
> > +/* x > y && x != XXX_MIN --> x > y */
> > +(for and (truth_and bit_and)
> > + (simplify
> > + (and:c (gt:c@3 @0 @1) (ne @0 INTEGER_CST@2))
> > + (if (INTEGRAL_TYPE_P (TREE_TYPE (@0)) && INTEGRAL_TYPE_P
> > (TREE_TYPE(@1))
> > + && (wi::eq_p (wi::to_wide (@2), wi::min_value (TREE_TYPE (@2)
> > +@3)))
> > +
> > +/* x > y && x == XXX_MIN --> false */
> > +(for and (truth_and bit_and)
> > + (simplify
> > + (and:c (gt:c @0 @1) (eq @0 INTEGER_CST@2))
> > + (if (INTEGRAL_TYPE_P (TREE_TYPE (@0)) && INTEGRAL_TYPE_P
> > (TREE_TYPE(@1))
> > + && (wi::eq_p (wi::to_wide (@2), wi::min_value (TREE_TYPE (@2)
> > +{ boolean_false_node; })))
> >
> > you could merge those two via
> >
> > (for eqne (eq ne)
> > (for and (
> >(simplify
> > (and:c (gt:c @0 @1) (eqne @0 INTEGER_CST@2))
> > (if (...)
> > (switch
> > (if (eqne == NE_EXPR)
> >@3)
> > (if (eqne == EQ_EXPR)
> >{ constant_boolean_node (false, type); }
> >
> > notice using constant_boolean_node (false, type); instead of
> > boolean_false_node. I suspect more unification is possible.
> >
> > Also you could do
> >
> > (match min_value
> > INTEGER_CST
> > (if (INTEGRAL_TYPE_P (type)
> > && wi::eq_p (wi::to_wide (t), wi::min_value (type)
> >
> > and then write
> >
> > (simplify
> > (and:c (gt:c @0 @1) (eq @0 min_value))
> > (...
> >
> > Your
> >
> > (if (INTEGRAL_TYPE_P (TREE_TYPE (@0)) && INTEGRAL_TYPE_P
> > (TREE_TYPE(@1))
> >
> > is redundant, it's enough to check either @0 or @1 given they
> > have to be compatible for the gt operation. Note you probably
> > want to use
> >
> > (and:c (gt:c @0 @1) (eq @@0 min_value))
> >
> > and verify that types_match (@1, @0) because when @0 are a constant
> > (and (eq @0 min_value) is not folded which can happen) then they
> > might have different types and thus you could have
> > (SHORT_MAX > intvar) && (SHORT_MAX == SHORT_MAX)
> >
> > That said, the patterns can be quite a bit simplified I think.
> >
> > Richard.
> >
>
> Likewise, I applied the suggested simplification.
>
> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>
> Ready to be installed?
+/* { dg-options "-O2 -fdump-tree-ifcombine --param
logical-op-non-short-circuit=0" } */
+
+#include
+
+_Bool and1(unsigned x, unsigned y)
+{
+ /* x > y && x != 0 --> x > y */
+ return x > y && x != 0;
+}
...
+/* { dg-final { scan-tree-dump-not " != " "ifcombine" } } */
since you iterate over bit_and and truth_and GENERIC folding should
already simplify this, no?
I suggest to remove the truth_and/or iteration.
Otherwise looks OK now.
Richard.
[PATCH 2/2] Fix PR88784, middle end is missing some optimizations about unsigned
On 9/5/19 3:17 PM, Richard Biener wrote:
> On Tue, 16 Jul 2019, Li Jia He wrote:
>
>> Hi,
>>
>> I made some changes based on the recommendations. Would you like to
>> help me to see it again ? Sorry, it took so long time to provide the
>> patch.
>>
>> Note: 1. I keep the code for and_comparisons_1 and or_comparisons_1.
>> The reason is that I did not found a good way to handle the
>> optimization of '((x CODE1 y) AND (x CODE2 y))' in match.pd.
>> Maybe I missing some important information about match.pd.
>> 2. The gimple_resimplify2 function is not used. Since stmt1,
>> stmt2, lhs1 and lhs2 are allocated on the stack, Is there a
>> question with the value on the stack as the return value ?
>> I may have misunderstood Richard's intention.
>
> And now for the match.pd patch.
>
> +/* x > y && x != XXX_MIN --> x > y */
> +(for and (truth_and bit_and)
> + (simplify
> + (and:c (gt:c@3 @0 @1) (ne @0 INTEGER_CST@2))
> + (if (INTEGRAL_TYPE_P (TREE_TYPE (@0)) && INTEGRAL_TYPE_P
> (TREE_TYPE(@1))
> + && (wi::eq_p (wi::to_wide (@2), wi::min_value (TREE_TYPE (@2)
> +@3)))
> +
> +/* x > y && x == XXX_MIN --> false */
> +(for and (truth_and bit_and)
> + (simplify
> + (and:c (gt:c @0 @1) (eq @0 INTEGER_CST@2))
> + (if (INTEGRAL_TYPE_P (TREE_TYPE (@0)) && INTEGRAL_TYPE_P
> (TREE_TYPE(@1))
> + && (wi::eq_p (wi::to_wide (@2), wi::min_value (TREE_TYPE (@2)
> +{ boolean_false_node; })))
>
> you could merge those two via
>
> (for eqne (eq ne)
> (for and (
>(simplify
> (and:c (gt:c @0 @1) (eqne @0 INTEGER_CST@2))
> (if (...)
> (switch
> (if (eqne == NE_EXPR)
>@3)
> (if (eqne == EQ_EXPR)
>{ constant_boolean_node (false, type); }
>
> notice using constant_boolean_node (false, type); instead of
> boolean_false_node. I suspect more unification is possible.
>
> Also you could do
>
> (match min_value
> INTEGER_CST
> (if (INTEGRAL_TYPE_P (type)
> && wi::eq_p (wi::to_wide (t), wi::min_value (type)
>
> and then write
>
> (simplify
> (and:c (gt:c @0 @1) (eq @0 min_value))
> (...
>
> Your
>
> (if (INTEGRAL_TYPE_P (TREE_TYPE (@0)) && INTEGRAL_TYPE_P
> (TREE_TYPE(@1))
>
> is redundant, it's enough to check either @0 or @1 given they
> have to be compatible for the gt operation. Note you probably
> want to use
>
> (and:c (gt:c @0 @1) (eq @@0 min_value))
>
> and verify that types_match (@1, @0) because when @0 are a constant
> (and (eq @0 min_value) is not folded which can happen) then they
> might have different types and thus you could have
> (SHORT_MAX > intvar) && (SHORT_MAX == SHORT_MAX)
>
> That said, the patterns can be quite a bit simplified I think.
>
> Richard.
>
Likewise, I applied the suggested simplification.
Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
Ready to be installed?
Thanks,
Martin
>From 1006e2cf69e97692e09e845eaadf55098d48f0e2 Mon Sep 17 00:00:00 2001
From: Li Jia He
Date: Mon, 15 Jul 2019 03:50:34 -0500
Subject: [PATCH 2/2] Fix PR88784, middle end is missing some optimizations
about unsigned
Hi,
According to the optimizable case described by Qi Feng on
issue 88784, we can combine the cases into the following:
1. x > y && x != XXX_MIN --> x > y
2. x > y && x == XXX_MIN --> false
3. x <= y && x == XXX_MIN --> x == XXX_MIN
4. x < y && x != XXX_MAX --> x < y
5. x < y && x == XXX_MAX --> false
6. x >= y && x == XXX_MAX --> x == XXX_MAX
7. x > y || x != XXX_MIN --> x != XXX_MIN
8. x <= y || x != XXX_MIN --> true
9. x <= y || x == XXX_MIN --> x <= y
10. x < y || x != XXX_MAX --> x != UXXX_MAX
11. x >= y || x != XXX_MAX --> true
12. x >= y || x == XXX_MAX --> x >= y
Note: XXX_MIN represents the minimum value of type x.
XXX_MAX represents the maximum value of type x.
Here we don't need to care about whether the operation is
signed or unsigned. For example, in the below equation:
'x > y && x != XXX_MIN --> x > y'
If the x type is signed int and XXX_MIN is INT_MIN, we can
optimize it to 'x > y'. However, if the type of x is unsigned
int and XXX_MIN is 0, we can still optimize it to 'x > y'.
The regression testing for the patch was done on GCC mainline on
powerpc64le-unknown-linux-gnu (Power 9 LE)
with no regressions. Is it OK for trunk ?
Thanks,
Lijia He
gcc/ChangeLog
2019-07-16
