diff mbox series

[3/3] PHIOPT: Allow BIT_AND and BIT_IOR in early phiopt

Message ID 20230824191455.3547513-3-apinski@marvell.com
State Unresolved
Headers
Series [1/3] MATCH: Move `a ? one_zero : one_zero` matching after min/max matching |

Checks

Context Check Description
snail/gcc-patch-check warning Git am fail log

Commit Message

Andrew Pinski Aug. 24, 2023, 7:14 p.m. UTC 
  Now that MIN/MAX can sometimes be transformed into BIT_AND/BIT_IOR,
we should allow BIT_AND and BIT_IOR in the early phiopt.
Also we produce BIT_AND/BIT_IOR for things like `bool0 ? bool1 : 0`
which seems like a good thing to allow early on too.

OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions.

gcc/ChangeLog:

	* tree-ssa-phiopt.cc (phiopt_early_allow): Allow
	BIT_AND_EXPR and BIT_IOR_EXPR.
---
 gcc/tree-ssa-phiopt.cc | 3 +++
 1 file changed, 3 insertions(+)

Comments

Richard Biener Aug. 25, 2023, 6:46 a.m. UTC | #1 
On Thu, Aug 24, 2023 at 9:16 PM Andrew Pinski via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> Now that MIN/MAX can sometimes be transformed into BIT_AND/BIT_IOR,
> we should allow BIT_AND and BIT_IOR in the early phiopt.
> Also we produce BIT_AND/BIT_IOR for things like `bool0 ? bool1 : 0`
> which seems like a good thing to allow early on too.

Hum.

I think if we allow AND/IOR we should also allow XOR and NOT.

Can you add dumping for replacements we disallow?  I'm esp. curious
for those otherwise being "singleton".  I know when doing early phiopt
I wanted to be very conservative (also to reduce testsuite fallout), and
I was mostly interested in MIN/MAX which I then extended to similar
things like ABS.  But maybe we can revisit this if we understand which
cases we definitely do not want to do early?

> OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions.
>
> gcc/ChangeLog:
>
>         * tree-ssa-phiopt.cc (phiopt_early_allow): Allow
>         BIT_AND_EXPR and BIT_IOR_EXPR.
> ---
>  gcc/tree-ssa-phiopt.cc | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/gcc/tree-ssa-phiopt.cc b/gcc/tree-ssa-phiopt.cc
> index 54706f4c7e7..7e63fb115db 100644
> --- a/gcc/tree-ssa-phiopt.cc
> +++ b/gcc/tree-ssa-phiopt.cc
> @@ -469,6 +469,9 @@ phiopt_early_allow (gimple_seq &seq, gimple_match_op &op)
>      {
>        case MIN_EXPR:
>        case MAX_EXPR:
> +      /* MIN/MAX could be convert into these. */
> +      case BIT_IOR_EXPR:
> +      case BIT_AND_EXPR:
>        case ABS_EXPR:
>        case ABSU_EXPR:
>        case NEGATE_EXPR:
> --
> 2.31.1
>
Andrew Pinski Aug. 25, 2023, 11:11 p.m. UTC | #2 
On Thu, Aug 24, 2023 at 11:47 PM Richard Biener via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> On Thu, Aug 24, 2023 at 9:16 PM Andrew Pinski via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
> >
> > Now that MIN/MAX can sometimes be transformed into BIT_AND/BIT_IOR,
> > we should allow BIT_AND and BIT_IOR in the early phiopt.
> > Also we produce BIT_AND/BIT_IOR for things like `bool0 ? bool1 : 0`
> > which seems like a good thing to allow early on too.
>
> Hum.
>
> I think if we allow AND/IOR we should also allow XOR and NOT.

Yes, XOR and NOT most likely should be added too. Maybe even
comparisons without a conversion too.

>
> Can you add dumping for replacements we disallow?  I'm esp. curious
> for those otherwise being "singleton".  I know when doing early phiopt
> I wanted to be very conservative (also to reduce testsuite fallout), and
> I was mostly interested in MIN/MAX which I then extended to similar
> things like ABS.  But maybe we can revisit this if we understand which
> cases we definitely do not want to do early?

I have a patch which prints out the dumping of the result and will
submit it later today. In the next couple of days I will look into the
dump when compiling GCC to see if there are others that seem fine. It
might be the case where we want to reject only non single statement
ones (except for MIN/MAX were allowing 2 there too).

Thanks,
Andrew

>
> > OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions.
> >
> > gcc/ChangeLog:
> >
> >         * tree-ssa-phiopt.cc (phiopt_early_allow): Allow
> >         BIT_AND_EXPR and BIT_IOR_EXPR.
> > ---
> >  gcc/tree-ssa-phiopt.cc | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/gcc/tree-ssa-phiopt.cc b/gcc/tree-ssa-phiopt.cc
> > index 54706f4c7e7..7e63fb115db 100644
> > --- a/gcc/tree-ssa-phiopt.cc
> > +++ b/gcc/tree-ssa-phiopt.cc
> > @@ -469,6 +469,9 @@ phiopt_early_allow (gimple_seq &seq, gimple_match_op &op)
> >      {
> >        case MIN_EXPR:
> >        case MAX_EXPR:
> > +      /* MIN/MAX could be convert into these. */
> > +      case BIT_IOR_EXPR:
> > +      case BIT_AND_EXPR:
> >        case ABS_EXPR:
> >        case ABSU_EXPR:
> >        case NEGATE_EXPR:
> > --
> > 2.31.1
> >
diff mbox series

Patch

diff --git a/gcc/tree-ssa-phiopt.cc b/gcc/tree-ssa-phiopt.cc
index 54706f4c7e7..7e63fb115db 100644
--- a/gcc/tree-ssa-phiopt.cc
+++ b/gcc/tree-ssa-phiopt.cc
@@ -469,6 +469,9 @@  phiopt_early_allow (gimple_seq &seq, gimple_match_op &op)
     {
       case MIN_EXPR:
       case MAX_EXPR:
+      /* MIN/MAX could be convert into these. */
+      case BIT_IOR_EXPR:
+      case BIT_AND_EXPR:
       case ABS_EXPR:
       case ABSU_EXPR:
       case NEGATE_EXPR: