On Mon, May 15, 2017 at 5:56 PM, Bin.Cheng <amker.ch...@gmail.com> wrote:
> On Thu, May 11, 2017 at 11:54 AM, Richard Biener
> <richard.guent...@gmail.com> wrote:
>> On Tue, Apr 18, 2017 at 12:53 PM, Bin Cheng <bin.ch...@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.
Thanks,
Richard.
> Part1:
> 2017-04-11 Bin Cheng <bin.ch...@arm.com>
>
> (aff_combination_expand): Move (T1)(X *+- CST) simplification to ...
> (tree_to_aff_combination): ... here.
>
> Part2:
> 2017-04-11 Bin Cheng <bin.ch...@arm.com>
>
> * tree-affine.c (tree_to_aff_combination): Handle (T1)(X + X).
>
> Part3:
> 2017-04-11 Bin Cheng <bin.ch...@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.ch...@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.
