Re: [PATCH][middle-end/88784] Middle end is missing some optimizations about unsigned

[Andrew Pinski <pinskia@gmail.com>]

On Thu, Jun 27, 2019 at 9:55 PM Li Jia He <helijia@linux.ibm.com> wrote:
>
>
>
> On 2019/6/27 11:48 PM, Jeff Law wrote:
> > On 6/27/19 12:11 AM, Li Jia He wrote:
> >> 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-06-27  Li Jia He  <helijia@linux.ibm.com>
> >>          Qi Feng  <ffengqi@linux.ibm.com>
> >>
> >>      PR middle-end/88784
> >>      * gimple-fold.c (and_comparisons_contain_equal_operands): New function.
> >>      (and_comparisons_1): Use and_comparisons_contain_equal_operands.
> >>      (or_comparisons_contain_equal_operands): New function.
> >>      (or_comparisons_1): Use or_comparisons_contain_equal_operands.
> > Would this be better done via match.pd?  ISTM this transformation would
> > be well suited for that framework.
>
> Hi, Jeff
>
> I did this because of the following test case:
> `
> _Bool comp(unsigned x, unsigned y)
> {
>    return x > y && x != 0;
> }
> `
> The gimple file dumped on the power platform is:
> `
> comp (unsigned int x, unsigned int y)
> {
>    _Bool D.2837;
>    int iftmp.0;
>
>    if (x > y) goto <D.2841>; else goto <D.2839>;
>    <D.2841>:
>    if (x != 0) goto <D.2842>; else goto <D.2839>;
>    <D.2842>:
>    iftmp.0 = 1;
>    goto <D.2840>;
>    <D.2839>:
>    iftmp.0 = 0;
>    <D.2840>:
>    D.2837 = (_Bool) iftmp.0;
>    return D.2837;
> }
> `
> However, the gimple file dumped on x86 is
> `
> comp (unsigned int x, unsigned int y)
> {
>    _Bool D.2837;
>
>    _1 = x > y;
>    _2 = x != 0;
>    _3 = _1 & _2;
>    _4 = (int) _3;
>    D.2837 = (_Bool) _4;
>    return D.2837;
> }
> `
>
> The reason for the inconsistency between these two behaviors is param
> logical-op-non-short-circuit.  If we add the pattern to the match.pd
> file, we can only optimize the situation in which the statement is in
> the same basic block (logical-op-non-short-circuit=1, x86).  But for
> a cross-basic block (logical-op-non-short-circuit=0, power), match.pd
> can't handle this situation.
>
> Another reason is that I found out maybe_fold_and_comparisons and
> maybe_fold_or_comparisons are not only called by ifcombine pass but
> also by reassoc pass. Using this method can basically unify param
> logical-op-non-short-circuit=0 or 1.


As mentioned before ifcombine pass should be using gimple-match
instead of fold_build.  Try converting ifcombine over to gimple-match
infrastructure and add these to match.pd.
NOTE tree-ssa-phiopt should also be moved over to gimple-match but
that is a different issue.


Thanks,
Andrew Pinski

>
> Thanks,
> Lijia He
>
> >
> > jeff
> >
>