diff mbox series

PHIOPT: Don't transform minmax if middle bb contains a phi [PR115143]

Message ID 20240518231146.954808-1-quic_apinski@quicinc.com
State New
Headers show
Series PHIOPT: Don't transform minmax if middle bb contains a phi [PR115143] | expand

Commit Message

Andrew Pinski May 18, 2024, 11:11 p.m. UTC
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.

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.

	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_apinski@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

Comments

Richard Biener May 19, 2024, 6:54 p.m. UTC | #1
> Am 19.05.2024 um 01:12 schrieb Andrew Pinski <quic_apinski@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_apinski@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
>
Andrew Pinski May 20, 2024, 9:37 p.m. UTC | #2
> -----Original Message-----
> From: Richard Biener <richard.guenther@gmail.com>
> Sent: Sunday, May 19, 2024 11:55 AM
> To: Andrew Pinski (QUIC) <quic_apinski@quicinc.com>
> Cc: gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH] PHIOPT: Don't transform minmax if
> middle bb contains a phi [PR115143]
> 
> 
> 
> > Am 19.05.2024 um 01:12 schrieb Andrew Pinski
> <quic_apinski@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 ?

It is only a single arg PHI since we already reject multiple edges in the middle BBs for these cases.
It was EVPR that produces the single arg PHI in the original testcase from folding of a conditional to false and evpr does not do simple name prop in this case and there is no pass inbetween evrp and phiopt that will clear up single arg PHI.
I added the Gimple based testcases basically to avoid the needing of depending on what previous passes could produce too.

> 
> > 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

Does this include the GCC 13 branch or should I wait until after the GCC 13.3.0 release?

Thanks,
Andrew Pinski

> 
> 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_apinski@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
> >
Richard Biener May 21, 2024, 6:07 a.m. UTC | #3
On Mon, May 20, 2024 at 11:37 PM Andrew Pinski (QUIC)
<quic_apinski@quicinc.com> wrote:
>
> > -----Original Message-----
> > From: Richard Biener <richard.guenther@gmail.com>
> > Sent: Sunday, May 19, 2024 11:55 AM
> > To: Andrew Pinski (QUIC) <quic_apinski@quicinc.com>
> > Cc: gcc-patches@gcc.gnu.org
> > Subject: Re: [PATCH] PHIOPT: Don't transform minmax if
> > middle bb contains a phi [PR115143]
> >
> >
> >
> > > Am 19.05.2024 um 01:12 schrieb Andrew Pinski
> > <quic_apinski@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 ?
>
> It is only a single arg PHI since we already reject multiple edges in the middle BBs for these cases.
> It was EVPR that produces the single arg PHI in the original testcase from folding of a conditional to false and evpr does not do simple name prop in this case and there is no pass inbetween evrp and phiopt that will clear up single arg PHI.
> I added the Gimple based testcases basically to avoid the needing of depending on what previous passes could produce too.
>
> >
> > > 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
>
> Does this include the GCC 13 branch or should I wait until after the GCC 13.3.0 release?

Please wait until after the release.

Thanks,
Richard.

> Thanks,
> Andrew Pinski
>
> >
> > 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_apinski@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
> > >
diff mbox series

Patch

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)