* [COMMITTED/13] Fix PR 111331: wrong code for `a > 28 ? MIN<a, 28> : 29`
@ 2023-10-01 20:23 Andrew Pinski
2024-05-08 16:26 ` Andrew Pinski
0 siblings, 1 reply; 2+ messages in thread
From: Andrew Pinski @ 2023-10-01 20:23 UTC (permalink / raw)
To: gcc-patches; +Cc: Andrew Pinski
From: Andrew Pinski <apinski@marvell.com>
The problem here is after r6-7425-ga9fee7cdc3c62d0e51730,
the comparison to see if the transformation could be done was using the
wrong value. Instead of see if the inner was LE (for MIN and GE for MAX)
the outer value, it was comparing the inner to the value used in the comparison
which was wrong.
Committed to GCC 13 branch after bootstrapped and tested on x86_64-linux-gnu.
gcc/ChangeLog:
PR tree-optimization/111331
* tree-ssa-phiopt.cc (minmax_replacement):
Fix the LE/GE comparison for the
`(a CMP CST1) ? max<a,CST2> : a` optimization.
gcc/testsuite/ChangeLog:
PR tree-optimization/111331
* gcc.c-torture/execute/pr111331-1.c: New test.
* gcc.c-torture/execute/pr111331-2.c: New test.
* gcc.c-torture/execute/pr111331-3.c: New test.
(cherry picked from commit 30e6ee074588bacefd2dfe745b188bb20c81fe5e)
---
.../gcc.c-torture/execute/pr111331-1.c | 17 +++++++++++++++++
.../gcc.c-torture/execute/pr111331-2.c | 19 +++++++++++++++++++
.../gcc.c-torture/execute/pr111331-3.c | 15 +++++++++++++++
gcc/tree-ssa-phiopt.cc | 8 ++++----
4 files changed, 55 insertions(+), 4 deletions(-)
create mode 100644 gcc/testsuite/gcc.c-torture/execute/pr111331-1.c
create mode 100644 gcc/testsuite/gcc.c-torture/execute/pr111331-2.c
create mode 100644 gcc/testsuite/gcc.c-torture/execute/pr111331-3.c
diff --git a/gcc/testsuite/gcc.c-torture/execute/pr111331-1.c b/gcc/testsuite/gcc.c-torture/execute/pr111331-1.c
new file mode 100644
index 00000000000..4c7f4fdbaa9
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/execute/pr111331-1.c
@@ -0,0 +1,17 @@
+int a;
+int b;
+int c(int d, int e, int f) {
+ if (d < e)
+ return e;
+ if (d > f)
+ return f;
+ return d;
+}
+int main() {
+ int g = -1;
+ a = c(b + 30, 29, g + 29);
+ volatile t = a;
+ if (t != 28)
+ __builtin_abort();
+ return 0;
+}
diff --git a/gcc/testsuite/gcc.c-torture/execute/pr111331-2.c b/gcc/testsuite/gcc.c-torture/execute/pr111331-2.c
new file mode 100644
index 00000000000..5c677f2caa9
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/execute/pr111331-2.c
@@ -0,0 +1,19 @@
+
+int a;
+int b;
+
+int main() {
+ int d = b+30;
+ {
+ int t;
+ if (d < 29)
+ t = 29;
+ else
+ t = (d > 28) ? 28 : d;
+ a = t;
+ }
+ volatile int t = a;
+ if (a != 28)
+ __builtin_abort();
+ return 0;
+}
diff --git a/gcc/testsuite/gcc.c-torture/execute/pr111331-3.c b/gcc/testsuite/gcc.c-torture/execute/pr111331-3.c
new file mode 100644
index 00000000000..213d9bdd539
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/execute/pr111331-3.c
@@ -0,0 +1,15 @@
+int a;
+int b;
+
+int main() {
+ int d = b+30;
+ {
+ int t;
+ t = d < 29 ? 29 : ((d > 28) ? 28 : d);
+ a = t;
+ }
+ volatile int t = a;
+ if (a != 28)
+ __builtin_abort();
+ return 0;
+}
diff --git a/gcc/tree-ssa-phiopt.cc b/gcc/tree-ssa-phiopt.cc
index a7ab6ce4ad9..c3d78d1400b 100644
--- a/gcc/tree-ssa-phiopt.cc
+++ b/gcc/tree-ssa-phiopt.cc
@@ -2270,7 +2270,7 @@ minmax_replacement (basic_block cond_bb, basic_block middle_bb, basic_block alt_
/* We need BOUND <= LARGER. */
if (!integer_nonzerop (fold_build2 (LE_EXPR, boolean_type_node,
- bound, larger)))
+ bound, arg_false)))
return false;
}
else if (operand_equal_for_phi_arg_p (arg_false, smaller)
@@ -2301,7 +2301,7 @@ minmax_replacement (basic_block cond_bb, basic_block middle_bb, basic_block alt_
/* We need BOUND >= SMALLER. */
if (!integer_nonzerop (fold_build2 (GE_EXPR, boolean_type_node,
- bound, smaller)))
+ bound, arg_false)))
return false;
}
else
@@ -2341,7 +2341,7 @@ minmax_replacement (basic_block cond_bb, basic_block middle_bb, basic_block alt_
/* We need BOUND >= LARGER. */
if (!integer_nonzerop (fold_build2 (GE_EXPR, boolean_type_node,
- bound, larger)))
+ bound, arg_true)))
return false;
}
else if (operand_equal_for_phi_arg_p (arg_true, smaller)
@@ -2368,7 +2368,7 @@ minmax_replacement (basic_block cond_bb, basic_block middle_bb, basic_block alt_
/* We need BOUND <= SMALLER. */
if (!integer_nonzerop (fold_build2 (LE_EXPR, boolean_type_node,
- bound, smaller)))
+ bound, arg_true)))
return false;
}
else
--
2.39.3
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [COMMITTED/13] Fix PR 111331: wrong code for `a > 28 ? MIN<a, 28> : 29`
2023-10-01 20:23 [COMMITTED/13] Fix PR 111331: wrong code for `a > 28 ? MIN<a, 28> : 29` Andrew Pinski
@ 2024-05-08 16:26 ` Andrew Pinski
0 siblings, 0 replies; 2+ messages in thread
From: Andrew Pinski @ 2024-05-08 16:26 UTC (permalink / raw)
To: gcc-patches; +Cc: Andrew Pinski
On Sun, Oct 1, 2023 at 1:23 PM Andrew Pinski <pinskia@gmail.com> wrote:
>
> From: Andrew Pinski <apinski@marvell.com>
>
> The problem here is after r6-7425-ga9fee7cdc3c62d0e51730,
> the comparison to see if the transformation could be done was using the
> wrong value. Instead of see if the inner was LE (for MIN and GE for MAX)
> the outer value, it was comparing the inner to the value used in the comparison
> which was wrong.
>
> Committed to GCC 13 branch after bootstrapped and tested on x86_64-linux-gnu.
Committed also to GCC 12 and 11 branches.
>
> gcc/ChangeLog:
>
> PR tree-optimization/111331
> * tree-ssa-phiopt.cc (minmax_replacement):
> Fix the LE/GE comparison for the
> `(a CMP CST1) ? max<a,CST2> : a` optimization.
>
> gcc/testsuite/ChangeLog:
>
> PR tree-optimization/111331
> * gcc.c-torture/execute/pr111331-1.c: New test.
> * gcc.c-torture/execute/pr111331-2.c: New test.
> * gcc.c-torture/execute/pr111331-3.c: New test.
>
> (cherry picked from commit 30e6ee074588bacefd2dfe745b188bb20c81fe5e)
> ---
> .../gcc.c-torture/execute/pr111331-1.c | 17 +++++++++++++++++
> .../gcc.c-torture/execute/pr111331-2.c | 19 +++++++++++++++++++
> .../gcc.c-torture/execute/pr111331-3.c | 15 +++++++++++++++
> gcc/tree-ssa-phiopt.cc | 8 ++++----
> 4 files changed, 55 insertions(+), 4 deletions(-)
> create mode 100644 gcc/testsuite/gcc.c-torture/execute/pr111331-1.c
> create mode 100644 gcc/testsuite/gcc.c-torture/execute/pr111331-2.c
> create mode 100644 gcc/testsuite/gcc.c-torture/execute/pr111331-3.c
>
> diff --git a/gcc/testsuite/gcc.c-torture/execute/pr111331-1.c b/gcc/testsuite/gcc.c-torture/execute/pr111331-1.c
> new file mode 100644
> index 00000000000..4c7f4fdbaa9
> --- /dev/null
> +++ b/gcc/testsuite/gcc.c-torture/execute/pr111331-1.c
> @@ -0,0 +1,17 @@
> +int a;
> +int b;
> +int c(int d, int e, int f) {
> + if (d < e)
> + return e;
> + if (d > f)
> + return f;
> + return d;
> +}
> +int main() {
> + int g = -1;
> + a = c(b + 30, 29, g + 29);
> + volatile t = a;
> + if (t != 28)
> + __builtin_abort();
> + return 0;
> +}
> diff --git a/gcc/testsuite/gcc.c-torture/execute/pr111331-2.c b/gcc/testsuite/gcc.c-torture/execute/pr111331-2.c
> new file mode 100644
> index 00000000000..5c677f2caa9
> --- /dev/null
> +++ b/gcc/testsuite/gcc.c-torture/execute/pr111331-2.c
> @@ -0,0 +1,19 @@
> +
> +int a;
> +int b;
> +
> +int main() {
> + int d = b+30;
> + {
> + int t;
> + if (d < 29)
> + t = 29;
> + else
> + t = (d > 28) ? 28 : d;
> + a = t;
> + }
> + volatile int t = a;
> + if (a != 28)
> + __builtin_abort();
> + return 0;
> +}
> diff --git a/gcc/testsuite/gcc.c-torture/execute/pr111331-3.c b/gcc/testsuite/gcc.c-torture/execute/pr111331-3.c
> new file mode 100644
> index 00000000000..213d9bdd539
> --- /dev/null
> +++ b/gcc/testsuite/gcc.c-torture/execute/pr111331-3.c
> @@ -0,0 +1,15 @@
> +int a;
> +int b;
> +
> +int main() {
> + int d = b+30;
> + {
> + int t;
> + t = d < 29 ? 29 : ((d > 28) ? 28 : d);
> + a = t;
> + }
> + volatile int t = a;
> + if (a != 28)
> + __builtin_abort();
> + return 0;
> +}
> diff --git a/gcc/tree-ssa-phiopt.cc b/gcc/tree-ssa-phiopt.cc
> index a7ab6ce4ad9..c3d78d1400b 100644
> --- a/gcc/tree-ssa-phiopt.cc
> +++ b/gcc/tree-ssa-phiopt.cc
> @@ -2270,7 +2270,7 @@ minmax_replacement (basic_block cond_bb, basic_block middle_bb, basic_block alt_
>
> /* We need BOUND <= LARGER. */
> if (!integer_nonzerop (fold_build2 (LE_EXPR, boolean_type_node,
> - bound, larger)))
> + bound, arg_false)))
> return false;
> }
> else if (operand_equal_for_phi_arg_p (arg_false, smaller)
> @@ -2301,7 +2301,7 @@ minmax_replacement (basic_block cond_bb, basic_block middle_bb, basic_block alt_
>
> /* We need BOUND >= SMALLER. */
> if (!integer_nonzerop (fold_build2 (GE_EXPR, boolean_type_node,
> - bound, smaller)))
> + bound, arg_false)))
> return false;
> }
> else
> @@ -2341,7 +2341,7 @@ minmax_replacement (basic_block cond_bb, basic_block middle_bb, basic_block alt_
>
> /* We need BOUND >= LARGER. */
> if (!integer_nonzerop (fold_build2 (GE_EXPR, boolean_type_node,
> - bound, larger)))
> + bound, arg_true)))
> return false;
> }
> else if (operand_equal_for_phi_arg_p (arg_true, smaller)
> @@ -2368,7 +2368,7 @@ minmax_replacement (basic_block cond_bb, basic_block middle_bb, basic_block alt_
>
> /* We need BOUND <= SMALLER. */
> if (!integer_nonzerop (fold_build2 (LE_EXPR, boolean_type_node,
> - bound, smaller)))
> + bound, arg_true)))
> return false;
> }
> else
> --
> 2.39.3
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2024-05-08 16:27 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-01 20:23 [COMMITTED/13] Fix PR 111331: wrong code for `a > 28 ? MIN<a, 28> : 29` Andrew Pinski
2024-05-08 16:26 ` 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).