Re: [PATCH GCC][1/4]Simplify (convert1 (minmax ((convert2 (x) c)))) into minmax (x c)

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

On Tue, Oct 25, 2016 at 12:48 PM, Richard Biener
<richard.guenther@gmail.com> wrote:
> On Tue, Oct 25, 2016 at 1:21 PM, Bin Cheng <Bin.Cheng@arm.com> wrote:
>> Hi,
>> This is a patch set adding various match.pd patterns in order to generate simplified MIN/MAX_EXPR mostly from COND_EXPR.  This is the first one optimizing (convert1 (minmax ((convert2 (x) c)))) to minmax (x c), if convert2 promotes x and convert1 demotes back to x's type.  With this patch, generated assembly for test:
>> .L4:
>>         ldr     q0, [x3, x1]
>>         add     w2, w2, 1
>>         cmp     w0, w2
>>         ushll   v2.4s, v0.4h, 0
>>         ushll2  v1.4s, v0.8h, 0
>>         umin    v2.4s, v2.4s, v3.4s
>>         umin    v1.4s, v1.4s, v3.4s
>>         xtn     v4.4h, v2.4s
>>         xtn2    v4.8h, v1.4s
>>         str     q4, [x3, x1]
>>         add     x1, x1, 16
>>         bhi     .L4
>>
>> can be improved to:
>> .L4:
>>         ldr     q0, [x3, x1]
>>         add     w2, w2, 1
>>         cmp     w0, w2
>>         umin    v1.8h, v0.8h, v2.8h
>>         str     q1, [x3, x1]
>>         add     x1, x1, 16
>>         bhi     .L4
>>
>> Bootstrap and test on x86_64 and AArch64 for whole patch set.  Is it OK?
>
> Why restrict to GIMPLE?
>
> +/* (convert1 (minmax ((convert2 (x) c)))) -> minmax (x c) if convert2
> +   promotes x and convert1 demotes back to x's type.  */
> +
> +(for minmax (min max)
> + (simplify
> +  (convert (minmax@0 (convert @1) INTEGER_CST@2))
> +  (if (types_compatible_p (TREE_TYPE (@1), type))
>
> comment mentions convert1 and convert2, just convert is correct I think.
>
> Please use types_match instead of types_compatible_p, this is a
> wrapper that does the correct thing for both GENERIC and GIMPLE.
>
> +   (with
> +    {
> +      tree minmax_type = TREE_TYPE (@0);
> +      signop sgn = TYPE_SIGN (type);
> +      widest_int type_min = widest_int::from (wi::min_value (type), sgn);
> +      widest_int type_max = widest_int::from (wi::max_value (type), sgn);
> +    }
> +    (if (sgn == TYPE_SIGN (minmax_type)
> +        && TYPE_PRECISION (minmax_type) >= TYPE_PRECISION (type)
> +        && wi::to_widest (@2) >= type_min && wi::to_widest (@2) <= type_max)
>
> instead of this you can use int_fits_type_p (type, @2)
>
> +     (minmax @1 { fold_convert (type, @2); }))))))
>
> use
>
>      (minmax @1 (convert @2))
>
> I believe the transform is also a win if @2 is not a constant but similarly
> promoted as @1.  This slightly complicates the patter and thus can
> be done as followup (if we think it's useful at this point).
>
> With the simplification you should get rid of the with{}

Thanks for reviewing, updated patch attached.  Is it OK?

Thanks,
bin

Attachment: 01-simplify-convminmaxconv-20161023.txt
Description: Text document