public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH 1/3] MATCH: Move `a ? one_zero : one_zero` matching after min/max matching
@ 2023-08-24 19:14 Andrew Pinski
  2023-08-24 19:14 ` [PATCH 2/3] MATCH: `a | C -> C` when we know that `a & ~C == 0` Andrew Pinski
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Andrew Pinski @ 2023-08-24 19:14 UTC (permalink / raw)
  To: gcc-patches; +Cc: Andrew Pinski

In PR 106677, I noticed that on the trunk we were producing:
```
  _25 = SR.116_117 == 0;
  _27 = (unsigned char) _25;
  _32 = _27 | SR.116_117;
```
From `SR.115_117 != 0 ? SR.115_117 : 1`
Rather than:
```
  _119 = MAX_EXPR <1, SR.115_117>;
```
Or (rather)
```
  _119 = SR.115_117 | 1;
```
Due to the order of the patterns.

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

gcc/ChangeLog:

	* match.pd (`a ? one_zero : one_zero`): Move
	below detection of minmax.
---
 gcc/match.pd | 38 ++++++++++++++++++++------------------
 1 file changed, 20 insertions(+), 18 deletions(-)

diff --git a/gcc/match.pd b/gcc/match.pd
index 890f050cbad..c87a0795667 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -4950,24 +4950,6 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
  )
 )
 
-(simplify
- (cond @0 zero_one_valued_p@1 zero_one_valued_p@2)
- (switch
-  /* bool0 ? bool1 : 0 -> bool0 & bool1 */
-  (if (integer_zerop (@2))
-   (bit_and (convert @0) @1))
-  /* bool0 ? 0 : bool2 -> (bool0^1) & bool2 */
-  (if (integer_zerop (@1))
-   (bit_and (bit_xor (convert @0) { build_one_cst (type); } ) @2))
-  /* bool0 ? 1 : bool2 -> bool0 | bool2 */
-  (if (integer_onep (@1))
-   (bit_ior (convert @0) @2))
-  /* bool0 ? bool1 : 1 -> (bool0^1) | bool1 */
-  (if (integer_onep (@2))
-   (bit_ior (bit_xor (convert @0) @2) @1))
- )
-)
-
 /* Optimize
    # x_5 in range [cst1, cst2] where cst2 = cst1 + 1
    x_5 ? cstN ? cst4 : cst3
@@ -5298,6 +5280,26 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
           && integer_nonzerop (fold_build2 (GE_EXPR, boolean_type_node, @3, @1)))
       (max @2 @4))))))
 
+#if GIMPLE
+(simplify
+ (cond @0 zero_one_valued_p@1 zero_one_valued_p@2)
+ (switch
+  /* bool0 ? bool1 : 0 -> bool0 & bool1 */
+  (if (integer_zerop (@2))
+   (bit_and (convert @0) @1))
+  /* bool0 ? 0 : bool2 -> (bool0^1) & bool2 */
+  (if (integer_zerop (@1))
+   (bit_and (bit_xor (convert @0) { build_one_cst (type); } ) @2))
+  /* bool0 ? 1 : bool2 -> bool0 | bool2 */
+  (if (integer_onep (@1))
+   (bit_ior (convert @0) @2))
+  /* bool0 ? bool1 : 1 -> (bool0^1) | bool1 */
+  (if (integer_onep (@2))
+   (bit_ior (bit_xor (convert @0) @2) @1))
+ )
+)
+#endif
+
 /* X != C1 ? -X : C2 simplifies to -X when -C1 == C2.  */
 (simplify
  (cond (ne @0 INTEGER_CST@1) (negate@3 @0) INTEGER_CST@2)
-- 
2.31.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 2/3] MATCH: `a | C -> C` when we know that `a & ~C == 0`
  2023-08-24 19:14 [PATCH 1/3] MATCH: Move `a ? one_zero : one_zero` matching after min/max matching Andrew Pinski
@ 2023-08-24 19:14 ` Andrew Pinski
  2023-08-25  6:36   ` Richard Biener
  2023-08-24 19:14 ` [PATCH 3/3] PHIOPT: Allow BIT_AND and BIT_IOR in early phiopt Andrew Pinski
  2023-08-25  6:38 ` [PATCH 1/3] MATCH: Move `a ? one_zero : one_zero` matching after min/max matching Richard Biener
  2 siblings, 1 reply; 10+ messages in thread
From: Andrew Pinski @ 2023-08-24 19:14 UTC (permalink / raw)
  To: gcc-patches; +Cc: Andrew Pinski

Even though this is handled by other code inside both VRP and CCP,
sometimes we want to optimize this outside of VRP and CCP.
An example is given in PR 106677 where phiopt will happen
after VRP (which removes a cast for a comparison) and then
phiopt will optimize the phi to be `a | 1` which can then
be optimized to `1` due to this patch.

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

Note Similar code already exists in simplify_rtx for the RTL level;
it was moved from combine to simplify_rtx in r0-72539-gbd1ef757767f6d.
gcc/ChangeLog:

	* match.pd (`a | C -> C`): New pattern.
---
 gcc/match.pd | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/gcc/match.pd b/gcc/match.pd
index c87a0795667..3bbeceb37b4 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -1456,6 +1456,12 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
  (if (INTEGRAL_TYPE_P (TREE_TYPE (@0))
       && wi::bit_and_not (get_nonzero_bits (@0), wi::to_wide (@1)) == 0)
   @0))
+/* x | C -> C if we know that x & ~C == 0.  */
+(simplify
+ (bit_ior SSA_NAME@0 INTEGER_CST@1)
+ (if (INTEGRAL_TYPE_P (TREE_TYPE (@0))
+      && wi::bit_and_not (get_nonzero_bits (@0), wi::to_wide (@1)) == 0)
+  @1))
 #endif
 
 /* ~(~X - Y) -> X + Y and ~(~X + Y) -> X - Y.  */
-- 
2.31.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 3/3] PHIOPT: Allow BIT_AND and BIT_IOR in early phiopt
  2023-08-24 19:14 [PATCH 1/3] MATCH: Move `a ? one_zero : one_zero` matching after min/max matching Andrew Pinski
  2023-08-24 19:14 ` [PATCH 2/3] MATCH: `a | C -> C` when we know that `a & ~C == 0` Andrew Pinski
