On Fri, Dec 2, 2016 at 12:58 PM, Bin.Cheng <amker.ch...@gmail.com> wrote:
> On Wed, Nov 30, 2016 at 3:10 PM, Richard Biener
> <richard.guent...@gmail.com> wrote:
>> On Fri, Nov 18, 2016 at 5:53 PM, Bin Cheng <bin.ch...@arm.com> wrote:
>>> Hi,
>>> This is a rework of 
>>> https://gcc.gnu.org/ml/gcc-patches/2016-10/msg02007.html.
>>> Though review comments suggested it could be merged with last kind 
>>> simplification
>>> of fold_cond_expr_with_comparison, it's not really applicable.  As a matter 
>>> of fact,
>>> the suggestion stands for patch 
>>> @https://gcc.gnu.org/ml/gcc-patches/2016-10/msg02005.html.
>>> I had previous patch 
>>> (https://gcc.gnu.org/ml/gcc-patches/2016-11/msg01898.html)
>>> moving fold_cond_expr_with_comparison to match.pd pattern and incorporated
>>> that patch into it.  For this one, the rework is trivial, just renames 
>>> several variable
>>> tags as suggested.  Bootstrap and test on x86_64 and AArch64, is it OK?
>>
>> +     A) Operand x is a unsigned to signed type conversion and c1 is
>> +       integer zero.  In this case,
>> +         (signed type)x  < 0  <=>  x  > MAX_VAL(signed type)
>> +         (signed type)x >= 0  <=>  x <= MAX_VAL(signed type)
>>
>> for (singed type)x < 0 -> x > signed-type-max we probably do a reverse
>> "canonicalization" transform?  Yeah,
>>
>> /* Non-equality compare simplifications from fold_binary  */
>> (for cmp (lt gt le ge)
>> ...
>>      (if (wi::eq_p (@1, signed_max)
>>           && TYPE_UNSIGNED (arg1_type)
>>           /* We will flip the signedness of the comparison operator
>>              associated with the mode of @1, so the sign bit is
>>              specified by this mode.  Check that @1 is the signed
>>              max associated with this sign bit.  */
>>           && prec == GET_MODE_PRECISION (TYPE_MODE (arg1_type))
>>           /* signed_type does not work on pointer types.  */
>>           && INTEGRAL_TYPE_P (arg1_type))
>>       /* The following case also applies to X < signed_max+1
>>          and X >= signed_max+1 because previous transformations.  */
>>       (if (cmp == LE_EXPR || cmp == GT_EXPR)
>>        (with { tree st = signed_type_for (arg1_type); }
>>         (if (cmp == LE_EXPR)
>>          (ge (convert:st @0) { build_zero_cst (st); })
>>          (lt (convert:st @0) { build_zero_cst (st); }))))))))))
>>
>> +           if (cmp_code == GE_EXPR)
>> +             cmp_code = LE_EXPR;
>> +           c1 = wide_int_to_tree (op_type, wi::max_value (to_type));
>> +         }
>> ...
>> +       if (op == PLUS_EXPR)
>> +         real_c1 = wide_int_to_tree (op_type,
>> +                                     wi::sub (c3, c2, sgn, &overflow));
>> +       else
>> +         real_c1 = wide_int_to_tree (op_type,
>> +                                     wi::add (c3, c2, sgn, &overflow));
>>
>> can you avoid the tree building here and just continue using wide-ints 
>> please?
>> Simply do the wide_int_to_tree in the result patterns.
> Hi,
> I updated patch wrto your comments, also deleted two useless
> variables.  Bootstrap and test, is it OK?

Ok.

Thanks,
Richard.

> Thanks,
> bin
>
> 2016-12-01  Bin Cheng  <bin.ch...@arm.com>
>
>     * match.pd: Add new pattern:
>     (cond (cmp (convert? x) c1) (op x c2) c3) -> (op (minmax x c1) c2).
>
> gcc/testsuite/ChangeLog
> 2016-12-01  Bin Cheng  <bin.ch...@arm.com>
>
>     * gcc.dg/fold-bopcond-1.c: New test.
>     * gcc.dg/fold-bopcond-2.c: New test.
>
>>
>> Otherwise looks ok to me.
>>
>> Thanks,
>> Richard.
>>
>>
>>> Thanks,
>>> bin
>>>
>>> 2016-11-17  Bin Cheng  <bin.ch...@arm.com>
>>>
>>>         * match.pd: Add new pattern:
>>>         (cond (cmp (convert? x) c1) (op x c2) c3) -> (op (minmax x c1) c2).
>>>
>>> gcc/testsuite/ChangeLog
>>> 2016-11-17  Bin Cheng  <bin.ch...@arm.com>
>>>
>>>         * gcc.dg/fold-bopcond-1.c: New test.
>>>         * gcc.dg/fold-bopcond-2.c: New test.

Reply via email to