Re: [PATCH] Implement known/maybe fpclassify like API for frange.

[Richard Biener <richard.guenther@gmail.com>]

> Am 08.09.2022 um 08:28 schrieb Aldy Hernandez <aldyh@redhat.com>:
> 
> This is what I have in mind for the fpclassify-like methods on the
> current implementation.  I'll get to the removal of the tristates after
> Cauldron.
> 
> As you mentioned, isnormal is kinda tricky, and likely to be confusing
> for the user.  We can revisit it if it's important.

Yeah.

> ?? I assume maybe_inf() does not return true for NAN ??

Only maybe_nan and known_nan return true for NaN.

> 
> Also, what are your thoughts on signbit?  Perhaps a method returning
> true if we're sure, and the actual signbit result as a reference?
> 
>    bool known_signbit (const frange &r, int &signbit);

That works for me.  You could look at
Tree_expr_nonnegative_p to see if that’s good enough to be used there.  Not sure if -0. is negative or if only x for which x < 0. counts as such..  But yes, access to the sign bit is useful.

> How does this look?  I must say, the uses look much cleaner.

Yes, I like it.

Richard 

> Aldy
> 
> gcc/ChangeLog:
> 
>    * gimple-range-fold.cc
>    (fold_using_range::range_of_builtin_int_call): Use fpclassify like API.
>    * range-op-float.cc (finite_operand_p): Same.
>    (finite_operands_p): Same.
>    (foperator_lt::fold_range): Same.
>    (foperator_le::fold_range): Same.
>    (foperator_gt::fold_range): Same.
>    (foperator_ge::fold_range): Same.
>    (foperator_unordered::fold_range): Same.
>    (foperator_unordered::op1_range): Same.
>    (foperator_ordered::fold_range): Same.
>    * value-range.cc (frange::set_nan): Same.
>    (frange::set_signbit): Same.
>    (frange::union_): Same.
>    (frange::intersect): Same.
>    (frange::operator==): Same.
>    (frange::singleton_p): Same.
>    (frange::verify_range): Same.
>    (range_tests_nan): Same.
>    (range_tests_floats): Same.
>    * value-range.h(frange::known_finite): New.
>    (frange::maybe_inf): New.
>    (frange::known_inf): New.
>    (frange::maybe_nan): New.
>    (frange::known_nan): New.
> ---
> gcc/gimple-range-fold.cc |  2 +-
> gcc/range-op-float.cc    | 26 ++++++++-----------
> gcc/value-range.cc       | 37 ++++++++++++++-------------
> gcc/value-range.h        | 54 +++++++++++++++++++++++++++++++++++++++-
> 4 files changed, 83 insertions(+), 36 deletions(-)
> 
> diff --git a/gcc/gimple-range-fold.cc b/gcc/gimple-range-fold.cc
> index c9c7a2ccc70..47a1f49eb36 100644
> --- a/gcc/gimple-range-fold.cc
> +++ b/gcc/gimple-range-fold.cc
> @@ -1031,7 +1031,7 @@ fold_using_range::range_of_builtin_int_call (irange &r, gcall *call,
>      {
>        if (tmp.get_signbit ().varying_p ()
>        // FIXME: We don't support signed NANs yet.
> -        || !tmp.get_nan ().no_p ())
> +        || tmp.maybe_nan ())
>          return false;
>        if (tmp.get_signbit ().yes_p ())
>          r.set_nonzero (type);
> diff --git a/gcc/range-op-float.cc b/gcc/range-op-float.cc
> index 5fbbaa1fb36..0f928b6c098 100644
> --- a/gcc/range-op-float.cc
> +++ b/gcc/range-op-float.cc
> @@ -167,7 +167,7 @@ frange_set_nan (frange &r, tree type)
> static inline bool
> finite_operand_p (const frange &op1)
> {
> -  return flag_finite_math_only || op1.get_nan ().no_p ();
> +  return flag_finite_math_only || !op1.maybe_nan ();
> }
> 
> // Return TRUE if OP1 and OP2 are known to be free of NANs.
> @@ -175,9 +175,7 @@ finite_operand_p (const frange &op1)
> static inline bool
> finite_operands_p (const frange &op1, const frange &op2)
> {
> -  return (flag_finite_math_only
> -      || (op1.get_nan ().no_p ()
> -          && op2.get_nan ().no_p ()));
> +  return flag_finite_math_only || (!op1.maybe_nan () && !op2.maybe_nan ());
> }
> 
> // Floating version of relop_early_resolve that takes into account NAN
> @@ -546,7 +544,7 @@ foperator_lt::fold_range (irange &r, tree type,
>       else
>    r = range_true_and_false (type);
>     }
> -  else if (op1.get_nan ().yes_p () || op2.get_nan ().yes_p ())
> +  else if (op1.known_nan () || op2.known_nan ())
>     r = range_false (type);
>   else
>     r = range_true_and_false (type);
> @@ -648,7 +646,7 @@ foperator_le::fold_range (irange &r, tree type,
>       else
>    r = range_true_and_false (type);
>     }
> -  else if (op1.get_nan ().yes_p () || op2.get_nan ().yes_p ())
> +  else if (op1.known_nan () || op2.known_nan ())
>     r = range_false (type);
>   else
>     r = range_true_and_false (type);
> @@ -742,7 +740,7 @@ foperator_gt::fold_range (irange &r, tree type,
>       else
>    r = range_true_and_false (type);
>     }
> -  else if (op1.get_nan ().yes_p () || op2.get_nan ().yes_p ())
> +  else if (op1.known_nan () || op2.known_nan ())
>     r = range_false (type);
>   else
>     r = range_true_and_false (type);
> @@ -844,7 +842,7 @@ foperator_ge::fold_range (irange &r, tree type,
>       else
>    r = range_true_and_false (type);
>     }
> -  else if (op1.get_nan ().yes_p () || op2.get_nan ().yes_p ())
> +  else if (op1.known_nan () || op2.known_nan ())
>     r = range_false (type);
>   else
>     r = range_true_and_false (type);
> @@ -927,10 +925,10 @@ foperator_unordered::fold_range (irange &r, tree type,
>                 relation_kind) const
> {
>   // UNORDERED is TRUE if either operand is a NAN.
> -  if (op1.get_nan ().yes_p () || op2.get_nan ().yes_p ())
> +  if (op1.known_nan () || op2.known_nan ())
>     r = range_true (type);
>   // UNORDERED is FALSE if neither operand is a NAN.
> -  else if (op1.get_nan ().no_p () && op2.get_nan ().no_p ())
> +  else if (!op1.maybe_nan () && !op2.maybe_nan ())
>     r = range_false (type);
>   else
>     r = range_true_and_false (type);
> @@ -949,7 +947,7 @@ foperator_unordered::op1_range (frange &r, tree type,
>       r.set_varying (type);
>       // Since at least one operand must be NAN, if one of them is
>       // not, the other must be.
> -      if (op2.get_nan ().no_p ())
> +      if (!op2.maybe_nan ())
>    frange_set_nan (r, type);
>       break;
> 
> @@ -993,11 +991,9 @@ foperator_ordered::fold_range (irange &r, tree type,
>                   const frange &op1, const frange &op2,
>                   relation_kind) const
> {
> -  // ORDERED is TRUE if neither operand is a NAN.
> -  if (op1.get_nan ().no_p () && op2.get_nan ().no_p ())
> +  if (!op1.maybe_nan () && !op2.maybe_nan ())
>     r = range_true (type);
> -  // ORDERED is FALSE if either operand is a NAN.
> -  else if (op1.get_nan ().yes_p () || op2.get_nan ().yes_p ())
> +  else if (op1.known_nan () || op2.known_nan ())
>     r = range_false (type);
>   else
>     r = range_true_and_false (type);
> diff --git a/gcc/value-range.cc b/gcc/value-range.cc
> index c3f668a811a..364919ca5c6 100644
> --- a/gcc/value-range.cc
> +++ b/gcc/value-range.cc
> @@ -274,7 +274,7 @@ frange::set_nan (fp_prop::kind k)
> {
>   if (k == fp_prop::YES)
>     {
> -      if (get_nan ().no_p ())
> +      if (!maybe_nan ())
>    {
>      set_undefined ();
>      return;
> @@ -284,7 +284,7 @@ frange::set_nan (fp_prop::kind k)
>       return;
>     }
> 
> -  if (k == fp_prop::NO && get_nan ().yes_p ())
> +  if (k == fp_prop::NO && known_nan ())
>     {
>       set_undefined ();
>       return;
> @@ -308,7 +308,7 @@ frange::set_signbit (fp_prop::kind k)
>   gcc_checking_assert (m_type);
> 
>   // No additional adjustments are needed for a NAN.
> -  if (get_nan ().yes_p ())
> +  if (known_nan ())
>     {
>       m_props.set_signbit (k);
>       return;
> @@ -467,7 +467,7 @@ frange::union_ (const vrange &v)
> 
>   // If one side has a NAN, the union is the other side, plus the union
>   // of the properties and the possibility of a NAN.
> -  if (get_nan ().yes_p ())
> +  if (known_nan ())
>     {
>       frange_props save = m_props;
>       *this = r;
> @@ -478,7 +478,7 @@ frange::union_ (const vrange &v)
>    verify_range ();
>       return true;
>     }
> -  if (r.get_nan ().yes_p ())
> +  if (r.known_nan ())
>     {
>       m_props.union_ (r.m_props);
>       set_nan (fp_prop::VARYING);
> @@ -525,7 +525,7 @@ frange::intersect (const vrange &v)
> 
>   // If two NANs are not exactly the same, drop to an unknown NAN,
>   // otherwise there's nothing to do.
> -  if (get_nan ().yes_p () && r.get_nan ().yes_p ())
> +  if (known_nan () && r.known_nan ())
>     {
>       if (m_props == r.m_props)
>    return false;
> @@ -534,7 +534,7 @@ frange::intersect (const vrange &v)
>       return true;
>     }
>   // ?? Perhaps the intersection of a NAN and anything is a NAN ??.
> -  if (get_nan ().yes_p () || r.get_nan ().yes_p ())
> +  if (known_nan () || r.known_nan ())
>     {
>       set_varying (m_type);
>       return true;
> @@ -590,8 +590,7 @@ frange::operator== (const frange &src) const
>       if (varying_p ())
>    return types_compatible_p (m_type, src.m_type);
> 
> -      if (m_props.get_nan ().yes_p ()
> -      || src.m_props.get_nan ().yes_p ())
> +      if (known_nan () || src.known_nan ())
>    return false;
> 
>       return (real_identical (&m_min, &src.m_min)
> @@ -644,7 +643,7 @@ frange::singleton_p (tree *result) const
>   if (m_kind == VR_RANGE && real_identical (&m_min, &m_max))
>     {
>       // Return false for any singleton that may be a NAN.
> -      if (HONOR_NANS (m_type) && !get_nan ().no_p ())
> +      if (HONOR_NANS (m_type) && maybe_nan ())
>    return false;
> 
>       // Return the appropriate zero if known.
> @@ -701,7 +700,7 @@ frange::verify_range ()
>     {
>       // If either is a NAN, both must be a NAN.
>       gcc_checking_assert (real_identical (&m_min, &m_max));
> -      gcc_checking_assert (get_nan ().yes_p ());
> +      gcc_checking_assert (known_nan ());
>     }
>   else
>     // Make sure we don't have swapped ranges.
> @@ -710,7 +709,7 @@ frange::verify_range ()
>   // If we're absolutely sure we have a NAN, the endpoints should
>   // reflect this, otherwise we'd have more than one way to represent
>   // a NAN.
> -  if (m_props.get_nan ().yes_p ())
> +  if (known_nan ())
>     {
>       gcc_checking_assert (real_isnan (&m_min));
>       gcc_checking_assert (real_isnan (&m_max));
> @@ -3637,7 +3636,7 @@ range_tests_nan ()
>   ASSERT_FALSE (r0 == r0);
>   ASSERT_TRUE (r0 != r0);
> 
> -  // [5,6] U NAN is [5,6] with an unknown NAN bit.
> +  // [5,6] U NAN.
>   r0 = frange_float ("5", "6");
>   r0.set_nan (fp_prop::NO);
>   r1 = frange_nan (float_type_node);
> @@ -3646,7 +3645,7 @@ range_tests_nan ()
>   real_from_string (&r, "6");
>   ASSERT_TRUE (real_identical (&q, &r0.lower_bound ()));
>   ASSERT_TRUE (real_identical (&r, &r0.upper_bound ()));
> -  ASSERT_TRUE (r0.get_nan ().varying_p ());
> +  ASSERT_TRUE (r0.maybe_nan ());
> 
>   // NAN U NAN = NAN
>   r0 = frange_nan (float_type_node);
> @@ -3654,7 +3653,7 @@ range_tests_nan ()
>   r0.union_ (r1);
>   ASSERT_TRUE (real_isnan (&r0.lower_bound ()));
>   ASSERT_TRUE (real_isnan (&r1.upper_bound ()));
> -  ASSERT_TRUE (r0.get_nan ().yes_p ());
> +  ASSERT_TRUE (r0.known_nan ());
> 
>   // [INF, INF] ^ NAN = VARYING
>   r0 = frange_nan (float_type_node);
> @@ -3666,18 +3665,18 @@ range_tests_nan ()
>   r0 = frange_nan (float_type_node);
>   r1 = frange_nan (float_type_node);
>   r0.intersect (r1);
> -  ASSERT_TRUE (r0.get_nan ().yes_p ());
> +  ASSERT_TRUE (r0.known_nan ());
> 
>   // VARYING ^ NAN = NAN.
>   r0 = frange_nan (float_type_node);
>   r1.set_varying (float_type_node);
>   r0.intersect (r1);
> -  ASSERT_TRUE (r0.get_nan ().yes_p ());
> +  ASSERT_TRUE (r0.known_nan ());
> 
>   // Setting the NAN bit to yes, forces to range to [NAN, NAN].
>   r0.set_varying (float_type_node);
>   r0.set_nan (fp_prop::YES);
> -  ASSERT_TRUE (r0.get_nan ().yes_p ());
> +  ASSERT_TRUE (r0.known_nan ());
>   ASSERT_TRUE (real_isnan (&r0.lower_bound ()));
>   ASSERT_TRUE (real_isnan (&r0.upper_bound ()));
> }
> @@ -3795,7 +3794,7 @@ range_tests_floats ()
>   // A range of [-INF,+INF] is actually VARYING if no other properties
>   // are set.
>   r0 = frange_float ("-Inf", "+Inf");
> -  if (r0.get_nan ().varying_p ())
> +  if (r0.maybe_nan ())
>     ASSERT_TRUE (r0.varying_p ());
>   // ...unless it has some special property...
>   r0.set_nan (fp_prop::NO);
> diff --git a/gcc/value-range.h b/gcc/value-range.h
> index 645dc76c33a..e426225eabf 100644
> --- a/gcc/value-range.h
> +++ b/gcc/value-range.h
> @@ -330,6 +330,7 @@ private:
> class frange : public vrange
> {
>   friend class frange_storage_slot;
> +  friend class vrange_printer;
> public:
>   frange ();
>   frange (const frange &);
> @@ -366,12 +367,19 @@ public:
>   const REAL_VALUE_TYPE &lower_bound () const;
>   const REAL_VALUE_TYPE &upper_bound () const;
> 
> +  // fpclassify like API
> +  bool known_finite () const;
> +  bool maybe_inf () const;
> +  bool known_inf () const;
> +  bool maybe_nan () const;
> +  bool known_nan () const;
> +
>   // Accessors for FP properties.
> -  fp_prop get_nan () const { return m_props.get_nan (); }
>   void set_nan (fp_prop::kind f);
>   fp_prop get_signbit () const { return m_props.get_signbit (); }
>   void set_signbit (fp_prop::kind);
> private:
> +  fp_prop get_nan () const { return m_props.get_nan (); }
>   void verify_range ();
>   bool normalize_kind ();
> 
> @@ -1187,4 +1195,48 @@ frange_nan (tree type)
>   return frange (type, r, r);
> }
> 
> +// Return TRUE if range is known to be finite.
> +
> +inline bool
> +frange::known_finite () const
> +{
> +  if (undefined_p () || varying_p () || m_kind == VR_ANTI_RANGE)
> +    return false;
> +  return (!real_isnan (&m_min)
> +      && !real_isinf (&m_min)
> +      && !real_isinf (&m_max));
> +}
> +
> +// Return TRUE if range may be infinite.
> +
> +inline bool
> +frange::maybe_inf () const
> +{
> +  if (undefined_p () || m_kind == VR_ANTI_RANGE)
> +    return false;
> +  if (varying_p ())
> +    return true;
> +  return real_isinf (&m_min) || real_isinf (&m_max);
> +}
> +
> +inline bool
> +frange::known_inf () const
> +{
> +  return (m_kind == VR_RANGE
> +      && real_identical (&m_min, &m_max)
> +      && real_isinf (&m_min));
> +}
> +
> +inline bool
> +frange::maybe_nan () const
> +{
> +  return !get_nan ().no_p ();
> +}
> +
> +inline bool
> +frange::known_nan () const
> +{
> +  return get_nan ().yes_p ();
> +}
> +
> #endif // GCC_VALUE_RANGE_H
> -- 
> 2.37.1
>