@ 2023-08-24 19:14 ` Andrew Pinski
  2023-08-25  6:46   ` Richard Biener
  2023-08-25  6:38 ` [PATCH 1/3] MATCH: Move `a ? one_zero : one_zero` matching after min/max matching Richard Biener
  2 siblings, 1 reply; 10+ messages in thread
From: Andrew Pinski @ 2023-08-24 19:14 UTC (permalink / raw)
  To: gcc-patches; +Cc: Andrew Pinski

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(+)

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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/3] MATCH: `a | C -> C` when we know that `a & ~C == 0`
  2023-08-24 19:14 ` [PATCH 2/3] MATCH: `a | C -> C` when we know that `a & ~C == 0` Andrew Pinski
@ 2023-08-25  6:36   ` Richard Biener
  2023-08-25  7:40     ` Andrew Pinski
  0 siblings, 1 reply; 10+ messages in thread
From: Richard Biener @ 2023-08-25  6:36 UTC (permalink / raw)
  To: Andrew Pinski; +Cc: gcc-patches

On Thu, Aug 24, 2023 at 9:16 PM Andrew Pinski via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> Even though this is handled by other code inside both VRP and CCP,
> sometimes we want to optimize this outside of VRP and CCP.
> An example is given in PR 106677 where phiopt will happen
> after VRP (which removes a cast for a comparison) and then
> phiopt will optimize the phi to be `a | 1` which can then
> be optimized to `1` due to this patch.

Also works for xor, no?

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

OK with or without adding XOR.

Richard.

