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

[Jeff Law]

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