On Fri, May 9, 2025 at 4:34 AM Andrew Pinski <pins...@gmail.com> wrote:
>
> On Fri, May 9, 2025 at 1:21 AM Richard Biener
> <richard.guent...@gmail.com> wrote:
> >
> > On Fri, May 9, 2025 at 4:51 AM Andrew Pinski <quic_apin...@quicinc.com>
> > wrote:
> > >
> > > These patterns are not needed any more. There were already
> > > 2 patterns which did `(ne bool_var 0)` into `bool_var` and
> > > `(eq bool_var 1)` into `bool_var`. Just they were after the
> > > pattern that did `(cmp (cond @0 @1 @2) @3)` simplification but
> > > that pattern is now after the ones.
> > > Also these patterns will cause in some cases a new statement to
> > > be created for the comparison. In the case of floating point comparison
> > > wiht non-call exceptions (and trapping math), can cause a new statement
> > > every time fold_stmt is called.
> >
> > Hmm, but do we still fold
> >
> > _1 = _2 < 1;
> > if (_1 != 0)
> >
> > to
> >
> > if (_2 < 1)
> >
> > or does that now again rely on forwprops explicit forwarding into
> > gcond? I wanted
> > to get rid of the latter eventually.
>
> Oh. Yes this does rely on forwprop explicitly now.
On the subject of removing that part of forwprop, right now if you
remove it you will end up with an infinite loop in forwprop.
This is because right now the return value of
forward_propagate_into_gimple_cond overwrites the changed variable.
And if you remove the forward_propagate_into_gimple_cond call,
fold_stmt will return true all the time for the non-call exceptions
throwing comparison case.
This patch (and the few other gimple-fold.cc patches which all have
been approved and pushed) have been moving towards fixing that
infinite loop even.
The one piece of forward_propagate_into_gimple_cond that might be easy
to move to fold_stmt is the `Canonicalize _Bool == 0 and _Bool != 1 to
_Bool != 0`.
And then after my new patch for the non-call exception issue (that
does not remove the above pattern), I am going to see if I can remove
forward_propagate_into_gimple_cond too.
Thanks,
Andrew Pinski
>
> >
> > I agree that the trapping math thing is bad - I wonder if we can catch that
> > more
> > intelligently (not sure how without following SSA use-def of gconds on bools
> > and see whether they can trap and then not simplifying)
>
> I think I know the way to fix the trapping issue without fully
> removing this. I am going to give it a go later today.
> Since trapping only depends on the code and the type it should be easy
> to add an extra condition here and the latter patterns catch the
> trapping case of removing `bool!=0` already.
>
> Thanks,
> Andrew Pinski
>
> >
> > > gcc.dg/tree-ssa/vrp24.c needed to be adjusted to before
> > > r13-322-g7f04b0d786e13f.
> > > gcc.dg/analyzer/null-deref-pr102671-2.c needs an increased
> > > analyzer-max-svalue-depth
> > > not to get an extra warning.
> > >
> > > gcc/ChangeLog:
> > >
> > > * match.pd (`(ne (cmp) 0)`, `(eq (cmp) 1)`): Remove.
> > >
> > > gcc/testsuite/ChangeLog:
> > >
> > > * gcc.dg/tree-ssa/vrp24.c: Adjust.
> > > * gcc.dg/analyzer/null-deref-pr102671-2.c: Increase
> > > analyzer-max-svalue-depth.
> > >
> > > Signed-off-by: Andrew Pinski <quic_apin...@quicinc.com>
> > > ---
> > > gcc/match.pd | 8 --------
> > > gcc/testsuite/gcc.dg/analyzer/null-deref-pr102671-2.c | 2 +-
> > > gcc/testsuite/gcc.dg/tree-ssa/vrp24.c | 2 +-
> > > 3 files changed, 2 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/gcc/match.pd b/gcc/match.pd
> > > index ab496d923cc..418efc4230a 100644
> > > --- a/gcc/match.pd
> > > +++ b/gcc/match.pd
> > > @@ -6898,14 +6898,6 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> > > (if (ic == ncmp)
> > > (ncmp @0 @1)))))
> > > /* The following bits are handled by
> > > fold_binary_op_with_conditional_arg. */
> > > - (simplify
> > > - (ne (cmp@2 @0 @1) integer_zerop)
> > > - (if (types_match (type, TREE_TYPE (@2)))
> > > - (cmp @0 @1)))
> > > - (simplify
> > > - (eq (cmp@2 @0 @1) integer_truep)
> > > - (if (types_match (type, TREE_TYPE (@2)))
> > > - (cmp @0 @1)))
> > > (simplify
> > > (ne (cmp@2 @0 @1) integer_truep)
> > > (if (types_match (type, TREE_TYPE (@2)))
> > > diff --git a/gcc/testsuite/gcc.dg/analyzer/null-deref-pr102671-2.c
> > > b/gcc/testsuite/gcc.dg/analyzer/null-deref-pr102671-2.c
> > > index 298e4839b98..bc141d5c028 100644
> > > --- a/gcc/testsuite/gcc.dg/analyzer/null-deref-pr102671-2.c
> > > +++ b/gcc/testsuite/gcc.dg/analyzer/null-deref-pr102671-2.c
> > > @@ -1,5 +1,5 @@
> > > /* { dg-require-effective-target ptr_eq_long } */
> > > -/* { dg-additional-options "-O2 -Wno-shift-count-overflow" } */
> > > +/* { dg-additional-options "-O2 -Wno-shift-count-overflow
> > > --param=analyzer-max-svalue-depth=19" } */
> > >
> > > struct lisp;
> > > union vectorlike_header { long size; };
> > > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/vrp24.c
> > > b/gcc/testsuite/gcc.dg/tree-ssa/vrp24.c
> > > index c28ca473fc6..f237b7741ec 100644
> > > --- a/gcc/testsuite/gcc.dg/tree-ssa/vrp24.c
> > > +++ b/gcc/testsuite/gcc.dg/tree-ssa/vrp24.c
> > > @@ -89,5 +89,5 @@ L7:
> > > boolean operation. */
> > >
> > > /* { dg-final { scan-tree-dump-times "Simplified relational" 2 "evrp" }
> > > } */
> > > -/* { dg-final { scan-tree-dump-times "if " 3 "optimized" } } */
> > > +/* { dg-final { scan-tree-dump-times "if " 4 "optimized" } } */
> > >
> > > --
> > > 2.43.0
> > >
