On Wed, Aug 19, 2020 at 11:17 AM Feng Xue OS
<f...@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 <f...@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.guent...@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 <f...@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 <f...@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.
>