> Note Similar code already exists in simplify_rtx for the RTL level;
> it was moved from combine to simplify_rtx in r0-72539-gbd1ef757767f6d.
> gcc/ChangeLog:
>
>         * match.pd (`a | C -> C`): New pattern.
> ---
>  gcc/match.pd | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/gcc/match.pd b/gcc/match.pd
> index c87a0795667..3bbeceb37b4 100644
> --- a/gcc/match.pd
> +++ b/gcc/match.pd
> @@ -1456,6 +1456,12 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>   (if (INTEGRAL_TYPE_P (TREE_TYPE (@0))
>        && wi::bit_and_not (get_nonzero_bits (@0), wi::to_wide (@1)) == 0)
>    @0))
> +/* x | C -> C if we know that x & ~C == 0.  */
> +(simplify
> + (bit_ior SSA_NAME@0 INTEGER_CST@1)
> + (if (INTEGRAL_TYPE_P (TREE_TYPE (@0))
> +      && wi::bit_and_not (get_nonzero_bits (@0), wi::to_wide (@1)) == 0)
> +  @1))
>  #endif
>
>  /* ~(~X - Y) -> X + Y and ~(~X + Y) -> X - Y.  */
> --
> 2.31.1
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/3] MATCH: Move `a ? one_zero : one_zero` matching after min/max matching
  2023-08-24 19:14 [PATCH 1/3] MATCH: Move `a ? one_zero : one_zero` matching after min/max matching Andrew Pinski
  2023-08-24 19:14 ` [PATCH 2/3] MATCH: `a | C -> C` when we know that `a & ~C == 0` Andrew Pinski
  2023-08-24 19:14 ` [PATCH 3/3] PHIOPT: Allow BIT_AND and BIT_IOR in early phiopt Andrew Pinski
@ 2023-08-25  6:38 ` Richard Biener
  2023-08-25 18:11   ` Andrew Pinski
  2 siblings, 1 reply; 10+ messages in thread
From: Richard Biener @ 2023-08-25  6:38 UTC (permalink / raw)
  To: Andrew Pinski; +Cc: gcc-patches

On Thu, Aug 24, 2023 at 9:16 PM Andrew Pinski via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> In PR 106677, I noticed that on the trunk we were producing:
> ```
>   _25 = SR.116_117 == 0;
>   _27 = (unsigned char) _25;
>   _32 = _27 | SR.116_117;
> ```
> From `SR.115_117 != 0 ? SR.115_117 : 1`
> Rather than:
> ```
>   _119 = MAX_EXPR <1, SR.115_117>;
> ```
> Or (rather)
> ```
>   _119 = SR.115_117 | 1;
> ```
> Due to the order of the patterns.

Hmm, that means the former when present in source isn't optimized?

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

OK, but please add a comment indicating the ordering requirement.

Can you also add a testcase?

Richard.

