[PATCH 2/3] Add simplify rules for wrapped binary operations.

[Richard Biener]

On Tue, Aug 13, 2019 at 10:36 AM Robin Dapp <rdapp@linux.ibm.com> wrote:
>
> We would like to simplify code like
>  (larger_type)(var + const1) + const2
> to
>  (larger_type)(var + combined_const1_const2)
> when we know that no overflow happens.

Trowing in my own comments...

> ---
>  gcc/match.pd | 101 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 101 insertions(+)
>
> diff --git a/gcc/match.pd b/gcc/match.pd
> index 0317bc704f7..94400529ad8 100644
> --- a/gcc/match.pd
> +++ b/gcc/match.pd
> @@ -2020,6 +2020,107 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>      (if (cst && !TREE_OVERFLOW (cst))
>       (plus { cst; } @0))))
>
> +/* ((T)(A + CST1)) + CST2 -> (T)(A) + CST  */
> +#if GIMPLE
> +  (simplify
> +    (plus (convert (plus @0 INTEGER_CST@1)) INTEGER_CST@2)
> +      (if (INTEGRAL_TYPE_P (type)
> +           && TYPE_PRECISION (type) > TYPE_PRECISION (TREE_TYPE (@0)))
> +       /* We actually want to perform two simplifications here:
> +         (1) (T)(A + CST1) + CST2  --> (T)(A) + (T)(CST1)
> +             If for (A + CST1) we either do not care about overflow (e.g.
> +             for a signed inner type) or the overflow is ok for an unsigned
> +             inner type.
> +         (2) (T)(A) + (T)(CST1) + CST2 --> (T)(A) + (T)(CST1 + CST2)

But the original is already (T)(A) + CST1-in-T + CST2-in-T and thus we
can always
do 2) by means of already existing patterns.  So it's only 1) you need
to implement?!
And 1) is really (T)(A + CST) -> (T)A + CST-in-T, no?

> +             If (CST1 + CST2) does not overflow and we do care about overflow
> +             (for a signed outer type) or we do not care about overflow in an
> +             unsigned outer type.  */
> +       (with
> +       {
> +         tree inner_type = TREE_TYPE (@0);
> +         wide_int wmin0, wmax0;
> +         wide_int cst1 = wi::to_wide (@1);
> +
> +        wi::overflow_type min_ovf = wi::OVF_OVERFLOW,
> +                          max_ovf = wi::OVF_OVERFLOW;
> +
> +        /* Get overflow behavior.  */
> +         bool ovf_undef_inner = TYPE_OVERFLOW_UNDEFINED (inner_type);
> +         bool ovf_undef_outer = TYPE_OVERFLOW_UNDEFINED (type);
> +
> +        /* Get value range of A.  */
> +         enum value_range_kind vr0 = get_range_info (@0, &wmin0, &wmax0);
> +
> +        /* If we have a proper range, determine min and max overflow
> +           of (A + CST1).
> +           ??? We might also want to handle anti ranges.  */
> +        if (vr0 == VR_RANGE)
> +          {
> +            wi::add (wmin0, cst1, TYPE_SIGN (inner_type), &min_ovf);
> +            wi::add (wmax0, cst1, TYPE_SIGN (inner_type), &max_ovf);
> +          }
> +
> +        /* Inner overflow does not matter in this case.  */
> +        if (ovf_undef_inner)
> +          {
> +            min_ovf = wi::OVF_NONE;
> +            max_ovf = wi::OVF_NONE;
> +          }
> +
> +        /* Extend CST from INNER_TYPE to TYPE.  */
> +        cst1 = cst1.from (cst1, TYPE_PRECISION (type), TYPE_SIGN (inner_type));
> +
> +        /* Check for overflow of (TYPE)(CST1 + CST2).  */
> +        wi::overflow_type outer_ovf = wi::OVF_OVERFLOW;
> +        wide_int cst = wi::add (cst1, wi::to_wide (@2), TYPE_SIGN (type),
> +            &outer_ovf);
> +
> +        /* We *do* care about an overflow here as we do not want to introduce
> +           new undefined behavior that was not there before.  */
> +        if (ovf_undef_outer && outer_ovf)
> +          {
> +            /* Set these here to prevent the final conversion below
> +               to take place instead of introducing a new guard variable.  */
> +            min_ovf = wi::OVF_OVERFLOW;
> +            max_ovf = wi::OVF_OVERFLOW;
> +          }
> +       }
> +   (if (min_ovf == wi::OVF_NONE && max_ovf == wi::OVF_NONE)
> +    (plus (convert @0) { wide_int_to_tree (type, cst); }
> +     )))))
> +#endif
> +
> +/* ((T)(A)) + CST -> (T)(A + CST)  */

But then this is the reverse... (as Marc already noticed).

So - what are you really after? (sorry if I don't remeber, testcase(s)
are missing
from this patch)

To me it seems that 1) loses information if A + CST was done in a signed type
and we know that overflow doesn't happen because of that.  For the reverse
transformation we don't.  Btw, if you make A == A' + CST' then
you get (T)A + CST -> (T)(A' + CST' + CST) which is again trivially handled
so why do you need both transforms again?

> +#if GIMPLE
> +  (simplify
> +   (plus (convert SSA_NAME@0) INTEGER_CST@1)
> +    (if (INTEGRAL_TYPE_P (TREE_TYPE (@0))
> +         && INTEGRAL_TYPE_P (type)
> +         && TYPE_PRECISION (type) > TYPE_PRECISION (TREE_TYPE (@0))
> +         && int_fits_type_p (@1, TREE_TYPE (@0)))
> +     /* Perform binary operation inside the cast if the constant fits
> +        and (A + CST)'s range does not overflow.  */
> +     (with
> +      {
> +       wi::overflow_type min_ovf = wi::OVF_OVERFLOW,
> +                         max_ovf = wi::OVF_OVERFLOW;
> +        tree inner_type = TREE_TYPE (@0);
> +
> +        wide_int w1 = w1.from (wi::to_wide (@1), TYPE_PRECISION (inner_type),
> +           TYPE_SIGN (inner_type));
> +
> +        wide_int wmin0, wmax0;
> +        if (get_range_info (@0, &wmin0, &wmax0) == VR_RANGE)
> +          {
> +            wi::add (wmin0, w1, TYPE_SIGN (inner_type), &min_ovf);
> +            wi::add (wmax0, w1, TYPE_SIGN (inner_type), &max_ovf);
> +          }
> +      }
> +     (if (min_ovf == wi::OVF_NONE && max_ovf == wi::OVF_NONE)
> +      (convert (plus @0 { {wide_int_to_tree (TREE_TYPE (@0), w1)}; })))
> +     )))
> +#endif
> +
>    /* ~A + A -> -1 */
>    (simplify
>     (plus:c (bit_not @0) @0)
> --
> 2.17.0
>