[1/3] MATCH: Move `a ? one_zero : one_zero` matching after min/max matching

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

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
>
  

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

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