[3/4] VN: Handle `(A CMP B) !=/== 0` for predicates [PR117414]
Commit Message
After the last patch, we also want to record `(A CMP B) != 0`
as `(A CMP B)` and `(A CMP B) == 0` as `(A CMP B)` with the
true/false edges swapped.
This is enough to fix the original issue in `gcc.dg/tree-ssa/pr111456-1.c`
and make sure we don't regress it when enhancing ifcombine.
This adds that predicate and allows us to optimize f
in fre-predicated-3.c.
Bootstrapped and tested on x86_64-linux-gnu.
PR tree-optimization/117414
gcc/ChangeLog:
* tree-ssa-sccvn.cc (insert_predicates_for_cond):
gcc/testsuite/ChangeLog:
* gcc.dg/tree-ssa/fre-predicated-3.c: New test.
Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com>
---
.../gcc.dg/tree-ssa/fre-predicated-3.c | 46 +++++++++++++++++++
gcc/tree-ssa-sccvn.cc | 14 ++++++
2 files changed, 60 insertions(+)
create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/fre-predicated-3.c
Comments
On Sat, Nov 2, 2024 at 4:10 PM Andrew Pinski <quic_apinski@quicinc.com> wrote:
>
> After the last patch, we also want to record `(A CMP B) != 0`
> as `(A CMP B)` and `(A CMP B) == 0` as `(A CMP B)` with the
> true/false edges swapped.
>
> This is enough to fix the original issue in `gcc.dg/tree-ssa/pr111456-1.c`
> and make sure we don't regress it when enhancing ifcombine.
>
> This adds that predicate and allows us to optimize f
> in fre-predicated-3.c.
>
> Bootstrapped and tested on x86_64-linux-gnu.
>
> PR tree-optimization/117414
>
> gcc/ChangeLog:
>
> * tree-ssa-sccvn.cc (insert_predicates_for_cond):
>
> gcc/testsuite/ChangeLog:
>
> * gcc.dg/tree-ssa/fre-predicated-3.c: New test.
>
> Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com>
> ---
> .../gcc.dg/tree-ssa/fre-predicated-3.c | 46 +++++++++++++++++++
> gcc/tree-ssa-sccvn.cc | 14 ++++++
> 2 files changed, 60 insertions(+)
> create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/fre-predicated-3.c
>
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/fre-predicated-3.c b/gcc/testsuite/gcc.dg/tree-ssa/fre-predicated-3.c
> new file mode 100644
> index 00000000000..4a89372fd70
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/fre-predicated-3.c
> @@ -0,0 +1,46 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fdump-tree-optimized" } */
> +
> +/* PR tree-optimization/117414 */
> +
> +/* Fre1 should figure out that `*aaa != 0`
> + For f0, f1, and f2. */
> +
> +void foo();
> +int f(int *aaa, int j, int t)
> +{
> + int b = *aaa;
> + int c = b == 0;
> + int d = t != 1;
> + if (c | d)
> + return 0;
> +
> + for(int i = 0; i < j; i++)
> + {
> + if (*aaa)
> + ;
> + else
> + foo();
> + }
> + return 0;
> +}
> +
> +int f1(int *aaa, int j, int t)
> +{
> + int b = *aaa;
> + if (b == 0)
> + return 0;
> + if (t != 1)
> + return 0;
> + for(int i = 0; i < j; i++)
> + {
> + if (*aaa)
> + ;
> + else
> + foo();
> + }
> + return 0;
> +}
> +
> +/* { dg-final { scan-tree-dump-not "foo " "optimized" } } */
> +/* { dg-final { scan-tree-dump "return 0;" "optimized" } } */
> diff --git a/gcc/tree-ssa-sccvn.cc b/gcc/tree-ssa-sccvn.cc
> index b3e6cd09007..190b7d24f1a 100644
> --- a/gcc/tree-ssa-sccvn.cc
> +++ b/gcc/tree-ssa-sccvn.cc
> @@ -7937,6 +7937,20 @@ insert_predicates_for_cond (tree_code code, tree lhs, tree rhs,
> && TREE_CODE (lhs) == SSA_NAME)
> {
> gimple *def_stmt = SSA_NAME_DEF_STMT (lhs);
> + /* (A CMP B) != 0 is the same as (A CMP B).
> + (A CMP B) == 0 is just (A CMP B) with the edges swapped. */
> + if (is_gimple_assign (def_stmt)
> + && TREE_CODE_CLASS (gimple_assign_rhs_code (def_stmt)) == tcc_comparison)
> + {
> + tree_code nc = gimple_assign_rhs_code (def_stmt);
> + tree nlhs = gimple_assign_rhs1 (def_stmt);
> + tree nrhs = gimple_assign_rhs2 (def_stmt);
Same as with the other patch.
We do forward/fold compares quite aggressively, so I wonder why (A CMP
B) != 0 wasn't
simplified earlier?
Otherwise OK.
Thanks,
Richard.
> + edge nt = true_e;
> + edge nf = false_e;
> + if (code == EQ_EXPR)
> + std::swap (nt, nf);
> + insert_predicates_for_cond (nc, nlhs, nrhs, nt, nf);
> + }
> /* (a | b) == 0 ->
> on true edge assert: a == 0 & b == 0. */
> /* (a | b) != 0 ->
> --
> 2.43.0
>
On Wed, Nov 6, 2024 at 4:56 AM Richard Biener
<richard.guenther@gmail.com> wrote:
>
> On Sat, Nov 2, 2024 at 4:10 PM Andrew Pinski <quic_apinski@quicinc.com> wrote:
> >
> > After the last patch, we also want to record `(A CMP B) != 0`
> > as `(A CMP B)` and `(A CMP B) == 0` as `(A CMP B)` with the
> > true/false edges swapped.
> >
> > This is enough to fix the original issue in `gcc.dg/tree-ssa/pr111456-1.c`
> > and make sure we don't regress it when enhancing ifcombine.
> >
> > This adds that predicate and allows us to optimize f
> > in fre-predicated-3.c.
> >
> > Bootstrapped and tested on x86_64-linux-gnu.
> >
> > PR tree-optimization/117414
> >
> > gcc/ChangeLog:
> >
> > * tree-ssa-sccvn.cc (insert_predicates_for_cond):
> >
> > gcc/testsuite/ChangeLog:
> >
> > * gcc.dg/tree-ssa/fre-predicated-3.c: New test.
> >
> > Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com>
> > ---
> > .../gcc.dg/tree-ssa/fre-predicated-3.c | 46 +++++++++++++++++++
> > gcc/tree-ssa-sccvn.cc | 14 ++++++
> > 2 files changed, 60 insertions(+)
> > create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/fre-predicated-3.c
> >
> > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/fre-predicated-3.c b/gcc/testsuite/gcc.dg/tree-ssa/fre-predicated-3.c
> > new file mode 100644
> > index 00000000000..4a89372fd70
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.dg/tree-ssa/fre-predicated-3.c
> > @@ -0,0 +1,46 @@
> > +/* { dg-do compile } */
> > +/* { dg-options "-O2 -fdump-tree-optimized" } */
> > +
> > +/* PR tree-optimization/117414 */
> > +
> > +/* Fre1 should figure out that `*aaa != 0`
> > + For f0, f1, and f2. */
> > +
> > +void foo();
> > +int f(int *aaa, int j, int t)
> > +{
> > + int b = *aaa;
> > + int c = b == 0;
> > + int d = t != 1;
> > + if (c | d)
> > + return 0;
> > +
> > + for(int i = 0; i < j; i++)
> > + {
> > + if (*aaa)
> > + ;
> > + else
> > + foo();
> > + }
> > + return 0;
> > +}
> > +
> > +int f1(int *aaa, int j, int t)
> > +{
> > + int b = *aaa;
> > + if (b == 0)
> > + return 0;
> > + if (t != 1)
> > + return 0;
> > + for(int i = 0; i < j; i++)
> > + {
> > + if (*aaa)
> > + ;
> > + else
> > + foo();
> > + }
> > + return 0;
> > +}
> > +
> > +/* { dg-final { scan-tree-dump-not "foo " "optimized" } } */
> > +/* { dg-final { scan-tree-dump "return 0;" "optimized" } } */
> > diff --git a/gcc/tree-ssa-sccvn.cc b/gcc/tree-ssa-sccvn.cc
> > index b3e6cd09007..190b7d24f1a 100644
> > --- a/gcc/tree-ssa-sccvn.cc
> > +++ b/gcc/tree-ssa-sccvn.cc
> > @@ -7937,6 +7937,20 @@ insert_predicates_for_cond (tree_code code, tree lhs, tree rhs,
> > && TREE_CODE (lhs) == SSA_NAME)
> > {
> > gimple *def_stmt = SSA_NAME_DEF_STMT (lhs);
> > + /* (A CMP B) != 0 is the same as (A CMP B).
> > + (A CMP B) == 0 is just (A CMP B) with the edges swapped. */
> > + if (is_gimple_assign (def_stmt)
> > + && TREE_CODE_CLASS (gimple_assign_rhs_code (def_stmt)) == tcc_comparison)
> > + {
> > + tree_code nc = gimple_assign_rhs_code (def_stmt);
> > + tree nlhs = gimple_assign_rhs1 (def_stmt);
> > + tree nrhs = gimple_assign_rhs2 (def_stmt);
>
> Same as with the other patch.
I will make the change to use vn_valueize and test it.
>
> We do forward/fold compares quite aggressively, so I wonder why (A CMP
> B) != 0 wasn't
> simplified earlier?
Because in this case, we are handling this recursively. We originally
had `((A CMP B) | (C CMP D)) != 0` which
then insert_predicates_for_cond gets called (again) with `bool0 != 0`
, we want to see that bool0 was defined as `A CMP B`.
The testcase is testing the above recursively call sequence rather
than the unfolled sequence you are thinking of.
Thanks,
Andrew Pinski
>
> Otherwise OK.
>
> Thanks,
> Richard.
>
> > + edge nt = true_e;
> > + edge nf = false_e;
> > + if (code == EQ_EXPR)
> > + std::swap (nt, nf);
> > + insert_predicates_for_cond (nc, nlhs, nrhs, nt, nf);
> > + }
> > /* (a | b) == 0 ->
> > on true edge assert: a == 0 & b == 0. */
> > /* (a | b) != 0 ->
> > --
> > 2.43.0
> >
new file mode 100644
@@ -0,0 +1,46 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-optimized" } */
+
+/* PR tree-optimization/117414 */
+
+/* Fre1 should figure out that `*aaa != 0`
+ For f0, f1, and f2. */
+
+void foo();
+int f(int *aaa, int j, int t)
+{
+ int b = *aaa;
+ int c = b == 0;
+ int d = t != 1;
+ if (c | d)
+ return 0;
+
+ for(int i = 0; i < j; i++)
+ {
+ if (*aaa)
+ ;
+ else
+ foo();
+ }
+ return 0;
+}
+
+int f1(int *aaa, int j, int t)
+{
+ int b = *aaa;
+ if (b == 0)
+ return 0;
+ if (t != 1)
+ return 0;
+ for(int i = 0; i < j; i++)
+ {
+ if (*aaa)
+ ;
+ else
+ foo();
+ }
+ return 0;
+}
+
+/* { dg-final { scan-tree-dump-not "foo " "optimized" } } */
+/* { dg-final { scan-tree-dump "return 0;" "optimized" } } */
@@ -7937,6 +7937,20 @@ insert_predicates_for_cond (tree_code code, tree lhs, tree rhs,
&& TREE_CODE (lhs) == SSA_NAME)
{
gimple *def_stmt = SSA_NAME_DEF_STMT (lhs);
+ /* (A CMP B) != 0 is the same as (A CMP B).
+ (A CMP B) == 0 is just (A CMP B) with the edges swapped. */
+ if (is_gimple_assign (def_stmt)
+ && TREE_CODE_CLASS (gimple_assign_rhs_code (def_stmt)) == tcc_comparison)
+ {
+ tree_code nc = gimple_assign_rhs_code (def_stmt);
+ tree nlhs = gimple_assign_rhs1 (def_stmt);
+ tree nrhs = gimple_assign_rhs2 (def_stmt);
+ edge nt = true_e;
+ edge nf = false_e;
+ if (code == EQ_EXPR)
+ std::swap (nt, nf);
+ insert_predicates_for_cond (nc, nlhs, nrhs, nt, nf);
+ }
/* (a | b) == 0 ->
on true edge assert: a == 0 & b == 0. */
/* (a | b) != 0 ->