On Mon, May 12, 2025 at 7:16 PM Andrew Pinski <pins...@gmail.com> wrote:
>
> On Mon, May 12, 2025 at 3:51 AM Richard Biener
> <richard.guent...@gmail.com> wrote:
> >
> > On Sat, May 10, 2025 at 3:19 AM Andrew Pinski <quic_apin...@quicinc.com>
> > wrote:
> > >
> > > with -ftrapping-math -fnon-call-exceptions and:
> > > ```
> > > tmp = FP0 CMP FP1;
> > >
> > > if (tmp != 0) ...
> > > ```
> > > a call fold_stmt on the GIMPLE_COND will replace the above with
> > > a new tmp each time and we even lose the eh informatin on the
> > > previous comparison too.
> > >
> > > gcc/ChangeLog:
> > >
> > > * gimple-fold.cc (replace_stmt_with_simplification): Reject for
> > > noncall exceptions replacing comparison with itself.
> > >
> > > Signed-off-by: Andrew Pinski <quic_apin...@quicinc.com>
> > > ---
> > > gcc/gimple-fold.cc | 26 ++++++++++++++++++++++++++
> > > 1 file changed, 26 insertions(+)
> > >
> > > diff --git a/gcc/gimple-fold.cc b/gcc/gimple-fold.cc
> > > index 7b3a3d30045..4ff5dbb8d50 100644
> > > --- a/gcc/gimple-fold.cc
> > > +++ b/gcc/gimple-fold.cc
> > > @@ -6276,6 +6276,32 @@ replace_stmt_with_simplification
> > > (gimple_stmt_iterator *gsi,
> > > }
> > > else if (!inplace)
> > > {
> > > + /* For throwing comparisons, see if the GIMPLE_COND is the same
> > > as
> > > + the comparison would be.
> > > + This can happen due to the match pattern for
> > > + `(ne (cmp @0 @1) integer_zerop)` which creates a new
> > > expression
> > > + for the comparison. */
> > > + if (TREE_CODE_CLASS (code) == tcc_comparison
> > > + && flag_exceptions
> > > + && cfun->can_throw_non_call_exceptions
> >
> > I think you should allow !cfun here (aka treat it conservatively).
>
> The code right above does not check cfun and that is where I copied
> the condition :
> ```
> else if (TREE_CODE_CLASS (code) == tcc_comparison
> /* GIMPLE_CONDs condition may not throw. */
> && (!flag_exceptions
> || !cfun->can_throw_non_call_exceptions
> || !operation_could_trap_p (code,
> FLOAT_TYPE_P (TREE_TYPE (ops[0])),
> false, NULL_TREE)))
> ```
>
> Should we add the check for cfun there too?
Yeah, I think so.
> >
> > > + && operation_could_trap_p (code,
> > > + FLOAT_TYPE_P (TREE_TYPE
> > > (ops[0])),
> > > + false, NULL_TREE))
> > > + {
> > > + tree lhs = gimple_cond_lhs (cond_stmt);
> > > + if (gimple_cond_code (cond_stmt) == NE_EXPR
> > > + && TREE_CODE (lhs) == SSA_NAME
> > > + && TREE_CODE (TREE_TYPE (lhs)) == BOOLEAN_TYPE
> >
> > Not sure about this - IIRC INTEGER_TYPE with appropriate precision
> > and sign is compatible. I'd just drop this.
>
> I am going to change it to be INTEGRAL_TYPE_P; is that ok? The check
> on the type is mainly to reduce when we walk back (a slight
> optimization).
OK.
Thanks,
Richard.
> Thanks,
> Andrew Pinski
>
> >
> > Otherwise OK. I prefer this one over guarding the match.pd pattern.
> >
> > Thanks,
> > Richard.
> >
> > > + && integer_zerop (gimple_cond_rhs (cond_stmt)))
> > > + {
> > > + gimple *s = SSA_NAME_DEF_STMT (lhs);
> > > + if (is_gimple_assign (s)
> > > + && gimple_assign_rhs_code (s) == code
> > > + && operand_equal_p (gimple_assign_rhs1 (s), ops[0])
> > > + && operand_equal_p (gimple_assign_rhs2 (s), ops[1]))
> > > + return false;
> > > + }
> > > + }
> > > tree res = maybe_push_res_to_seq (res_op, seq);
> > > if (!res)
> > > return false;
> > > --
> > > 2.43.0
> > >
