MATCH: Improve zero_one_valued_p for cases without range information

Message ID 20230915010855.1582726-1-apinski@marvell.com
State Unresolved
Headers
Series MATCH: Improve zero_one_valued_p for cases without range information |

Checks

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

Commit Message

Andrew Pinski Sept. 15, 2023, 1:08 a.m. UTC
  I noticed we sometimes lose range information in forwprop due to a few
match and simplify patterns optimizing away casts. So the easier way
to these cases is to add a match for zero_one_valued_p wich mathes
a cast from another zero_one_valued_p.
This also adds the case of `x & zero_one_valued_p` as being zero_one_valued_p
which allows catching more cases too.

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

gcc/ChangeLog:

	* match.pd (zero_one_valued_p): Match a cast from a zero_one_valued_p.
	Also match `a & zero_one_valued_p` too.

gcc/testsuite/ChangeLog:

	* gcc.dg/tree-ssa/bool-13.c: Update testcase as we now do
	the MIN/MAX during forwprop1.
---
 gcc/match.pd                            | 10 ++++++++++
 gcc/testsuite/gcc.dg/tree-ssa/bool-13.c | 15 +++++----------
 2 files changed, 15 insertions(+), 10 deletions(-)
  

Comments

Richard Biener Sept. 15, 2023, 6:27 a.m. UTC | #1
On Fri, Sep 15, 2023 at 3:09 AM Andrew Pinski via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> I noticed we sometimes lose range information in forwprop due to a few
> match and simplify patterns optimizing away casts. So the easier way
> to these cases is to add a match for zero_one_valued_p wich mathes
> a cast from another zero_one_valued_p.
> This also adds the case of `x & zero_one_valued_p` as being zero_one_valued_p
> which allows catching more cases too.
>
> OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions.

OK.

I wonder if it would make a difference if we'd enable ranger unconditionally
in forwprop (maybe with -O2+), currently it gets enabled sometimes only.

Richard.