> gcc/ChangeLog:
>
>         * match.pd (`a ? one_zero : one_zero`): Move
>         below detection of minmax.
> ---
>  gcc/match.pd | 38 ++++++++++++++++++++------------------
>  1 file changed, 20 insertions(+), 18 deletions(-)
>
> diff --git a/gcc/match.pd b/gcc/match.pd
> index 890f050cbad..c87a0795667 100644
> --- a/gcc/match.pd
> +++ b/gcc/match.pd
> @@ -4950,24 +4950,6 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>   )
>  )
>
> -(simplify
> - (cond @0 zero_one_valued_p@1 zero_one_valued_p@2)
> - (switch
> -  /* bool0 ? bool1 : 0 -> bool0 & bool1 */
> -  (if (integer_zerop (@2))
> -   (bit_and (convert @0) @1))
> -  /* bool0 ? 0 : bool2 -> (bool0^1) & bool2 */
> -  (if (integer_zerop (@1))
> -   (bit_and (bit_xor (convert @0) { build_one_cst (type); } ) @2))
> -  /* bool0 ? 1 : bool2 -> bool0 | bool2 */
> -  (if (integer_onep (@1))
> -   (bit_ior (convert @0) @2))
> -  /* bool0 ? bool1 : 1 -> (bool0^1) | bool1 */
> -  (if (integer_onep (@2))
> -   (bit_ior (bit_xor (convert @0) @2) @1))
> - )
> -)
> -
>  /* Optimize
>     # x_5 in range [cst1, cst2] where cst2 = cst1 + 1
>     x_5 ? cstN ? cst4 : cst3
> @@ -5298,6 +5280,26 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>            && integer_nonzerop (fold_build2 (GE_EXPR, boolean_type_node, @3, @1)))
>        (max @2 @4))))))
>
> +#if GIMPLE
> +(simplify
> + (cond @0 zero_one_valued_p@1 zero_one_valued_p@2)
> + (switch
> +  /* bool0 ? bool1 : 0 -> bool0 & bool1 */
> +  (if (integer_zerop (@2))
> +   (bit_and (convert @0) @1))
> +  /* bool0 ? 0 : bool2 -> (bool0^1) & bool2 */
> +  (if (integer_zerop (@1))
> +   (bit_and (bit_xor (convert @0) { build_one_cst (type); } ) @2))
> +  /* bool0 ? 1 : bool2 -> bool0 | bool2 */
> +  (if (integer_onep (@1))
> +   (bit_ior (convert @0) @2))
> +  /* bool0 ? bool1 : 1 -> (bool0^1) | bool1 */
> +  (if (integer_onep (@2))
> +   (bit_ior (bit_xor (convert @0) @2) @1))
> + )
> +)
> +#endif
> +
>  /* X != C1 ? -X : C2 simplifies to -X when -C1 == C2.  */
>  (simplify
>   (cond (ne @0 INTEGER_CST@1) (negate@3 @0) INTEGER_CST@2)
> --
> 2.31.1
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 3/3] PHIOPT: Allow BIT_AND and BIT_IOR in early phiopt
  2023-08-24 19:14 ` [PATCH 3/3] PHIOPT: Allow BIT_AND and BIT_IOR in early phiopt Andrew Pinski
@ 2023-08-25  6:46   ` Richard Biener
  2023-08-25 23:11     ` Andrew Pinski
  0 siblings, 1 reply; 10+ messages in thread
From: Richard Biener @ 2023-08-25  6:46 UTC (permalink / raw)
  To: Andrew Pinski; +Cc: gcc-patches

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
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/3] MATCH: `a | C -> C` when we know that `a & ~C == 0`
  2023-08-25  6:36   ` Richard Biener
@ 2023-08-25  7:40     ` Andrew Pinski
  0 siblings, 0 replies; 10+ messages in thread
From: Andrew Pinski @ 2023-08-25  7:40 UTC (permalink / raw)
  To: Richard Biener; +Cc: Andrew Pinski, gcc-patches

On Thu, Aug 24, 2023 at 11:37 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:
> >
> > Even though this is handled by other code inside both VRP and CCP,
> > sometimes we want to optimize this outside of VRP and CCP.
> > An example is given in PR 106677 where phiopt will happen
> > after VRP (which removes a cast for a comparison) and then
> > phiopt will optimize the phi to be `a | 1` which can then
> > be optimized to `1` due to this patch.
>
> Also works for xor, no?

No, because IOR is a saturation operation while XOR is not. So if you
know that x and C are full subsets (nonzero(x) & ~nonzero(C) == 0)
then A^C is not the constant C but rather just (~A)&C which we already
a pattern for that to turn it back in to A^C:
/* Simplify (~X & Y) to X ^ Y if we know that (X & ~Y) is 0.  */

The only thing you can do for XOR is that if you have `A ^ B` and A
and B are known not to share any bits in common (that is nonzero(A) &
nonzero(B) == 0), you can convert it to `A | B` (that is what
simplify-rtx.cc does). Which looks like we don't do on the gimple
level.

>
> > OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions.
>
> OK with or without adding XOR.

Ok.

Thanks,
Andrew

>
> Richard.
>
> > Note Similar code already exists in simplify_rtx for the RTL level;
> > it was moved from combine to simplify_rtx in r0-72539-gbd1ef757767f6d.
> > gcc/ChangeLog:
> >
> >         * match.pd (`a | C -> C`): New pattern.
> > ---
> >  gcc/match.pd | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/gcc/match.pd b/gcc/match.pd
> > index c87a0795667..3bbeceb37b4 100644
> > --- a/gcc/match.pd
> > +++ b/gcc/match.pd
> > @@ -1456,6 +1456,12 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> >   (if (INTEGRAL_TYPE_P (TREE_TYPE (@0))
> >        && wi::bit_and_not (get_nonzero_bits (@0), wi::to_wide (@1)) == 0)
> >    @0))
> > +/* x | C -> C if we know that x & ~C == 0.  */
> > +(simplify
> > + (bit_ior SSA_NAME@0 INTEGER_CST@1)
> > + (if (INTEGRAL_TYPE_P (TREE_TYPE (@0))
> > +      && wi::bit_and_not (get_nonzero_bits (@0), wi::to_wide (@1)) == 0)
> > +  @1))
> >  #endif
> >
> >  /* ~(~X - Y) -> X + Y and ~(~X + Y) -> X - Y.  */
> > --
> > 2.31.1
> >

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/3] MATCH: Move `a ? one_zero : one_zero` matching after min/max matching
  2023-08-25  6:38 ` [PATCH 1/3] MATCH: Move `a ? one_zero : one_zero` matching after min/max matching Richard Biener
