Re: [PATCH GCC]Simplify (cond (cmp (convert? x) c1) (op x c2) c3) -> (op (minmax x c1) c2)

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

On Wed, Nov 30, 2016 at 3:10 PM, Richard Biener
<richard.guenther@gmail.com> wrote:
> On Fri, Nov 18, 2016 at 5:53 PM, Bin Cheng <Bin.Cheng@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?

Thanks,
bin

2016-12-01  Bin Cheng  <bin.cheng@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.cheng@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.cheng@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.cheng@arm.com>
>>
>>         * gcc.dg/fold-bopcond-1.c: New test.
>>         * gcc.dg/fold-bopcond-2.c: New test.

Attachment: 04-simplify-bopcond-20161201.txt
Description: Text document