On Sun, Oct 1, 2023 at 1:23 PM Andrew Pinski <pins...@gmail.com> wrote:
>
> From: Andrew Pinski <apin...@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
>
