Re: [PATCH GCC8][30/33]Fold more type conversion into binary arithmetic operations

["Bin.Cheng" <amker dot cheng at gmail dot com>]

On Wed, May 17, 2017 at 1:27 PM, Richard Biener
<richard.guenther@gmail.com> wrote:
> On Mon, May 15, 2017 at 5:56 PM, Bin.Cheng <amker.cheng@gmail.com> wrote:
>> On Thu, May 11, 2017 at 11:54 AM, Richard Biener
>> <richard.guenther@gmail.com> wrote:
>>> On Tue, Apr 18, 2017 at 12:53 PM, Bin Cheng <Bin.Cheng@arm.com> wrote:
>>>> Hi,
>>>> Simplification of (T1)(X *+- CST) is already implemented in aff_combination_expand,
>>>> this patch moves it to tree_to_aff_combination.  It also supports unsigned types
>>>> if range information allows the transformation, as well as special case (T1)(X + X).
>>>> Is it OK?
>>>
>>> Can you first please simply move it?
>>>
>>> +           /* In case X's type has wrapping overflow behavior, we can still
>>> +              convert (T1)(X - CST) into (T1)X - (T1)CST if X - CST doesn't
>>> +              overflow by range information.  Also Convert (T1)(X + CST) as
>>> +              if it's (T1)(X - (-CST)).  */
>>> +           if (TYPE_UNSIGNED (itype)
>>> +               && TYPE_OVERFLOW_WRAPS (itype)
>>> +               && TREE_CODE (op0) == SSA_NAME
>>> +               && TREE_CODE (op1) == INTEGER_CST
>>> +               && (icode == PLUS_EXPR || icode == MINUS_EXPR)
>>> +               && get_range_info (op0, &minv, &maxv) == VR_RANGE)
>>> +             {
>>> +               if (icode == PLUS_EXPR)
>>> +                 op1 = fold_build1 (NEGATE_EXPR, itype, op1);
>>>
>>> Negating -INF will produce -INF(OVF) which we don't want to have in our IL,
>>> I suggest to use
>>>
>>>                   op1 = wide_int_to_tree (itype, wi::neg (op1));
>>>
>>> instead.
>>>
>>> +               if (wi::geu_p (minv, op1))
>>> +                 {
>>> +                   op0 = fold_convert (otype, op0);
>>> +                   op1 = fold_convert (otype, op1);
>>> +                   expr = fold_build2 (MINUS_EXPR, otype, op0, op1);
>>> +                   tree_to_aff_combination (expr, type, comb);
>>> +                   return;
>>> +                 }
>>> +             }
>>>
>>> I think this is similar to a part of what Robin Dapp (sp?) is
>>> proposing as fix for PR69526?
>>>
>>> The same trick should work for (int)((unsigned)X - CST) with different
>>> overflow checks
>>> (you need to make sure the resulting expr does not overflow).
>> Hi,
>> As suggested, I separated the patch into three.  Other review comments
>> are also addressed.
>> I read Robin's PR and patch, I think it's two different issues sharing
>> some aspects, for example, the overflow check using range information
>> are quite the same.  In effect, this should also captures the result
>> of Robin's patch because we don't want to fold (T1)(x +- CST) in
>> general, but here in tree-affine.
>>
>> Bootstrap and test, is it OK?
>
> Ok.  Please commit as separate revisions.
Three patches applied separately @r248955/r248956/r2489557.

Thanks,
bin
>
> Thanks,
> Richard.
>
>> Part1:
>> 2017-04-11  Bin Cheng  <bin.cheng@arm.com>
>>
>>     (aff_combination_expand): Move (T1)(X *+- CST) simplification to ...
>>     (tree_to_aff_combination): ... here.
>>
>> Part2:
>> 2017-04-11  Bin Cheng  <bin.cheng@arm.com>
>>
>>     * tree-affine.c (tree_to_aff_combination): Handle (T1)(X + X).
>>
>> Part3:
>> 2017-04-11  Bin Cheng  <bin.cheng@arm.com>
>>
>>     * tree-affine.c (ssa.h): Include header file.
>>     (tree_to_aff_combination): Handle (T1)(X - CST) when inner type
>>     has wrapping overflow behavior.
>>
>> Thanks,
>> bin
>>>
>>> Richard.
>>>
>>>
>>>> Thanks,
>>>> bin
>>>> 2017-04-11  Bin Cheng  <bin.cheng@arm.com>
>>>>
>>>>         * tree-affine.c: Include header file.
>>>>         (aff_combination_expand): Move (T1)(X *+- CST) simplification to ...
>>>>         (tree_to_aff_combination): ... here.  Support (T1)(X + X) case, and
>>>>         unsigned type case if range information allows.