On Wed, Feb 11, 2015 at 6:05 PM, Jeff Law <l...@redhat.com> wrote:
> 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 <l...@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 <rguent...@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.
Yeah, I'd leave it as you did it for now. Btw, -ENOPATCH.
Richard.
> jeff
>
