Re: [RFA][PATCH][PR rtl-optimization/47477] Type narrowing in match.pd

[Jeff Law <law at redhat dot com>]

On 02/11/15 03:56, Richard Biener wrote:
> > diff --git a/gcc/ChangeLog b/gcc/ChangeLog
> > index 7f3816c..7a95029 100644
> > --- a/gcc/ChangeLog
> > +++ b/gcc/ChangeLog
> > @@ -1,3 +1,8 @@
> > +2015-02-10  Jeff Law  <law@redhat.com>
> > +
> > +       * match.pd (convert (plus/minus (convert @0) (convert @1): New
> > +       simplifier to narrow arithmetic.
> > +
>
> Please reference PR47477 from here
Doh.  Fixed.


> >   2015-02-10  Richard Biener  <rguenther@suse.de>
> >
> >          PR tree-optimization/64909
> > diff --git a/gcc/match.pd b/gcc/match.pd
> > index 81c4ee6..abc703e 100644
> > --- a/gcc/match.pd
> > +++ b/gcc/match.pd
> > @@ -1018,3 +1018,21 @@ along with GCC; see the file COPYING3.  If not see
> >      (logs (pows @0 @1))
> >      (mult @1 (logs @0)))))
> >
> > +/* If we have a narrowing conversion of an arithmetic operation where
> > +   both operands are widening conversions from the same type as the outer
> > +   narrowing conversion.  Then convert the innermost operands to a suitable
> > +   unsigned type (to avoid introducing undefined behaviour), perform the
> > +   operation and convert the result to the desired type.
> > +
> > +   This narrows the arithmetic operation.  */
>
> This is also a part of what shorten_binary_op does, so please refer to
> that in the comment so we can eventually complete this from there
> and remove shorten_binary_op.
Done.  I've created a block comment meant to cover this pattern and any 
additions we need along the way to moving shortening/narrowing out of 
the front-ends.


> > +(for op (plus minus)
> > +  (simplify
> > +    (convert (op (convert@2 @0) (convert @1)))
> > +    (if (TREE_TYPE (@0) == TREE_TYPE (@1)
> > +         && TREE_TYPE (@0) == type
>
> Otherwhere in match.pd we use
>
> (GIMPLE && types_compatible_p (TREE_TYPE (@0), TREE_TYPE (@1)))
>   || (GENERIC && TREE_TYPE (@0) == TREE_TYPE (@1))
>
> for type equality checks.  And I think even for GENERIC you want
> to ignore qualifiers and thus use TYPE_MAIN_VARIANT (TREE_TYPE (@0))
> == TYPE_MAIN_VARAINT (TREE_TYPE (@1)).
Seems reasonable.    Makes for some ugly formatting though.  It might 
make sense to factor that test so we don't end up repeating it senselessly.

> > +         && INTEGRAL_TYPE_P (type)
>
> So instead of testing TREE_TYPE (@0) == type we probably want
> to just assert that TYPE_PRECISON (TREE_TYPE (@0)) == TYPE_PRECISION
> (type) to allow sign-changes.  Say for
Yea, makes sense.

> short i, j;
> (unsigned short) (i + j)
>
> we still want to narrow the widened i and j.
Right.
> > +         && TYPE_PRECISION (TREE_TYPE (@2)) > TYPE_PRECISION (TREE_TYPE
> > (@0))
> > +        /* This prevents infinite recursion.  */
> > +        && unsigned_type_for (TREE_TYPE (@0)) != TREE_TYPE (@2))
>
> How can it recurse with the type precision constraint right above?
It may no longer be needed.  The precision check used to be >=, but that 
regressed some java codes.  So it got tightened in the last iteration 
before posting.

> Note that technically we don't need to perform the operation in an unsigned
> type iff TYPE_OVERFLOW_WRAPS (TREE_TYPE (@0)).  Thus
>
>             (if (TYPE_OVERFLOW_WRAPS (TREE_TYPE (@0))
>               (convert (op @0 @1)))
>             (with { tree utype = unsigned_type_for (TREE_TYPE (@0)); }
>               (convert (op (convert:utype @0) (convert:utype @1))))))
>
> You can see that GCC exploits that with -fwrapv.  Maybe this
> simplification should be split out into a separate pattern though,
> tacking sign-changes for binary ops only.
No strong opinion on separate pattern or integrating into existing pattern.

jeff