> gcc/ChangeLog:
>
>         * match.pd (zero_one_valued_p): Match a cast from a zero_one_valued_p.
>         Also match `a & zero_one_valued_p` too.
>
> gcc/testsuite/ChangeLog:
>
>         * gcc.dg/tree-ssa/bool-13.c: Update testcase as we now do
>         the MIN/MAX during forwprop1.
> ---
>  gcc/match.pd                            | 10 ++++++++++
>  gcc/testsuite/gcc.dg/tree-ssa/bool-13.c | 15 +++++----------
>  2 files changed, 15 insertions(+), 10 deletions(-)
>
> diff --git a/gcc/match.pd b/gcc/match.pd
> index 97db0eb5f25..39c9c81966a 100644
> --- a/gcc/match.pd
> +++ b/gcc/match.pd
> @@ -2181,6 +2181,16 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>        && (TYPE_UNSIGNED (type)
>           || TYPE_PRECISION (type) > 1))))
>
> +/* (a&1) is always [0,1] too. This is useful again when
> +   the range is not known. */
> +(match zero_one_valued_p
> + (bit_and:c@0 @1 zero_one_valued_p))
> +
> +/* A conversion from an zero_one_valued_p is still a [0,1].
> +   This is useful when the range of a variable is not known */
> +(match zero_one_valued_p
> + (convert@0 zero_one_valued_p))
> +
>  /* Transform { 0 or 1 } * { 0 or 1 } into { 0 or 1 } & { 0 or 1 }.  */
>  (simplify
>   (mult zero_one_valued_p@0 zero_one_valued_p@1)
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/bool-13.c b/gcc/testsuite/gcc.dg/tree-ssa/bool-13.c
> index 438f15a484a..de8c99a7727 100644
> --- a/gcc/testsuite/gcc.dg/tree-ssa/bool-13.c
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/bool-13.c
> @@ -1,5 +1,5 @@
>  /* { dg-do compile } */
> -/* { dg-options "-O1 -fdump-tree-optimized -fdump-tree-original -fdump-tree-phiopt1 -fdump-tree-forwprop2" } */
> +/* { dg-options "-O1 -fdump-tree-optimized -fdump-tree-original -fdump-tree-forwprop1 -fdump-tree-forwprop2" } */
>  #define bool _Bool
>  int maxbool(bool ab, bool bb)
>  {
> @@ -22,15 +22,10 @@ int minbool(bool ab, bool bb)
>  /* { dg-final { scan-tree-dump-times "MIN_EXPR" 1 "original" } } */
>  /* { dg-final { scan-tree-dump-times "if " 0 "original" } } */
>
> -/* PHI-OPT1 should have kept it as min/max. */
> -/* { dg-final { scan-tree-dump-times "MAX_EXPR" 1 "phiopt1" } } */
> -/* { dg-final { scan-tree-dump-times "MIN_EXPR" 1 "phiopt1" } } */
> -/* { dg-final { scan-tree-dump-times "if " 0 "phiopt1" } } */
> -
> -/* Forwprop2 (after ccp) will convert it into &\| */
> -/* { dg-final { scan-tree-dump-times "MAX_EXPR" 0 "forwprop2" } } */
> -/* { dg-final { scan-tree-dump-times "MIN_EXPR" 0 "forwprop2" } } */
> -/* { dg-final { scan-tree-dump-times "if " 0 "forwprop2" } } */
> +/* Forwprop1 will convert it into &\| as we can detect that the arguments are one_zero. */
> +/* { dg-final { scan-tree-dump-times "MAX_EXPR" 0 "forwprop1" } } */
> +/* { dg-final { scan-tree-dump-times "MIN_EXPR" 0 "forwprop1" } } */
> +/* { dg-final { scan-tree-dump-times "if " 0 "forwprop1" } } */
>
>  /* By optimize there should be no min/max nor if  */
>  /* { dg-final { scan-tree-dump-times "MAX_EXPR" 0 "optimized" } } */
> --
> 2.31.1
>
  
Andrew Pinski Sept. 15, 2023, 2:58 p.m. UTC | #2
On Thu, Sep 14, 2023 at 11:28 PM Richard Biener via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> On Fri, Sep 15, 2023 at 3:09 AM Andrew Pinski via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
> >
> > I noticed we sometimes lose range information in forwprop due to a few
> > match and simplify patterns optimizing away casts. So the easier way
> > to these cases is to add a match for zero_one_valued_p wich mathes
> > a cast from another zero_one_valued_p.
> > This also adds the case of `x & zero_one_valued_p` as being zero_one_valued_p
> > which allows catching more cases too.
> >
> > OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions.
>
> OK.
>
> I wonder if it would make a difference if we'd enable ranger unconditionally
> in forwprop (maybe with -O2+), currently it gets enabled sometimes only.

I was thinking about that though currently zero_one_valued_p only uses
the global ranger info via tree_nonzero_bits but I have patches to use
the local one too.

Thanks,
Andrew

>
> Richard.
>
> > gcc/ChangeLog:
> >
> >         * match.pd (zero_one_valued_p): Match a cast from a zero_one_valued_p.
> >         Also match `a & zero_one_valued_p` too.
> >
> > gcc/testsuite/ChangeLog:
> >
> >         * gcc.dg/tree-ssa/bool-13.c: Update testcase as we now do
> >         the MIN/MAX during forwprop1.
> > ---
> >  gcc/match.pd                            | 10 ++++++++++
> >  gcc/testsuite/gcc.dg/tree-ssa/bool-13.c | 15 +++++----------
> >  2 files changed, 15 insertions(+), 10 deletions(-)
> >
> > diff --git a/gcc/match.pd b/gcc/match.pd
> > index 97db0eb5f25..39c9c81966a 100644
> > --- a/gcc/match.pd
> > +++ b/gcc/match.pd
> > @@ -2181,6 +2181,16 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> >        && (TYPE_UNSIGNED (type)
> >           || TYPE_PRECISION (type) > 1))))
> >
> > +/* (a&1) is always [0,1] too. This is useful again when
> > +   the range is not known. */
> > +(match zero_one_valued_p
> > + (bit_and:c@0 @1 zero_one_valued_p))
> > +
> > +/* A conversion from an zero_one_valued_p is still a [0,1].
> > +   This is useful when the range of a variable is not known */
> > +(match zero_one_valued_p
> > + (convert@0 zero_one_valued_p))
> > +
> >  /* Transform { 0 or 1 } * { 0 or 1 } into { 0 or 1 } & { 0 or 1 }.  */
> >  (simplify
> >   (mult zero_one_valued_p@0 zero_one_valued_p@1)
> > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/bool-13.c b/gcc/testsuite/gcc.dg/tree-ssa/bool-13.c
> > index 438f15a484a..de8c99a7727 100644
> > --- a/gcc/testsuite/gcc.dg/tree-ssa/bool-13.c
> > +++ b/gcc/testsuite/gcc.dg/tree-ssa/bool-13.c
> > @@ -1,5 +1,5 @@
> >  /* { dg-do compile } */
> > -/* { dg-options "-O1 -fdump-tree-optimized -fdump-tree-original -fdump-tree-phiopt1 -fdump-tree-forwprop2" } */
> > +/* { dg-options "-O1 -fdump-tree-optimized -fdump-tree-original -fdump-tree-forwprop1 -fdump-tree-forwprop2" } */
> >  #define bool _Bool
> >  int maxbool(bool ab, bool bb)
> >  {
> > @@ -22,15 +22,10 @@ int minbool(bool ab, bool bb)
> >  /* { dg-final { scan-tree-dump-times "MIN_EXPR" 1 "original" } } */
> >  /* { dg-final { scan-tree-dump-times "if " 0 "original" } } */
> >
> > -/* PHI-OPT1 should have kept it as min/max. */
> > -/* { dg-final { scan-tree-dump-times "MAX_EXPR" 1 "phiopt1" } } */
> > -/* { dg-final { scan-tree-dump-times "MIN_EXPR" 1 "phiopt1" } } */
> > -/* { dg-final { scan-tree-dump-times "if " 0 "phiopt1" } } */
> > -
> > -/* Forwprop2 (after ccp) will convert it into &\| */
> > -/* { dg-final { scan-tree-dump-times "MAX_EXPR" 0 "forwprop2" } } */
> > -/* { dg-final { scan-tree-dump-times "MIN_EXPR" 0 "forwprop2" } } */
> > -/* { dg-final { scan-tree-dump-times "if " 0 "forwprop2" } } */
> > +/* Forwprop1 will convert it into &\| as we can detect that the arguments are one_zero. */
> > +/* { dg-final { scan-tree-dump-times "MAX_EXPR" 0 "forwprop1" } } */
> > +/* { dg-final { scan-tree-dump-times "MIN_EXPR" 0 "forwprop1" } } */
> > +/* { dg-final { scan-tree-dump-times "if " 0 "forwprop1" } } */
> >
> >  /* By optimize there should be no min/max nor if  */
> >  /* { dg-final { scan-tree-dump-times "MAX_EXPR" 0 "optimized" } } */
> > --
> > 2.31.1
> >
  

Patch

diff --git a/gcc/match.pd b/gcc/match.pd
index 97db0eb5f25..39c9c81966a 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -2181,6 +2181,16 @@  DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
       && (TYPE_UNSIGNED (type)
 	  || TYPE_PRECISION (type) > 1))))
 