@ 2023-08-25 18:11   ` Andrew Pinski
  2023-08-25 18:18     ` Andrew Pinski
  0 siblings, 1 reply; 10+ messages in thread
From: Andrew Pinski @ 2023-08-25 18:11 UTC (permalink / raw)
  To: Richard Biener; +Cc: Andrew Pinski, gcc-patches

On Thu, Aug 24, 2023 at 11:39 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:
> >
> > In PR 106677, I noticed that on the trunk we were producing:
> > ```
> >   _25 = SR.116_117 == 0;
> >   _27 = (unsigned char) _25;
> >   _32 = _27 | SR.116_117;
> > ```
> > From `SR.115_117 != 0 ? SR.115_117 : 1`
> > Rather than:
> > ```
> >   _119 = MAX_EXPR <1, SR.115_117>;
> > ```
> > Or (rather)
> > ```
> >   _119 = SR.115_117 | 1;
> > ```
> > Due to the order of the patterns.
>
> Hmm, that means the former when present in source isn't optimized?

That it is correct; they are not optimized at the gimple level down to
1. it is sometimes (but not on all targets) optimized at the RTL level
though.

>
> > OK? Bootstrapped and tested on x86_64-linux-gnu with no
> > regressions.
>
> OK, but please add a comment indicating the ordering requirement.
>
> Can you also add a testcase?

Yes and yes. Will send out a new patch in a few minutes with the added
comment and testcase.

Thanks,
Andrew

