[7/7] MATCH: Add patterns from phiopt's minmax_replacement

Message ID 20230424213011.528181-8-apinski@marvell.com
State Accepted
Headers
Series Some more phiopt cleanups and double minmax to match |

Checks

Context Check Description
snail/gcc-patch-check success Github commit url

Commit Message

Andrew Pinski April 24, 2023, 9:30 p.m. UTC
  This adds a few patterns from phiopt's minmax_replacement
for (A CMP B) ? MIN/MAX<A, C> : MIN/MAX <B, C> .
It is progress to remove minmax_replacement from phiopt.
There are still some more cases dealing with constants on the
edges (0/INT_MAX) to handle in match.

OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions.

gcc/ChangeLog:

	* match.pd: Add patterns for
	"(A CMP B) ? MIN/MAX<A, C> : MIN/MAX <B, C>".

gcc/testsuite/ChangeLog:

	* gcc.dg/tree-ssa/minmax-16.c: Update testcase slightly.
	* gcc.dg/tree-ssa/split-path-1.c: Also disable tree-loop-if-convert
	as that now does the combining.
---
 gcc/match.pd                                 | 16 ++++++++++++++++
 gcc/testsuite/gcc.dg/tree-ssa/minmax-16.c    | 10 ++++++++--
 gcc/testsuite/gcc.dg/tree-ssa/split-path-1.c |  3 ++-
 3 files changed, 26 insertions(+), 3 deletions(-)
  

Comments

Richard Biener April 27, 2023, 10:58 a.m. UTC | #1
On Mon, Apr 24, 2023 at 11:33 PM Andrew Pinski via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> This adds a few patterns from phiopt's minmax_replacement
> for (A CMP B) ? MIN/MAX<A, C> : MIN/MAX <B, C> .
> It is progress to remove minmax_replacement from phiopt.
> There are still some more cases dealing with constants on the
> edges (0/INT_MAX) to handle in match.
>
> OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions.

OK.

> gcc/ChangeLog:
>
>         * match.pd: Add patterns for
>         "(A CMP B) ? MIN/MAX<A, C> : MIN/MAX <B, C>".
>
> gcc/testsuite/ChangeLog:
>
>         * gcc.dg/tree-ssa/minmax-16.c: Update testcase slightly.
>         * gcc.dg/tree-ssa/split-path-1.c: Also disable tree-loop-if-convert
>         as that now does the combining.
> ---
>  gcc/match.pd                                 | 16 ++++++++++++++++
>  gcc/testsuite/gcc.dg/tree-ssa/minmax-16.c    | 10 ++++++++--
>  gcc/testsuite/gcc.dg/tree-ssa/split-path-1.c |  3 ++-
>  3 files changed, 26 insertions(+), 3 deletions(-)
>
> diff --git a/gcc/match.pd b/gcc/match.pd
> index 6d3aaf45a93..5d5aae24509 100644
> --- a/gcc/match.pd
> +++ b/gcc/match.pd
> @@ -4843,6 +4843,22 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>         (convert @c0))))))))
>  #endif
>
> +/* These was part of minmax phiopt.  */
> +/* Optimize (a CMP b) ? minmax<a, c> : minmax<b, c>
> +   to minmax<min/max<a, b>, c> */
> +(for minmax (min max)
> + (for cmp (lt le gt ge)
> +  (simplify
> +   (cond (cmp @1 @3) (minmax:c @1 @4) (minmax:c @2 @4))
> +   (with
> +    {
> +      tree_code code = minmax_from_comparison (cmp, @1, @2, @1, @3);
> +    }
> +    (if (code == MIN_EXPR)
> +     (minmax (min @1 @2) @4)
> +     (if (code == MAX_EXPR)
> +      (minmax (max @1 @2) @4)))))))
> +
>  /* X != C1 ? -X : C2 simplifies to -X when -C1 == C2.  */
>  (simplify
>   (cond (ne @0 INTEGER_CST@1) (negate@3 @0) INTEGER_CST@2)
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/minmax-16.c b/gcc/testsuite/gcc.dg/tree-ssa/minmax-16.c
> index 4febd092d83..623b12b3f74 100644
> --- a/gcc/testsuite/gcc.dg/tree-ssa/minmax-16.c
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/minmax-16.c
> @@ -1,5 +1,5 @@
>  /* { dg-do run } */
> -/* { dg-options "-O -fdump-tree-phiopt -g" } */
> +/* { dg-options "-O -fdump-tree-phiopt -fdump-tree-optimized -g" } */
>
>  #include <stdint.h>
>
> @@ -25,5 +25,11 @@ main (void)
>    return 0;
>  }
>
> -/* { dg-final { scan-tree-dump-times "MIN_EXPR" 3 "phiopt1" } } */
> +/* After phiopt1, there really should be only 3 MIN_EXPR in the IR (including debug statements).
> +   But the way phiopt does not cleanup the CFG all the time, the PHI might still reference the
> +   alternative bb's moved statement.
> +   Note in the end, we do dce the statement and other debug statements to end up with only 2 MIN_EXPR.
> +   So check that too. */
> +/* { dg-final { scan-tree-dump-times "MIN_EXPR" 4 "phiopt1" } } */
> +/* { dg-final { scan-tree-dump-times "MIN_EXPR" 2 "optimized" } } */
>  /* { dg-final { scan-tree-dump-times "MAX_EXPR" 0 "phiopt1" } } */
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/split-path-1.c b/gcc/testsuite/gcc.dg/tree-ssa/split-path-1.c
> index 902dde44a50..b670dee8d10 100644
> --- a/gcc/testsuite/gcc.dg/tree-ssa/split-path-1.c
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/split-path-1.c
> @@ -1,5 +1,6 @@
>  /* { dg-do run } */
> -/* { dg-options "-O2 -fsplit-paths -fdump-tree-split-paths-details --param max-jump-thread-duplication-stmts=20 -fno-ssa-phiopt" } */
> +/* Note both PHI-OPT and the loop if conversion pass converts the inner if to be branchless using min/max. */
> +/* { dg-options "-O2 -fsplit-paths -fdump-tree-split-paths-details --param max-jump-thread-duplication-stmts=20 -fno-ssa-phiopt -fno-tree-loop-if-convert" } */
>
>  #include <stdio.h>
>  #include <stdlib.h>
> --
> 2.39.1
>
  

