[PATCH V2] Add pattern for pointer-diff on addresses with same base/offset (PR 94234)

[Richard Biener]

On Wed, Aug 19, 2020 at 11:17 AM Feng Xue OS
<fxue@os.amperecomputing.com> wrote:
>
> As Richard's comment, this patch is composed to simplify generalized
> binary-with-convert pattern like ((T) X) OP ((T) Y). Instead of creating
> almost duplicated rules into match.pd, we try to transform it to (T) (X OP Y),
> and apply simplification on (X OP Y) in forwprop pass.
>
> Regards,
> Feng
> ---
> 2020-08-19  Feng Xue  <fxue@os.amperecomputing.com>
>
> gcc/
>         PR tree-optimization/94234
>         * tree-ssa-forwprop.c (simplify_binary_with_convert): New function.
>         * (fwprop_ssa_val): Move it before its new caller.

No * at this line.  There's an entry for (pass_forwprop::execute) missing.

I don't think the transform as implemented, ((T) X) OP ((T) Y) to
(T) (X OP Y) is useful to do in tree-ssa-forwprop.c.  Instead what I
suggested was to do the original

+/* (T)(A * C) +- (T)(B * C) -> (T)((A +- B) * C) and
+   (T)(A * C) +- (T)(A) -> (T)(A * (C +- 1)). */

but realize we already do this for GENERIC in fold_plusminus_mult_expr, just
without the conversions (also look at the conditions in the callers).  This
function takes great care for handling overflow correctly and thus I suggested
to take that over to GIMPLE in tree-ssa-forwprop.c and try extend it to cover
the conversions you need for the specific cases.

Alternatively one could move the GENERIC bits to match.pd, leaving a
worker in fold-const.c.  Then try to extend that there.

I just remember this is a very fragile area with respect to overflow
correctness.

Thanks,
Richard.

> gcc/testsuite/
>         PR tree-optimization/94234
>         * gcc.dg/ifcvt-3.c: Modified to suppress forward propagation.
>         * gcc.dg/tree-ssa/20030807-10.c: Likewise.
>         * gcc.dg/pr94234-2.c: New test.
>
> > ________________________________________
> > From: Richard Biener <richard.guenther@gmail.com>
> > Sent: Monday, June 15, 2020 3:41 PM
> > To: Feng Xue OS
> > Cc: gcc-patches@gcc.gnu.org; Marc Glisse
> > Subject: Re: [PATCH] Add pattern for pointer-diff on addresses with same base/offset (PR 94234)
> >
> > On Fri, Jun 5, 2020 at 11:20 AM Feng Xue OS <fxue@os.amperecomputing.com> wrote:
> >>
> >>  As Marc suggested, removed the new pointer_diff rule, and add another rule to fold
> >>  convert-add expression. This new rule is:
> >>
> >>    (T)(A * C) +- (T)(B * C) -> (T) ((A +- B) * C)
> >>
> >>  Regards,
> >>  Feng
> >>
> >>  ---
> >> 2020-06-01  Feng Xue  <fxue@os.amperecomputing.com>
> >>
> >>  gcc/
> >>          PR tree-optimization/94234
> >>          * match.pd ((T)(A * C) +- (T)(B * C)) -> (T)((A +- B) * C): New
> >>          simplification.
> >>          * ((PTR_A + OFF) - (PTR_B + OFF)) -> (PTR_A - PTR_B): New
> >>          simplification.
> >>
> >>  gcc/testsuite/
> >>          PR tree-optimization/94234
> >>          * gcc.dg/pr94234.c: New test.
> >>  ---
> >>   gcc/match.pd                   | 28 ++++++++++++++++++++++++++++
> >>   gcc/testsuite/gcc.dg/pr94234.c | 24 ++++++++++++++++++++++++
> >>   2 files changed, 52 insertions(+)
> >>   create mode 100644 gcc/testsuite/gcc.dg/pr94234.c
> >>
> >>  diff --git a/gcc/match.pd b/gcc/match.pd
> >>  index 33ee1a920bf..4f340bfe40a 100644
> >>  --- a/gcc/match.pd
> >>  +++ b/gcc/match.pd
> >>  @@ -2515,6 +2515,9 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> >>               && TREE_CODE (@2) == INTEGER_CST
> >>               && tree_int_cst_sign_bit (@2) == 0))
> >>        (minus (convert @1) (convert @2)))))
> >>  +   (simplify
> >>  +    (pointer_diff (pointer_plus @0 @2) (pointer_plus @1 @2))
> >>  +     (pointer_diff @0 @1))
> >
> > This new pattern is OK.  Please commit it separately.
> >
> >>      (simplify
> >>       (pointer_diff (pointer_plus @@0 @1) (pointer_plus @0 @2))
> >>       /* The second argument of pointer_plus must be interpreted as signed, and
> >>  @@ -2526,6 +2529,31 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> >>        (minus (convert (view_convert:stype @1))
> >>              (convert (view_convert:stype @2)))))))
> >>
> >>  +/* (T)(A * C) +- (T)(B * C) -> (T)((A +- B) * C) and
> >>  +   (T)(A * C) +- (T)(A) -> (T)(A * (C +- 1)). */
> >>  +(if (INTEGRAL_TYPE_P (type))
> >>  + (for plusminus (plus minus)
> >>  +  (simplify
> >>  +   (plusminus (convert:s (mult:cs @0 @1)) (convert:s (mult:cs @0 @2)))
> >>  +   (if (element_precision (type) <= element_precision (TREE_TYPE (@0))
> >>  +       && (TYPE_OVERFLOW_UNDEFINED (type) || TYPE_OVERFLOW_WRAPS (type))
> >>  +       && TYPE_OVERFLOW_WRAPS (TREE_TYPE (@0)))
> >>  +    (convert (mult (plusminus @1 @2) @0))))
> >>  +  (simplify
> >>  +   (plusminus (convert @0) (convert@2 (mult:c@3 @0 @1)))
> >>  +   (if (element_precision (type) <= element_precision (TREE_TYPE (@0))
> >>  +       && (TYPE_OVERFLOW_UNDEFINED (type) || TYPE_OVERFLOW_WRAPS (type))
> >>  +       && TYPE_OVERFLOW_WRAPS (TREE_TYPE (@0))
> >>  +       && single_use (@2) && single_use (@3))
> >>  +    (convert (mult (plusminus { build_one_cst (TREE_TYPE (@1)); } @1) @0))))
> >>  +  (simplify
> >>  +   (plusminus (convert@2 (mult:c@3 @0 @1)) (convert @0))
> >>  +   (if (element_precision (type) <= element_precision (TREE_TYPE (@0))
> >>  +       && (TYPE_OVERFLOW_UNDEFINED (type) || TYPE_OVERFLOW_WRAPS (type))
> >>  +       && TYPE_OVERFLOW_WRAPS (TREE_TYPE (@0))
> >>  +       && single_use (@2) && single_use (@3))
> >>  +    (convert (mult (plusminus @1 { build_one_cst (TREE_TYPE (@1)); }) @0))))))
> >>  +
> >
> > This shows the limit of pattern matching IMHO.  I'm also not convinced
> > it gets the
> > overflow cases correct (but I didn't spend too much time here).  Note we have
> > similar functionality implemented in fold_plusminus_mult_expr.  IMHO instead
> > of doing the above moving fold_plusminus_mult_expr to GIMPLE by executing
> > it from inside the forwprop pass would make more sense.  Or finally biting the
> > bullet and try to teach reassociation about how to handle signed arithmetic
> > with non-wrapping overflow behavior.
> >
> > Richard.
>