>
> Richard.
>
> > gcc/ChangeLog:
> >
> >         * match.pd (`a ? one_zero : one_zero`): Move
> >         below detection of minmax.
> > ---
> >  gcc/match.pd | 38 ++++++++++++++++++++------------------
> >  1 file changed, 20 insertions(+), 18 deletions(-)
> >
> > diff --git a/gcc/match.pd b/gcc/match.pd
> > index 890f050cbad..c87a0795667 100644
> > --- a/gcc/match.pd
> > +++ b/gcc/match.pd
> > @@ -4950,24 +4950,6 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> >   )
> >  )
> >
> > -(simplify
> > - (cond @0 zero_one_valued_p@1 zero_one_valued_p@2)
> > - (switch
> > -  /* bool0 ? bool1 : 0 -> bool0 & bool1 */
> > -  (if (integer_zerop (@2))
> > -   (bit_and (convert @0) @1))
> > -  /* bool0 ? 0 : bool2 -> (bool0^1) & bool2 */
> > -  (if (integer_zerop (@1))
> > -   (bit_and (bit_xor (convert @0) { build_one_cst (type); } ) @2))
> > -  /* bool0 ? 1 : bool2 -> bool0 | bool2 */
> > -  (if (integer_onep (@1))
> > -   (bit_ior (convert @0) @2))
> > -  /* bool0 ? bool1 : 1 -> (bool0^1) | bool1 */
> > -  (if (integer_onep (@2))
> > -   (bit_ior (bit_xor (convert @0) @2) @1))
> > - )
> > -)
> > -
> >  /* Optimize
> >     # x_5 in range [cst1, cst2] where cst2 = cst1 + 1
> >     x_5 ? cstN ? cst4 : cst3
> > @@ -5298,6 +5280,26 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> >            && integer_nonzerop (fold_build2 (GE_EXPR, boolean_type_node, @3, @1)))
> >        (max @2 @4))))))
> >
> > +#if GIMPLE
> > +(simplify
> > + (cond @0 zero_one_valued_p@1 zero_one_valued_p@2)
> > + (switch
> > +  /* bool0 ? bool1 : 0 -> bool0 & bool1 */
> > +  (if (integer_zerop (@2))
> > +   (bit_and (convert @0) @1))
> > +  /* bool0 ? 0 : bool2 -> (bool0^1) & bool2 */
> > +  (if (integer_zerop (@1))
> > +   (bit_and (bit_xor (convert @0) { build_one_cst (type); } ) @2))
> > +  /* bool0 ? 1 : bool2 -> bool0 | bool2 */
> > +  (if (integer_onep (@1))
> > +   (bit_ior (convert @0) @2))
> > +  /* bool0 ? bool1 : 1 -> (bool0^1) | bool1 */
> > +  (if (integer_onep (@2))
> > +   (bit_ior (bit_xor (convert @0) @2) @1))
> > + )
> > +)
> > +#endif
> > +
> >  /* X != C1 ? -X : C2 simplifies to -X when -C1 == C2.  */
> >  (simplify
> >   (cond (ne @0 INTEGER_CST@1) (negate@3 @0) INTEGER_CST@2)
> > --
> > 2.31.1
> >

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/3] MATCH: Move `a ? one_zero : one_zero` matching after min/max matching
  2023-08-25 18:11   ` Andrew Pinski
@ 2023-08-25 18:18     ` Andrew Pinski
  0 siblings, 0 replies; 10+ messages in thread
From: Andrew Pinski @ 2023-08-25 18:18 UTC (permalink / raw)
  To: Richard Biener; +Cc: Andrew Pinski, gcc-patches

On Fri, Aug 25, 2023 at 11:11 AM Andrew Pinski <pinskia@gmail.com> wrote:
>
> On Thu, Aug 24, 2023 at 11:39 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:
> > >
> > > In PR 106677, I noticed that on the trunk we were producing:
> > > ```
> > >   _25 = SR.116_117 == 0;
> > >   _27 = (unsigned char) _25;
> > >   _32 = _27 | SR.116_117;
> > > ```
> > > From `SR.115_117 != 0 ? SR.115_117 : 1`
> > > Rather than:
> > > ```
> > >   _119 = MAX_EXPR <1, SR.115_117>;
> > > ```
> > > Or (rather)
> > > ```
> > >   _119 = SR.115_117 | 1;
> > > ```
> > > Due to the order of the patterns.
> >
> > Hmm, that means the former when present in source isn't optimized?
>
> That it is correct; they are not optimized at the gimple level down to
> 1. it is sometimes (but not on all targets) optimized at the RTL level
> though.

I forgot to mention that this is recorded as PR 110637 already.

Thanks,
Andrew

