On Thu, May 9, 2024 at 10:05 AM Mikael Morin <morin-mik...@orange.fr> wrote: > > Hello, > > Le 07/05/2024 à 04:37, HAO CHEN GUI a écrit : > > Hi, > > The former patch adds isfinite optab for __builtin_isfinite. > > https://gcc.gnu.org/pipermail/gcc-patches/2024-April/649339.html > > > > Thus the builtin might not be folded at front end. The range op for > > isfinite is needed for value range analysis. This patch adds them. > > > > Compared to last version, this version fixes a typo. > > > > Bootstrapped and tested on x86 and powerpc64-linux BE and LE with no > > regressions. Is it OK for the trunk? > > > > Thanks > > Gui Haochen > > > > ChangeLog > > Value Range: Add range op for builtin isfinite > > > > The former patch adds optab for builtin isfinite. Thus builtin isfinite > > might > > not be folded at front end. So the range op for isfinite is needed for > > value > > range analysis. This patch adds range op for builtin isfinite. > > > > gcc/ > > * gimple-range-op.cc (class cfn_isfinite): New. > > (op_cfn_finite): New variables. > > (gimple_range_op_handler::maybe_builtin_call): Handle > > CFN_BUILT_IN_ISFINITE. > > > > gcc/testsuite/ > > * gcc/testsuite/gcc.dg/tree-ssa/range-isfinite.c: New test. > > > > patch.diff > > diff --git a/gcc/gimple-range-op.cc b/gcc/gimple-range-op.cc > > index 9de130b4022..99c511728d3 100644 > > --- a/gcc/gimple-range-op.cc > > +++ b/gcc/gimple-range-op.cc > > @@ -1192,6 +1192,56 @@ public: > > } > > } op_cfn_isinf; > > > > +//Implement range operator for CFN_BUILT_IN_ISFINITE > > +class cfn_isfinite : public range_operator > > +{ > > +public: > > + using range_operator::fold_range; > > + using range_operator::op1_range; > > + virtual bool fold_range (irange &r, tree type, const frange &op1, > > + const irange &, relation_trio) const override > > + { > > + if (op1.undefined_p ()) > > + return false; > > + > > + if (op1.known_isfinite ()) > > + { > > + r.set_nonzero (type); > > + return true; > > + } > > + > > + if (op1.known_isnan () > > + || op1.known_isinf ()) > > + { > > + r.set_zero (type); > > + return true; > > + } > > + > > + return false; > I think the canonical API behaviour sets R to varying and returns true > instead of just returning false if nothing is known about the range.
Correct. If we know it's varying, we just set varying and return true. Returning false is usually reserved for "I have no idea". However, every caller of fold_range() should know to ignore a return of false, so you should be safe. > > I'm not sure whether it makes any difference; Aldy can probably tell. > But if the type is bool, varying is [0,1] which is better than unknown > range. Also, I see you're setting zero/nonzero. Is the return type known to be boolean, because if so, we usually prefer to one of: r = range_true () r = range_false () r = range_true_and_false (); It doesn't matter either way, but it's probably best to use these as they force boolean_type_node automatically. I don't have a problem with this patch, but I would prefer the floating point savvy people to review this, as there are no members of the ranger team that are floating point experts :). Also, I see you mention in your original post that this patch was needed as a follow-up to this one: https://gcc.gnu.org/pipermail/gcc-patches/2024-April/649339.html I don't see the above patch in the source tree currently: Thanks. Aldy > > > + } > > + virtual bool op1_range (frange &r, tree type, const irange &lhs, > > + const frange &, relation_trio) const override > > + { > > + if (lhs.zero_p ()) > > + { > > + // The range is [-INF,-INF][+INF,+INF] NAN, but it can't be > > represented. > > + // Set range to varying > > + r.set_varying (type); > > + return true; > > + } > > + > > + if (!range_includes_zero_p (&lhs)) > > + { > > + nan_state nan (false); > > + r.set (type, real_min_representable (type), > > + real_max_representable (type), nan); > > + return true; > > + } > > + > > + return false; > Same here. > > > + } > > +} op_cfn_isfinite; > > + > > // Implement range operator for CFN_BUILT_IN_ > > class cfn_parity : public range_operator > > { >
