[v2,2/2] VN: Handle `(A CMP B) !=/== 0` for predicates [PR117414]

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 shows up more due to the new handling of
`(A | B) ==/!= 0` in insert_predicates_for_cond
as now we can notice these comparisons which were not seen before.

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.

Changes since v1:
* v2:  Use vn_valueize.

Bootstrapped and tested on x86_64-linux-gnu.

	PR tree-optimization/117414

gcc/ChangeLog:

	* tree-ssa-sccvn.cc (insert_predicates_for_cond): Handle `(A CMP B) !=/== 0`.

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

On Thu, Nov 7, 2024 at 12:43 AM 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 shows up more due to the new handling of
> `(A | B) ==/!= 0` in insert_predicates_for_cond
> as now we can notice these comparisons which were not seen before.
>
> 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.
>
> Changes since v1:
> * v2:  Use vn_valueize.
>
> Bootstrapped and tested on x86_64-linux-gnu.
>
>         PR tree-optimization/117414
>
> gcc/ChangeLog:
>
>         * tree-ssa-sccvn.cc (insert_predicates_for_cond): Handle `(A CMP B) !=/== 0`.
>
> 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 c6dddd0ba6d..67ed2cd8ffe 100644
> --- a/gcc/tree-ssa-sccvn.cc
> +++ b/gcc/tree-ssa-sccvn.cc
> @@ -7948,6 +7948,20 @@ insert_predicates_for_cond (tree_code code, tree lhs, tree rhs,
>        && (code == NE_EXPR || code == EQ_EXPR))
>      {
>        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 = vn_valueize (gimple_assign_rhs1 (def_stmt));
> +           tree nrhs = vn_valueize (gimple_assign_rhs2 (def_stmt));
> +           edge nt = true_e;
> +           edge nf = false_e;
> +           if (code == EQ_EXPR)
> +             std::swap (nt, nf);

I'll note there's canonicalization for tree_swap_operands as well,
_1 < _2 vs. _2 > _1 - but I think this can be fixed as followup?

OK.

Thanks,
Richard.

> +           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 Thu, Nov 7, 2024 at 12:50 AM Richard Biener
<richard.guenther@gmail.com> wrote:
>
> On Thu, Nov 7, 2024 at 12:43 AM 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 shows up more due to the new handling of
> > `(A | B) ==/!= 0` in insert_predicates_for_cond
> > as now we can notice these comparisons which were not seen before.
> >
> > 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.
> >
> > Changes since v1:
> > * v2:  Use vn_valueize.
> >
> > Bootstrapped and tested on x86_64-linux-gnu.
> >
> >         PR tree-optimization/117414
> >
> > gcc/ChangeLog:
> >
> >         * tree-ssa-sccvn.cc (insert_predicates_for_cond): Handle `(A CMP B) !=/== 0`.
> >
> > 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 c6dddd0ba6d..67ed2cd8ffe 100644
> > --- a/gcc/tree-ssa-sccvn.cc
> > +++ b/gcc/tree-ssa-sccvn.cc
> > @@ -7948,6 +7948,20 @@ insert_predicates_for_cond (tree_code code, tree lhs, tree rhs,
> >        && (code == NE_EXPR || code == EQ_EXPR))
> >      {
> >        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 = vn_valueize (gimple_assign_rhs1 (def_stmt));
> > +           tree nrhs = vn_valueize (gimple_assign_rhs2 (def_stmt));
> > +           edge nt = true_e;
> > +           edge nf = false_e;
> > +           if (code == EQ_EXPR)
> > +             std::swap (nt, nf);
>
> I'll note there's canonicalization for tree_swap_operands as well,
> _1 < _2 vs. _2 > _1 - but I think this can be fixed as followup?

Yes I will handle that later today in a followup patch because it will
touch other areas of sccvn including places which does the lookup.

Thanks,
Andrew

>
> OK.
>
> Thanks,
> Richard.
>
> > +           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
> >