>
> >
> > > OK? Bootstrapped and tested on x86_64-linux-gnu with no
> > > regressions.
> >
> > OK, but please add a comment indicating the ordering requirement.
> >
> > Can you also add a testcase?
>
> Yes and yes. Will send out a new patch in a few minutes with the added
> comment and testcase.
>
> Thanks,
> Andrew
>
> >
> > Richard.
> >
> > > gcc/ChangeLog:
> > >
> > >         * match.pd (`a ? one_zero : one_zero`): Move
> > >         below detection of minmax.
> > > ---
> > >  gcc/match.pd | 38 ++++++++++++++++++++------------------
> > >  1 file changed, 20 insertions(+), 18 deletions(-)
> > >
> > > diff --git a/gcc/match.pd b/gcc/match.pd
> > > index 890f050cbad..c87a0795667 100644
> > > --- a/gcc/match.pd
> > > +++ b/gcc/match.pd
> > > @@ -4950,24 +4950,6 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> > >   )
> > >  )
> > >
> > > -(simplify
> > > - (cond @0 zero_one_valued_p@1 zero_one_valued_p@2)
> > > - (switch
> > > -  /* bool0 ? bool1 : 0 -> bool0 & bool1 */
> > > -  (if (integer_zerop (@2))
> > > -   (bit_and (convert @0) @1))
> > > -  /* bool0 ? 0 : bool2 -> (bool0^1) & bool2 */
> > > -  (if (integer_zerop (@1))
> > > -   (bit_and (bit_xor (convert @0) { build_one_cst (type); } ) @2))
> > > -  /* bool0 ? 1 : bool2 -> bool0 | bool2 */
> > > -  (if (integer_onep (@1))
> > > -   (bit_ior (convert @0) @2))
> > > -  /* bool0 ? bool1 : 1 -> (bool0^1) | bool1 */
> > > -  (if (integer_onep (@2))
> > > -   (bit_ior (bit_xor (convert @0) @2) @1))
> > > - )
> > > -)
> > > -
> > >  /* Optimize
> > >     # x_5 in range [cst1, cst2] where cst2 = cst1 + 1
> > >     x_5 ? cstN ? cst4 : cst3
> > > @@ -5298,6 +5280,26 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> > >            && integer_nonzerop (fold_build2 (GE_EXPR, boolean_type_node, @3, @1)))
> > >        (max @2 @4))))))
> > >
> > > +#if GIMPLE
> > > +(simplify
> > > + (cond @0 zero_one_valued_p@1 zero_one_valued_p@2)
> > > + (switch
> > > +  /* bool0 ? bool1 : 0 -> bool0 & bool1 */
> > > +  (if (integer_zerop (@2))
> > > +   (bit_and (convert @0) @1))
> > > +  /* bool0 ? 0 : bool2 -> (bool0^1) & bool2 */
> > > +  (if (integer_zerop (@1))
> > > +   (bit_and (bit_xor (convert @0) { build_one_cst (type); } ) @2))
> > > +  /* bool0 ? 1 : bool2 -> bool0 | bool2 */
> > > +  (if (integer_onep (@1))
> > > +   (bit_ior (convert @0) @2))
> > > +  /* bool0 ? bool1 : 1 -> (bool0^1) | bool1 */
> > > +  (if (integer_onep (@2))
> > > +   (bit_ior (bit_xor (convert @0) @2) @1))
> > > + )
> > > +)
> > > +#endif
> > > +
> > >  /* X != C1 ? -X : C2 simplifies to -X when -C1 == C2.  */
> > >  (simplify
> > >   (cond (ne @0 INTEGER_CST@1) (negate@3 @0) INTEGER_CST@2)
> > > --
> > > 2.31.1
> > >

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 3/3] PHIOPT: Allow BIT_AND and BIT_IOR in early phiopt
  2023-08-25  6:46   ` Richard Biener
@ 2023-08-25 23:11     ` Andrew Pinski
  0 siblings, 0 replies; 10+ messages in thread
From: Andrew Pinski @ 2023-08-25 23:11 UTC (permalink / raw)
  To: Richard Biener; +Cc: Andrew Pinski, gcc-patches

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2023-08-25 23:12 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-24 19:14 [PATCH 1/3] MATCH: Move `a ? one_zero : one_zero` matching after min/max matching Andrew Pinski
2023-08-24 19:14 ` [PATCH 2/3] MATCH: `a | C -> C` when we know that `a & ~C == 0` Andrew Pinski
2023-08-25  6:36   ` Richard Biener
2023-08-25  7:40     ` Andrew Pinski
2023-08-24 19:14 ` [PATCH 3/3] PHIOPT: Allow BIT_AND and BIT_IOR in early phiopt Andrew Pinski
2023-08-25  6:46   ` Richard Biener
2023-08-25 23:11     ` Andrew Pinski
2023-08-25  6:38 ` [PATCH 1/3] MATCH: Move `a ? one_zero : one_zero` matching after min/max matching Richard Biener
2023-08-25 18:11   ` Andrew Pinski
2023-08-25 18:18     ` 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).