> Am 19.05.2024 um 01:12 schrieb Andrew Pinski <quic_apin...@quicinc.com>:
>
> The problem here is even if last_and_only_stmt returns a statement,
> the bb might still contain a phi node which defines a ssa name
> which is used in that statement so we need to add a check to make sure
> that the phi nodes are empty for the middle bbs in both the
> `CMP?MINMAX:MINMAX` case and the `CMP?MINMAX:B` cases.
Is that single arg PHIs or do we have an extra edge into the middle BB? I
think that might be unexpected, at least costing wise. Maybe
Also to some of the replacement code we have ?
> OK for trunk and backport to all open branches since r14-3827-g30e6ee074588ba
> was backported?
> Bootstrapped and tested on x86_64_linux-gnu with no regressions.
>
Ok
Richard
> PR tree-optimization/115143
>
> gcc/ChangeLog:
>
> * tree-ssa-phiopt.cc (minmax_replacement): Check for empty
> phi nodes for middle bbs for the case where middle bb is not empty.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.c-torture/compile/pr115143-1.c: New test.
> * gcc.c-torture/compile/pr115143-2.c: New test.
> * gcc.c-torture/compile/pr115143-3.c: New test.
>
> Signed-off-by: Andrew Pinski <quic_apin...@quicinc.com>
> ---
> .../gcc.c-torture/compile/pr115143-1.c | 21 +++++++++++++
> .../gcc.c-torture/compile/pr115143-2.c | 30 +++++++++++++++++++
> .../gcc.c-torture/compile/pr115143-3.c | 29 ++++++++++++++++++
> gcc/tree-ssa-phiopt.cc | 12 ++++++++
> 4 files changed, 92 insertions(+)
> create mode 100644 gcc/testsuite/gcc.c-torture/compile/pr115143-1.c
> create mode 100644 gcc/testsuite/gcc.c-torture/compile/pr115143-2.c
> create mode 100644 gcc/testsuite/gcc.c-torture/compile/pr115143-3.c
>
> diff --git a/gcc/testsuite/gcc.c-torture/compile/pr115143-1.c
> b/gcc/testsuite/gcc.c-torture/compile/pr115143-1.c
> new file mode 100644
> index 00000000000..5cb119ea432
> --- /dev/null
> +++ b/gcc/testsuite/gcc.c-torture/compile/pr115143-1.c
> @@ -0,0 +1,21 @@
> +/* PR tree-optimization/115143 */
> +/* This used to ICE.
> + minmax part of phiopt would transform,
> + would transform `a!=0?min(a, b) : 0` into `min(a,b)`
> + which was correct except b was defined by a phi in the inner
> + bb which was not handled. */
> +short a, d;
> +char b;
> +long c;
> +unsigned long e, f;
> +void g(unsigned long h) {
> + if (c ? e : b)
> + if (e)
> + if (d) {
> + a = f ? ({
> + unsigned long i = d ? f : 0, j = e ? h : 0;
> + i < j ? i : j;
> + }) : 0;
> + }
> +}
> +
> diff --git a/gcc/testsuite/gcc.c-torture/compile/pr115143-2.c
> b/gcc/testsuite/gcc.c-torture/compile/pr115143-2.c
> new file mode 100644
> index 00000000000..05c3bbe9738
> --- /dev/null
> +++ b/gcc/testsuite/gcc.c-torture/compile/pr115143-2.c
> @@ -0,0 +1,30 @@
> +/* { dg-options "-fgimple" } */
> +/* PR tree-optimization/115143 */
> +/* This used to ICE.
> + minmax part of phiopt would transform,
> + would transform `a!=0?min(a, b) : 0` into `min(a,b)`
> + which was correct except b was defined by a phi in the inner
> + bb which was not handled. */
> +unsigned __GIMPLE (ssa,startwith("phiopt"))
> +foo (unsigned a, unsigned b)
> +{
> + unsigned j;
> + unsigned _23;
> + unsigned _12;
> +
> + __BB(2):
> + if (a_6(D) != 0u)
> + goto __BB3;
> + else
> + goto __BB4;
> +
> + __BB(3):
> + j_10 = __PHI (__BB2: b_11(D));
> + _23 = __MIN (a_6(D), j_10);
> + goto __BB4;
> +
> + __BB(4):
> + _12 = __PHI (__BB3: _23, __BB2: 0u);
> + return _12;
> +
> +}
> diff --git a/gcc/testsuite/gcc.c-torture/compile/pr115143-3.c
> b/gcc/testsuite/gcc.c-torture/compile/pr115143-3.c
> new file mode 100644
> index 00000000000..53c5fb5588e
> --- /dev/null
> +++ b/gcc/testsuite/gcc.c-torture/compile/pr115143-3.c
> @@ -0,0 +1,29 @@
> +/* { dg-options "-fgimple" } */
> +/* PR tree-optimization/115143 */
> +/* This used to ICE.
> + minmax part of phiopt would transform,
> + would transform `a!=0?min(a, b) : 0` into `min(a,b)`
> + which was correct except b was defined by a phi in the inner
> + bb which was not handled. */
> +unsigned __GIMPLE (ssa,startwith("phiopt"))
> +foo (unsigned a, unsigned b)
> +{
> + unsigned j;
> + unsigned _23;
> + unsigned _12;
> +
> + __BB(2):
> + if (a_6(D) > 0u)
> + goto __BB3;
> + else
> + goto __BB4;
> +
> + __BB(3):
> + j_10 = __PHI (__BB2: b_7(D));
> + _23 = __MIN (a_6(D), j_10);
> + goto __BB4;
> +
> + __BB(4):
> + _12 = __PHI (__BB3: _23, __BB2: 0u);
> + return _12;
> +}
> diff --git a/gcc/tree-ssa-phiopt.cc b/gcc/tree-ssa-phiopt.cc
> index f166c3132cb..918cf50b589 100644
> --- a/gcc/tree-ssa-phiopt.cc
> +++ b/gcc/tree-ssa-phiopt.cc
> @@ -1925,6 +1925,10 @@ minmax_replacement (basic_block cond_bb, basic_block
> middle_bb, basic_block alt_
> || gimple_code (assign) != GIMPLE_ASSIGN)
> return false;
>
> + /* There cannot be any phi nodes in the middle bb. */
> + if (!gimple_seq_empty_p (phi_nodes (middle_bb)))
> + return false;
> +
> lhs = gimple_assign_lhs (assign);
> ass_code = gimple_assign_rhs_code (assign);
> if (ass_code != MAX_EXPR && ass_code != MIN_EXPR)
> @@ -1938,6 +1942,10 @@ minmax_replacement (basic_block cond_bb, basic_block
> middle_bb, basic_block alt_
> || gimple_code (assign) != GIMPLE_ASSIGN)
> return false;
>
> + /* There cannot be any phi nodes in the alt middle bb. */
> + if (!gimple_seq_empty_p (phi_nodes (alt_middle_bb)))
> + return false;
> +
> alt_lhs = gimple_assign_lhs (assign);
> if (ass_code != gimple_assign_rhs_code (assign))
> return false;
> @@ -2049,6 +2057,10 @@ minmax_replacement (basic_block cond_bb, basic_block
> middle_bb, basic_block alt_
> || gimple_code (assign) != GIMPLE_ASSIGN)
> return false;
>
> + /* There cannot be any phi nodes in the middle bb. */
> + if (!gimple_seq_empty_p (phi_nodes (middle_bb)))
> + return false;
> +
> lhs = gimple_assign_lhs (assign);
> ass_code = gimple_assign_rhs_code (assign);
> if (ass_code != MAX_EXPR && ass_code != MIN_EXPR)
> --
> 2.43.0
>
