public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] move the (a-b) CMP 0 ? (a-b) : (b-a) optimization from fold_cond_expr_with_comparison to match
@ 2021-07-11  2:11 apinski
  2021-07-12 11:46 ` Richard Biener
  0 siblings, 1 reply; 4+ messages in thread
From: apinski @ 2021-07-11  2:11 UTC (permalink / raw)
  To: gcc-patches; +Cc: Andrew Pinski

From: Andrew Pinski <apinski@marvell.com>

This patch moves the (a-b) CMP 0 ? (a-b) : (b-a) optimization
from fold_cond_expr_with_comparison to match.

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))
+    (if (!HONOR_SIGNED_ZEROS (type))
+     @3))
+  (simplify
+   (cnd (cmp @1 @2) integer_zerop (minus@3 @2 @1))
+    (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


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] move the (a-b) CMP 0 ? (a-b) : (b-a) optimization from fold_cond_expr_with_comparison to match
  2021-07-11  2:11 [PATCH] move the (a-b) CMP 0 ? (a-b) : (b-a) optimization from fold_cond_expr_with_comparison to match apinski
@ 2021-07-12 11:46 ` Richard Biener
  2023-10-20  5:13   ` Andrew Pinski
  0 siblings, 1 reply; 4+ messages in thread
From: Richard Biener @ 2021-07-12 11:46 UTC (permalink / raw)
  To: Andrew Pinski; +Cc: GCC Patches

On Sun, Jul 11, 2021 at 4:12 AM apinski--- via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> From: Andrew Pinski <apinski@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?

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.

Don't we also need the inverted condition case for completeness?

> +    (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)

> +    (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
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] move the (a-b) CMP 0 ? (a-b) : (b-a) optimization from fold_cond_expr_with_comparison to match
  2021-07-12 11:46 ` Richard Biener
@ 2023-10-20  5:13   ` Andrew Pinski
  2023-10-21 20:14     ` Andrew Pinski
  0 siblings, 1 reply; 4+ messages in thread
From: Andrew Pinski @ 2023-10-20  5:13 UTC (permalink / raw)
  To: Richard Biener; +Cc: GCC Patches

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 <apinski@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.

>
> 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
> >

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] move the (a-b) CMP 0 ? (a-b) : (b-a) optimization from fold_cond_expr_with_comparison to match
  2023-10-20  5:13   ` Andrew Pinski
@ 2023-10-21 20:14     ` Andrew Pinski
  0 siblings, 0 replies; 4+ messages in thread
From: Andrew Pinski @ 2023-10-21 20:14 UTC (permalink / raw)
  To: Richard Biener; +Cc: GCC Patches

On Thu, Oct 19, 2023 at 10:13 PM Andrew Pinski <pinskia@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 <apinski@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
> > >

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2023-10-21 20:14 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-11  2:11 [PATCH] move the (a-b) CMP 0 ? (a-b) : (b-a) optimization from fold_cond_expr_with_comparison to match apinski
2021-07-12 11:46 ` Richard Biener
2023-10-20  5:13   ` Andrew Pinski
2023-10-21 20:14     ` Andrew Pinski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).