+/* (a&1) is always [0,1] too. This is useful again when
+   the range is not known. */
+(match zero_one_valued_p
+ (bit_and:c@0 @1 zero_one_valued_p))
+
+/* A conversion from an zero_one_valued_p is still a [0,1].
+   This is useful when the range of a variable is not known */
+(match zero_one_valued_p
+ (convert@0 zero_one_valued_p))
+
 /* Transform { 0 or 1 } * { 0 or 1 } into { 0 or 1 } & { 0 or 1 }.  */
 (simplify
  (mult zero_one_valued_p@0 zero_one_valued_p@1)
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/bool-13.c b/gcc/testsuite/gcc.dg/tree-ssa/bool-13.c
index 438f15a484a..de8c99a7727 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/bool-13.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/bool-13.c
@@ -1,5 +1,5 @@ 
 /* { dg-do compile } */
-/* { dg-options "-O1 -fdump-tree-optimized -fdump-tree-original -fdump-tree-phiopt1 -fdump-tree-forwprop2" } */
+/* { dg-options "-O1 -fdump-tree-optimized -fdump-tree-original -fdump-tree-forwprop1 -fdump-tree-forwprop2" } */
 #define bool _Bool
 int maxbool(bool ab, bool bb)
 {
@@ -22,15 +22,10 @@  int minbool(bool ab, bool bb)
 /* { dg-final { scan-tree-dump-times "MIN_EXPR" 1 "original" } } */
 /* { dg-final { scan-tree-dump-times "if " 0 "original" } } */
 
-/* PHI-OPT1 should have kept it as min/max. */
-/* { dg-final { scan-tree-dump-times "MAX_EXPR" 1 "phiopt1" } } */
-/* { dg-final { scan-tree-dump-times "MIN_EXPR" 1 "phiopt1" } } */
-/* { dg-final { scan-tree-dump-times "if " 0 "phiopt1" } } */
-
-/* Forwprop2 (after ccp) will convert it into &\| */
-/* { dg-final { scan-tree-dump-times "MAX_EXPR" 0 "forwprop2" } } */
-/* { dg-final { scan-tree-dump-times "MIN_EXPR" 0 "forwprop2" } } */
-/* { dg-final { scan-tree-dump-times "if " 0 "forwprop2" } } */
+/* Forwprop1 will convert it into &\| as we can detect that the arguments are one_zero. */
+/* { dg-final { scan-tree-dump-times "MAX_EXPR" 0 "forwprop1" } } */
+/* { dg-final { scan-tree-dump-times "MIN_EXPR" 0 "forwprop1" } } */
+/* { dg-final { scan-tree-dump-times "if " 0 "forwprop1" } } */
 
 /* By optimize there should be no min/max nor if  */
 /* { dg-final { scan-tree-dump-times "MAX_EXPR" 0 "optimized" } } */