Patch

diff --git a/gcc/match.pd b/gcc/match.pd
index 6d3aaf45a93..5d5aae24509 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -4843,6 +4843,22 @@  DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
        (convert @c0))))))))
 #endif
 
+/* These was part of minmax phiopt.  */
+/* Optimize (a CMP b) ? minmax<a, c> : minmax<b, c>
+   to minmax<min/max<a, b>, c> */
+(for minmax (min max)
+ (for cmp (lt le gt ge)
+  (simplify
+   (cond (cmp @1 @3) (minmax:c @1 @4) (minmax:c @2 @4))
+   (with
+    {
+      tree_code code = minmax_from_comparison (cmp, @1, @2, @1, @3);
+    }
+    (if (code == MIN_EXPR)
+     (minmax (min @1 @2) @4)
+     (if (code == MAX_EXPR)
+      (minmax (max @1 @2) @4)))))))
+
 /* X != C1 ? -X : C2 simplifies to -X when -C1 == C2.  */
 (simplify
  (cond (ne @0 INTEGER_CST@1) (negate@3 @0) INTEGER_CST@2)
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/minmax-16.c b/gcc/testsuite/gcc.dg/tree-ssa/minmax-16.c
index 4febd092d83..623b12b3f74 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/minmax-16.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/minmax-16.c
@@ -1,5 +1,5 @@ 
 /* { dg-do run } */
-/* { dg-options "-O -fdump-tree-phiopt -g" } */
+/* { dg-options "-O -fdump-tree-phiopt -fdump-tree-optimized -g" } */
 
 #include <stdint.h>
 
@@ -25,5 +25,11 @@  main (void)
   return 0;
 }
 
-/* { dg-final { scan-tree-dump-times "MIN_EXPR" 3 "phiopt1" } } */
+/* After phiopt1, there really should be only 3 MIN_EXPR in the IR (including debug statements).
+   But the way phiopt does not cleanup the CFG all the time, the PHI might still reference the
+   alternative bb's moved statement.
+   Note in the end, we do dce the statement and other debug statements to end up with only 2 MIN_EXPR.
+   So check that too. */
+/* { dg-final { scan-tree-dump-times "MIN_EXPR" 4 "phiopt1" } } */
+/* { dg-final { scan-tree-dump-times "MIN_EXPR" 2 "optimized" } } */
 /* { dg-final { scan-tree-dump-times "MAX_EXPR" 0 "phiopt1" } } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/split-path-1.c b/gcc/testsuite/gcc.dg/tree-ssa/split-path-1.c
index 902dde44a50..b670dee8d10 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/split-path-1.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/split-path-1.c
@@ -1,5 +1,6 @@ 
 /* { dg-do run } */
-/* { dg-options "-O2 -fsplit-paths -fdump-tree-split-paths-details --param max-jump-thread-duplication-stmts=20 -fno-ssa-phiopt" } */
+/* Note both PHI-OPT and the loop if conversion pass converts the inner if to be branchless using min/max. */
+/* { dg-options "-O2 -fsplit-paths -fdump-tree-split-paths-details --param max-jump-thread-duplication-stmts=20 -fno-ssa-phiopt -fno-tree-loop-if-convert" } */
 
 #include <stdio.h>
 #include <stdlib.h>