Re: [PATCH]middle-end: fix min/max phiopts reduction [PR106744]

2022-08-29 Thread Richard Biener via Gcc-patches 
On Tue, 30 Aug 2022, Tamar Christina wrote:

> Hi All,
> 
> This corrects the argument usage to use them in the order that they occur in
> the comparisons in gimple.
> 
> This was tested by disabling the pass, adding the runtime checks and 
> re-enabling
> the pass and verifying the tests still pass.
> 
> Also tested that the runtime test caught the issue by updating the tests on an
> unpatched tree and observing that some fail.
> 
> Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.
> 
> Ok for master?

OK.
 
> Thanks,
> Tamar
> 
> gcc/ChangeLog:
> 
>   PR tree-optimization/106744
>   * tree-ssa-phiopt.cc (minmax_replacement): Correct arguments.
> 
> gcc/testsuite/ChangeLog:
> 
>   PR tree-optimization/106744
>   * gcc.dg/tree-ssa/minmax-10.c: Make runtime test.
>   * gcc.dg/tree-ssa/minmax-11.c: Likewise.
>   * gcc.dg/tree-ssa/minmax-12.c: Likewise.
>   * gcc.dg/tree-ssa/minmax-13.c: Likewise.
>   * gcc.dg/tree-ssa/minmax-14.c: Likewise.
>   * gcc.dg/tree-ssa/minmax-15.c: Likewise.
>   * gcc.dg/tree-ssa/minmax-16.c: Likewise.
>   * gcc.dg/tree-ssa/minmax-3.c: Likewise.
>   * gcc.dg/tree-ssa/minmax-4.c: Likewise.
>   * gcc.dg/tree-ssa/minmax-5.c: Likewise.
>   * gcc.dg/tree-ssa/minmax-6.c: Likewise.
>   * gcc.dg/tree-ssa/minmax-7.c: Likewise.
>   * gcc.dg/tree-ssa/minmax-8.c: Likewise.
>   * gcc.dg/tree-ssa/minmax-9.c: Likewise.
> 
> --- inline copy of patch -- 
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/minmax-10.c 
> b/gcc/testsuite/gcc.dg/tree-ssa/minmax-10.c
> index 
> 589953684416a9d263084deb58f6cde7094dd517..c9322a17a4af8e01add2f04176805c812af62e07
>  100644
> --- a/gcc/testsuite/gcc.dg/tree-ssa/minmax-10.c
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/minmax-10.c
> @@ -1,8 +1,9 @@
> -/* { dg-do compile } */
> +/* { dg-do run } */
>  /* { dg-options "-O -fdump-tree-optimized" } */
>  
>  #include 
>  
> +__attribute__ ((noipa, noinline))
>  uint8_t three_max (uint8_t xc, uint8_t xm, uint8_t xy) {
>  uint8_t   xk;
>  xc=~xc;
> @@ -16,5 +17,16 @@ uint8_t three_max (uint8_t xc, uint8_t xm, uint8_t xy) {
>  return xk;
>  }
>  
> +int
> +main (void)
> +{
> +  volatile uint8_t xy = 255;
> +  volatile uint8_t xm = 0;
> +  volatile uint8_t xc = 127;
> +  if (three_max (xc, xm, xy) != 255)
> +__builtin_abort ();
> +  return 0;
> +}
> +
>  /* { dg-final { scan-tree-dump-times "MIN_EXPR" 2 "optimized" } } */
>  /* { dg-final { scan-tree-dump-times "= ~" 1 "optimized" } } */
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/minmax-11.c 
> b/gcc/testsuite/gcc.dg/tree-ssa/minmax-11.c
> index 
> 1c2ef01b5d1e639fbf95bb5ca473b63cc98e9df1..b1da41712b342cd7344167a0da91ffd419491391
>  100644
> --- a/gcc/testsuite/gcc.dg/tree-ssa/minmax-11.c
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/minmax-11.c
> @@ -1,8 +1,10 @@
> -/* { dg-do compile } */
> +/* { dg-do run } */
>  /* { dg-options "-O -fdump-tree-optimized" } */
>  
>  #include 
>  
> +
> +__attribute__ ((noipa, noinline))
>  uint8_t three_minmax1 (uint8_t xc, uint8_t xm, uint8_t xy) {
>   uint8_t  xk;
>  xc=~xc;
> @@ -16,6 +18,17 @@ uint8_t three_minmax1 (uint8_t xc, uint8_t xm, uint8_t xy) 
> {
>  return xk;
>  }
>  
> +int
> +main (void)
> +{
> +  volatile uint8_t xy = 255;
> +  volatile uint8_t xm = 0;
> +  volatile uint8_t xc = 127;
> +  if (three_minmax1 (xc, xm, xy) != 0)
> +__builtin_abort ();
> +  return 0;
> +}
> +
>  /* { dg-final { scan-tree-dump-times "MIN_EXPR" 1 "optimized" } } */
>  /* { dg-final { scan-tree-dump-times "MAX_EXPR" 1 "optimized" } } */
>  /* { dg-final { scan-tree-dump-times "= ~" 1 "optimized" } } */
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/minmax-12.c 
> b/gcc/testsuite/gcc.dg/tree-ssa/minmax-12.c
> index 
> 3d0c07d9b57dd689bcb89653937727ab441e7f2b..cb9188f90e8e12c6244d559e63723177102177ee
>  100644
> --- a/gcc/testsuite/gcc.dg/tree-ssa/minmax-12.c
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/minmax-12.c
> @@ -1,8 +1,9 @@
> -/* { dg-do compile } */
> +/* { dg-do run } */
>  /* { dg-options "-O -fdump-tree-phiopt" } */
>  
>  #include 
>  
> +__attribute__ ((noinline, noipa))
>  uint8_t three_minmax3 (uint8_t xc, uint8_t xm, uint8_t xy) {
>  uint8_t  xk;
>  xc=~xc;
> @@ -16,5 +17,16 @@ uint8_t three_minmax3 (uint8_t xc, uint8_t xm, uint8_t xy) 
> {
>  return xk;
>  }
>  
> +int
> +main (void)
> +{
> +  volatile uint8_t xy = 255;
> +  volatile uint8_t xm = 0;
> +  volatile uint8_t xc = 127;
> +  if (three_minmax3 (xc, xm, xy) != 0)
> +__builtin_abort ();
> +  return 0;
> +}
> +
>  /* { dg-final { scan-tree-dump-times "MIN_EXPR" 1 "phiopt1" } } */
>  /* { dg-final { scan-tree-dump-times "MAX_EXPR" 1 "phiopt1" } } */
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/minmax-13.c 
> b/gcc/testsuite/gcc.dg/tree-ssa/minmax-13.c
> index 
> c0d0f27c8027ae87654532d1b919cfeccf4413e0..62ba71e8c3f21f1cb33ae2473fd2b30bfdc13c81
>  100644
> --- a/gcc/testsuite/gcc.dg/tree-ssa/minmax-13.c
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/minmax-13.c
> @@ -1,8

[PATCH]middle-end: fix min/max phiopts reduction [PR106744]

2022-08-29 Thread Tamar Christina via Gcc-patches 
Hi All,

This corrects the argument usage to use them in the order that they occur in
the comparisons in gimple.

This was tested by disabling the pass, adding the runtime checks and re-enabling
the pass and verifying the tests still pass.

Also tested that the runtime test caught the issue by updating the tests on an
unpatched tree and observing that some fail.

Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.

Ok for master?

Thanks,
Tamar

gcc/ChangeLog:

PR tree-optimization/106744
* tree-ssa-phiopt.cc (minmax_replacement): Correct arguments.

gcc/testsuite/ChangeLog:

PR tree-optimization/106744
* gcc.dg/tree-ssa/minmax-10.c: Make runtime test.
* gcc.dg/tree-ssa/minmax-11.c: Likewise.
* gcc.dg/tree-ssa/minmax-12.c: Likewise.
* gcc.dg/tree-ssa/minmax-13.c: Likewise.
* gcc.dg/tree-ssa/minmax-14.c: Likewise.
* gcc.dg/tree-ssa/minmax-15.c: Likewise.
* gcc.dg/tree-ssa/minmax-16.c: Likewise.
* gcc.dg/tree-ssa/minmax-3.c: Likewise.
* gcc.dg/tree-ssa/minmax-4.c: Likewise.
* gcc.dg/tree-ssa/minmax-5.c: Likewise.
* gcc.dg/tree-ssa/minmax-6.c: Likewise.
* gcc.dg/tree-ssa/minmax-7.c: Likewise.
* gcc.dg/tree-ssa/minmax-8.c: Likewise.
* gcc.dg/tree-ssa/minmax-9.c: Likewise.

--- inline copy of patch -- 
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/minmax-10.c 
b/gcc/testsuite/gcc.dg/tree-ssa/minmax-10.c
index 
589953684416a9d263084deb58f6cde7094dd517..c9322a17a4af8e01add2f04176805c812af62e07
 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/minmax-10.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/minmax-10.c
@@ -1,8 +1,9 @@
-/* { dg-do compile } */
+/* { dg-do run } */
 /* { dg-options "-O -fdump-tree-optimized" } */
 
 #include 
 
+__attribute__ ((noipa, noinline))
 uint8_t three_max (uint8_t xc, uint8_t xm, uint8_t xy) {
 uint8_t xk;
 xc=~xc;
@@ -16,5 +17,16 @@ uint8_t three_max (uint8_t xc, uint8_t xm, uint8_t xy) {
 return xk;
 }
 
+int
+main (void)
+{
+  volatile uint8_t xy = 255;
+  volatile uint8_t xm = 0;
+  volatile uint8_t xc = 127;
+  if (three_max (xc, xm, xy) != 255)
+__builtin_abort ();
+  return 0;
+}
+
 /* { dg-final { scan-tree-dump-times "MIN_EXPR" 2 "optimized" } } */
 /* { dg-final { scan-tree-dump-times "= ~" 1 "optimized" } } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/minmax-11.c 
b/gcc/testsuite/gcc.dg/tree-ssa/minmax-11.c
index 
1c2ef01b5d1e639fbf95bb5ca473b63cc98e9df1..b1da41712b342cd7344167a0da91ffd419491391
 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/minmax-11.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/minmax-11.c
@@ -1,8 +1,10 @@
-/* { dg-do compile } */
+/* { dg-do run } */
 /* { dg-options "-O -fdump-tree-optimized" } */
 
 #include 
 
+
+__attribute__ ((noipa, noinline))
 uint8_t three_minmax1 (uint8_t xc, uint8_t xm, uint8_t xy) {
uint8_t  xk;
 xc=~xc;
@@ -16,6 +18,17 @@ uint8_t three_minmax1 (uint8_t xc, uint8_t xm, uint8_t xy) {
 return xk;
 }
 
+int
+main (void)
+{
+  volatile uint8_t xy = 255;
+  volatile uint8_t xm = 0;
+  volatile uint8_t xc = 127;
+  if (three_minmax1 (xc, xm, xy) != 0)
+__builtin_abort ();
+  return 0;
+}
+
 /* { dg-final { scan-tree-dump-times "MIN_EXPR" 1 "optimized" } } */
 /* { dg-final { scan-tree-dump-times "MAX_EXPR" 1 "optimized" } } */
 /* { dg-final { scan-tree-dump-times "= ~" 1 "optimized" } } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/minmax-12.c 
b/gcc/testsuite/gcc.dg/tree-ssa/minmax-12.c
index 
3d0c07d9b57dd689bcb89653937727ab441e7f2b..cb9188f90e8e12c6244d559e63723177102177ee
 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/minmax-12.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/minmax-12.c
@@ -1,8 +1,9 @@
-/* { dg-do compile } */
+/* { dg-do run } */
 /* { dg-options "-O -fdump-tree-phiopt" } */
 
 #include 
 
+__attribute__ ((noinline, noipa))
 uint8_t three_minmax3 (uint8_t xc, uint8_t xm, uint8_t xy) {
 uint8_t  xk;
 xc=~xc;
@@ -16,5 +17,16 @@ uint8_t three_minmax3 (uint8_t xc, uint8_t xm, uint8_t xy) {
 return xk;
 }
 
+int
+main (void)
+{
+  volatile uint8_t xy = 255;
+  volatile uint8_t xm = 0;
+  volatile uint8_t xc = 127;
+  if (three_minmax3 (xc, xm, xy) != 0)
+__builtin_abort ();
+  return 0;
+}
+
 /* { dg-final { scan-tree-dump-times "MIN_EXPR" 1 "phiopt1" } } */
 /* { dg-final { scan-tree-dump-times "MAX_EXPR" 1 "phiopt1" } } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/minmax-13.c 
b/gcc/testsuite/gcc.dg/tree-ssa/minmax-13.c
index 
c0d0f27c8027ae87654532d1b919cfeccf4413e0..62ba71e8c3f21f1cb33ae2473fd2b30bfdc13c81
 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/minmax-13.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/minmax-13.c
@@ -1,8 +1,9 @@
-/* { dg-do compile } */
+/* { dg-do run } */
 /* { dg-options "-O -fdump-tree-phiopt" } */
 
 #include 
 
+__attribute__ ((noipa, noinline))
 uint8_t three_minmax2 (uint8_t xc, uint8_t xm, uint8_t xy) {
uint8_t  xk;
 xc=~xc;
@@ -15,5 +16,17 @@ uint8_t three_minmax2 (uint8_t xc, uint8_t xm, uint8_t xy) {
 }