On Thu, Oct 19, 2023 at 10:13 PM Andrew Pinski <pins...@gmail.com> wrote:
>
> On Mon, Jul 12, 2021 at 4:47 AM Richard Biener via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
> >
> > On Sun, Jul 11, 2021 at 4:12 AM apinski--- via Gcc-patches
> > <gcc-patches@gcc.gnu.org> wrote:
> > >
> > > From: Andrew Pinski <apin...@marvell.com>
> > >
> > > This patch moves the (a-b) CMP 0 ? (a-b) : (b-a) optimization
> > > from fold_cond_expr_with_comparison to match.
> >
> > So I searched and I guess these transforms are produced from
> >
> > /* If we have A op 0 ? A : -A, consider applying the following
> > transformations:
> >
> > A == 0? A : -A same as -A
> > A != 0? A : -A same as A
> > A >= 0? A : -A same as abs (A)
> > A > 0? A : -A same as abs (A)
> > A <= 0? A : -A same as -abs (A)
> > A < 0? A : -A same as -abs (A)
> >
> > None of these transformations work for modes with signed
> > zeros. If A is +/-0, the first two transformations will
> > change the sign of the result (from +0 to -0, or vice
> > versa). The last four will fix the sign of the result,
> > even though the original expressions could be positive or
> > negative, depending on the sign of A.
> >
> > Note that all these transformations are correct if A is
> > NaN, since the two alternatives (A and -A) are also NaNs. */
> > if (!HONOR_SIGNED_ZEROS (type)
> > && (FLOAT_TYPE_P (TREE_TYPE (arg01))
> > ? real_zerop (arg01)
> > : integer_zerop (arg01))
> > && ((TREE_CODE (arg2) == NEGATE_EXPR
> > && operand_equal_p (TREE_OPERAND (arg2, 0), arg1, 0))
> > /* In the case that A is of the form X-Y, '-A' (arg2) may
> > have already been folded to Y-X, check for that. */
> > || (TREE_CODE (arg1) == MINUS_EXPR
> > && TREE_CODE (arg2) == MINUS_EXPR
> > && operand_equal_p (TREE_OPERAND (arg1, 0),
> > TREE_OPERAND (arg2, 1), 0)
> > && operand_equal_p (TREE_OPERAND (arg1, 1),
> > TREE_OPERAND (arg2, 0), 0))))
> > ...
> >
> > I wonder at which point we can remove the code from fold-const.c?
>
> I have to double check if after an updated patch, if that code does
> anything that match does not do.
> I will do that before I submit an updated patch.
I looked and the main thing left is solving the stripping of sign nops
that happen at the beginning of fold_cond_expr_with_comparison.
I did solve part of that with the recent
r14-4662-gc7609acb8a8210188d21b2cd7 but not with this new patterns; I
will solve that in a separate patch.
Thanks,
Andrew Pinski
>
> >
> > Some comments inline below.
> >
> > > OK? Bootstrapped and tested on x86_64-linux-gnu.
> > >
> > > gcc/ChangeLog:
> > >
> > > * match.pd ((A-B) CMP 0 ? (A-B) : (B - A)):
> > > New patterns.
> > >
> > > gcc/testsuite/ChangeLog:
> > >
> > > * gcc.dg/tree-ssa/phi-opt-25.c: New test.
> > > ---
> > > gcc/match.pd | 48 ++++++++++++++++++++--
> > > gcc/testsuite/gcc.dg/tree-ssa/phi-opt-25.c | 45 ++++++++++++++++++++
> > > 2 files changed, 90 insertions(+), 3 deletions(-)
> > > create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/phi-opt-25.c
> > >
> > > diff --git a/gcc/match.pd b/gcc/match.pd
> > > index 30680d488ab..aa88381fdcb 100644
> > > --- a/gcc/match.pd
> > > +++ b/gcc/match.pd
> > > @@ -4040,9 +4040,7 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> > > (cnd (logical_inverted_value truth_valued_p@0) @1 @2)
> > > (cnd @0 @2 @1)))
> > >
> > > -/* abs/negative simplifications moved from
> > > fold_cond_expr_with_comparison,
> > > - Need to handle (A - B) case as fold_cond_expr_with_comparison does.
> > > - Need to handle UN* comparisons.
> > > +/* abs/negative simplifications moved from
> > > fold_cond_expr_with_comparison.
> > >
> > > None of these transformations work for modes with signed
> > > zeros. If A is +/-0, the first two transformations will
> > > @@ -4098,6 +4096,50 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> > > (convert (negate (absu:utype @0))))
> > > (negate (abs @0)))))
> > > )
> > > +
> > > + /* (A - B) == 0 ? (A - B) : (B - A) same as (B - A) */
> > > + (for cmp (eq uneq)
> > > + (simplify
> > > + (cnd (cmp (minus@0 @1 @2) zerop) @0 (minus@3 @2 @1))
> > > + (if (!HONOR_SIGNED_ZEROS (type))
> > > + @3))
> > > + (simplify
> > > + (cnd (cmp (minus@0 @1 @2) zerop) integer_zerop (minus@3 @2 @1))
> >
> > So that makes me think why integer_zerop? 'type' should then be
> > integer and thus never HONOR_SIGNED_ZEROS.
>
> yes that should be done.
>
> >
> > Don't we also need the inverted condition case for completeness?
>
> Yes we should. Though for phiopt we don't.
>
>
> >
> > > + (if (!HONOR_SIGNED_ZEROS (type))
> > > + @3))
> > > + (simplify
> > > + (cnd (cmp @1 @2) integer_zerop (minus@3 @2 @1))
> >
> > I think this needs to be (cmp:c @1 @2)
>
> This is now actually handled already by r14-3606-g3d86e7f4a8ae so I removed
> it.
> I will submit a new patch in the next few days for this too.
>
> Thanks,
> Andrew Pinski
>
> >
> > > + (if (!HONOR_SIGNED_ZEROS (type))
> > > + @3))
> > > + )
> > > + /* (A - B) != 0 ? (A - B) : (B - A) same as (A - B) */
> > > + (for cmp (ne ltgt)
> > > + (simplify
> > > + (cnd (cmp (minus@0 @1 @2) zerop) @0 (minus @2 @1))
> > > + (if (!HONOR_SIGNED_ZEROS (type))
> > > + @0))
> > > + )
> > > + /* (A - B) >=/> 0 ? (A - B) : (B - A) same as abs (A - B) */
> > > + (for cmp (ge gt)
> > > + (simplify
> > > + (cnd (cmp (minus@0 @1 @2) zerop) @0 (minus @2 @1))
> > > + (if (!HONOR_SIGNED_ZEROS (type)
> > > + && !TYPE_UNSIGNED (type))
> > > + (abs @0))))
> > > + /* (A - B) <=/< 0 ? (A - B) : (B - A) same as -abs (A - B) */
> > > + (for cmp (le lt)
> > > + (simplify
> > > + (cnd (cmp (minus@0 @1 @2) zerop) @0 (minus @2 @1))
> > > + (if (!HONOR_SIGNED_ZEROS (type)
> > > + && !TYPE_UNSIGNED (type))
> > > + (if (ANY_INTEGRAL_TYPE_P (type)
> > > + && !TYPE_OVERFLOW_WRAPS (type))
> > > + (with {
> > > + tree utype = unsigned_type_for (type);
> > > + }
> > > + (convert (negate (absu:utype @0))))
> > > + (negate (abs @0)))))
> > > + )
> > > )
> > >
> > > /* -(type)!A -> (type)A - 1. */
> > > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-25.c
> > > b/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-25.c
> > > new file mode 100644
> > > index 00000000000..0f0e3170f8d
> > > --- /dev/null
> > > +++ b/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-25.c
> > > @@ -0,0 +1,45 @@
> > > +/* { dg-options "-O2 -fno-signed-zeros -fdump-tree-phiopt" } */
> > > +int minus1(int a, int b)
> > > +{
> > > + int c = a - b;
> > > + if (c == 0) c = b - a;
> > > + return c;
> > > +}
> > > +int minus2(int a, int b)
> > > +{
> > > + int c = a - b;
> > > + if (c != 0) c = b - a;
> > > + return c;
> > > +}
> > > +int minus3(int a, int b)
> > > +{
> > > + int c = a - b;
> > > + if (c == 0) c = 0;
> > > + else c = b - a;
> > > + return c;
> > > +}
> > > +int minus4(int a, int b)
> > > +{
> > > + int c;
> > > + if (a == b) c = 0;
> > > + else
> > > + c = b - a;
> > > + return c;
> > > +}
> > > +int abs0(int a, int b)
> > > +{
> > > + int c = a - b;
> > > + if (c <= 0) c = b - a;
> > > + return c;
> > > +}
> > > +int negabs(int a, int b)
> > > +{
> > > + int c = a - b;
> > > + if (c >= 0) c = b - a;
> > > + return c;
> > > +}
> > > +
> > > +/* The above should be optimized at phiopt1 except for negabs which has
> > > to wait
> > > + until phiopt2 as -abs is not acceptable in early phiopt. */
> > > +/* { dg-final { scan-tree-dump-times "if" 1 "phiopt1" } } */
> > > +/* { dg-final { scan-tree-dump-not "if" "phiopt2" } } */
> > > --
> > > 2.27.0
> > >
