[PATCH] frange: dump hex values when dumping FP numbers.

[PATCH] frange: dump hex values when dumping FP numbers.

[Aldy Hernandez]

It has been suggested that if we start bumping numbers by an ULP when
calculating open ranges (for example the numbers less than 3.0) that
dumping these will become increasingly harder to read, and instead we
should opt for the hex representation.  I still find the floating
point representation easier to read for most numbers, but perhaps we
could have both?

With this patch this is the representation for [15.0, 20.0]:

     [frange] float [1.5e+1 (0x0.fp+4), 2.0e+1 (0x0.ap+5)]

Would you find this useful, or should we stick to the hex
representation only (or something altogether different)?

Tested on x86-64 Linux.

gcc/ChangeLog:

	* value-range-pretty-print.cc (vrange_printer::print_real_value): New.
	(vrange_printer::visit): Call print_real_value.
	* value-range-pretty-print.h: New print_real_value.
---
 gcc/value-range-pretty-print.cc | 16 ++++++++++++----
 gcc/value-range-pretty-print.h  |  1 +
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/gcc/value-range-pretty-print.cc b/gcc/value-range-pretty-print.cc
index eb7442229ba..51be037c254 100644
--- a/gcc/value-range-pretty-print.cc
+++ b/gcc/value-range-pretty-print.cc
@@ -117,6 +117,16 @@ vrange_printer::print_irange_bitmasks (const irange &r) const
   pp_string (pp, buf);
 }
 
+void
+vrange_printer::print_real_value (tree type, const REAL_VALUE_TYPE &r) const
+{
+  char s[60];
+  tree t = build_real (type, r);
+  dump_generic_node (pp, t, 0, TDF_NONE, false);
+  real_to_hexadecimal (s, &r, sizeof (s), 0, 1);
+  pp_printf (pp, " (%s)", s);
+}
+
 // Print an frange.
 
 void
@@ -141,11 +151,9 @@ vrange_printer::visit (const frange &r) const
   bool has_endpoints = !r.known_isnan ();
   if (has_endpoints)
     {
-      dump_generic_node (pp,
-			 build_real (type, r.lower_bound ()), 0, TDF_NONE, false);
+      print_real_value (type, r.lower_bound ());
       pp_string (pp, ", ");
-      dump_generic_node (pp,
-			 build_real (type, r.upper_bound ()), 0, TDF_NONE, false);
+      print_real_value (type, r.upper_bound ());
     }
   pp_character (pp, ']');
   print_frange_nan (r);
diff --git a/gcc/value-range-pretty-print.h b/gcc/value-range-pretty-print.h
index 20c26598fe7..a9ae5a7b4cc 100644
--- a/gcc/value-range-pretty-print.h
+++ b/gcc/value-range-pretty-print.h
@@ -32,6 +32,7 @@ private:
   void print_irange_bound (const wide_int &w, tree type) const;
   void print_irange_bitmasks (const irange &) const;
   void print_frange_nan (const frange &) const;
+  void print_real_value (tree type, const REAL_VALUE_TYPE &r) const;
 
   pretty_printer *pp;
 };
-- 
2.37.1

[PATCH] frange: drop endpoints to min/max representable numbers for -ffinite-math-only.

[Aldy Hernandez]

Similarly to how we drop NANs to UNDEFINED when -ffinite-math-only, I
think we can drop the numbers outside of the min/max representable
numbers to the representable number.

This means the endpoings to VR_VARYING for -ffinite-math-only can now
be the min/max representable, instead of -INF and +INF.

Saturating in the setter means that the upcoming implementation for
binary operators no longer have to worry about doing the right
thing for -ffinite-math-only.  If the range goes outside the limits,
it'll get chopped down.

How does this look?

Tested on x86-64 Linux.

gcc/ChangeLog:

	* range-op-float.cc (build_le): Use vrp_val_*.
	(build_lt): Same.
	(build_ge): Same.
	(build_gt): Same.
	* value-range.cc (frange::set): Chop ranges outside of the
	representable numbers for -ffinite-math-only.
	(frange::normalize_kind): Use vrp_val*.
	(frange::verify_range): Same.
	(frange::set_nonnegative): Same.
	(range_tests_floats): Remove tests that depend on -INF and +INF.
	* value-range.h (real_max_representable): Add prototype.
	(real_min_representable): Same.
	(vrp_val_max): Set max representable number for
	-ffinite-math-only.
	(vrp_val_min): Same but for min.
	(frange::set_varying): Use vrp_val*.
---
 gcc/range-op-float.cc | 12 +++++++----
 gcc/value-range.cc    | 46 ++++++++++++++++++++-----------------------
 gcc/value-range.h     | 30 ++++++++++++++++++++++------
 3 files changed, 53 insertions(+), 35 deletions(-)

diff --git a/gcc/range-op-float.cc b/gcc/range-op-float.cc
index 2bd3dc9253f..15ba19c2deb 100644
--- a/gcc/range-op-float.cc
+++ b/gcc/range-op-float.cc
@@ -232,7 +232,8 @@ build_le (frange &r, tree type, const frange &val)
 {
   gcc_checking_assert (!val.known_isnan ());
 
-  r.set (type, dconstninf, val.upper_bound ());
+  REAL_VALUE_TYPE ninf = *TREE_REAL_CST_PTR (vrp_val_min (type));
+  r.set (type, ninf, val.upper_bound ());
 
   // Add both zeros if there's the possibility of zero equality.
   frange_add_zeros (r, type);
@@ -257,7 +258,8 @@ build_lt (frange &r, tree type, const frange &val)
       return false;
     }
   // We only support closed intervals.
-  r.set (type, dconstninf, val.upper_bound ());
+  REAL_VALUE_TYPE ninf = *TREE_REAL_CST_PTR (vrp_val_min (type));
+  r.set (type, ninf, val.upper_bound ());
   return true;
 }
 
@@ -268,7 +270,8 @@ build_ge (frange &r, tree type, const frange &val)
 {
   gcc_checking_assert (!val.known_isnan ());
 
-  r.set (type, val.lower_bound (), dconstinf);
+  REAL_VALUE_TYPE inf = *TREE_REAL_CST_PTR (vrp_val_max (type));
+  r.set (type, val.lower_bound (), inf);
 
   // Add both zeros if there's the possibility of zero equality.
   frange_add_zeros (r, type);
@@ -294,7 +297,8 @@ build_gt (frange &r, tree type, const frange &val)
     }
 
   // We only support closed intervals.
-  r.set (type, val.lower_bound (), dconstinf);
+  REAL_VALUE_TYPE inf = *TREE_REAL_CST_PTR (vrp_val_max (type));
+  r.set (type, val.lower_bound (), inf);
   return true;
 }
 
diff --git a/gcc/value-range.cc b/gcc/value-range.cc
index 7e8028eced2..e57d60e1bac 100644
--- a/gcc/value-range.cc
+++ b/gcc/value-range.cc
@@ -338,6 +338,18 @@ frange::set (tree min, tree max, value_range_kind kind)
       m_neg_nan = false;
     }
 
+  // For -ffinite-math-only we can drop ranges outside the
+  // representable numbers to min/max for the type.
+  if (flag_finite_math_only)
+    {
+      REAL_VALUE_TYPE min_repr = *TREE_REAL_CST_PTR (vrp_val_min (m_type));
+      REAL_VALUE_TYPE max_repr = *TREE_REAL_CST_PTR (vrp_val_max (m_type));
+      if (real_less (&m_min, &min_repr))
+	m_min = min_repr;
+      if (real_less (&max_repr, &m_max))
+	m_max = max_repr;
+    }
+
   // Check for swapped ranges.
   gcc_checking_assert (tree_compare (LE_EXPR, min, max));
 
@@ -371,8 +383,8 @@ bool
 frange::normalize_kind ()
 {
   if (m_kind == VR_RANGE
-      && real_isinf (&m_min, 1)
-      && real_isinf (&m_max, 0))
+      && vrp_val_is_min (build_real (m_type, m_min))
+      && vrp_val_is_max (build_real (m_type, m_max)))
     {
       if (m_pos_nan && m_neg_nan)
 	{
@@ -385,8 +397,8 @@ frange::normalize_kind ()
       if (!m_pos_nan || !m_neg_nan)
 	{
 	  m_kind = VR_RANGE;
-	  m_min = dconstninf;
-	  m_max = dconstinf;
+	  m_min = *TREE_REAL_CST_PTR (vrp_val_min (m_type));
+	  m_max = *TREE_REAL_CST_PTR (vrp_val_max (m_type));
 	  return true;
 	}
     }
@@ -706,8 +718,8 @@ frange::verify_range ()
     case VR_VARYING:
       gcc_checking_assert (m_type);
       gcc_checking_assert (m_pos_nan && m_neg_nan);
-      gcc_checking_assert (real_isinf (&m_min, 1));
-      gcc_checking_assert (real_isinf (&m_max, 0));
+      gcc_checking_assert (vrp_val_is_min (build_real (m_type, m_min)));
+      gcc_checking_assert (vrp_val_is_max (build_real (m_type, m_max)));
       return;
     case VR_RANGE:
       gcc_checking_assert (m_type);
@@ -732,7 +744,8 @@ frange::verify_range ()
   // If all the properties are clear, we better not span the entire
   // domain, because that would make us varying.
   if (m_pos_nan && m_neg_nan)
-    gcc_checking_assert (!real_isinf (&m_min, 1) || !real_isinf (&m_max, 0));
+    gcc_checking_assert (!vrp_val_is_min (build_real (m_type, m_min))
+			 || !vrp_val_is_max (build_real (m_type, m_max)));
 }
 
 // We can't do much with nonzeros yet.
@@ -779,7 +792,7 @@ frange::zero_p () const
 void
 frange::set_nonnegative (tree type)
 {
-  set (type, dconst0, dconstinf);
+  set (type, dconst0, *TREE_REAL_CST_PTR (vrp_val_max (type)));
 
   // Set +NAN as the only possibility.
   if (HONOR_NANS (type))
@@ -3886,23 +3899,6 @@ range_tests_floats ()
   r0.clear_nan ();
   ASSERT_FALSE (r0.varying_p ());
 
-  // The endpoints of a VARYING are +-INF.
-  r0.set_varying (float_type_node);
-  ASSERT_TRUE (real_identical (&r0.lower_bound (), &dconstninf));
-  ASSERT_TRUE (real_identical (&r0.upper_bound (), &dconstinf));
-
-  // The maximum representable range for a type is still a subset of VARYING.
-  REAL_VALUE_TYPE q, r;
-  real_min_representable (&q, float_type_node);
-  real_max_representable (&r, float_type_node);
-  r0 = frange (float_type_node, q, r);
-  // r0 is not a varying, because it does not include -INF/+INF.
-  ASSERT_FALSE (r0.varying_p ());
-  // The upper bound of r0 must be less than +INF.
-  ASSERT_TRUE (real_less (&r0.upper_bound (), &dconstinf));
-  // The lower bound of r0 must be greater than -INF.
-  ASSERT_TRUE (real_less (&dconstninf, &r0.lower_bound ()));
-
   // For most architectures, where float and double are different
   // sizes, having the same endpoints does not necessarily mean the
   // ranges are equal.
diff --git a/gcc/value-range.h b/gcc/value-range.h
index 3668b331187..74879b76b40 100644
--- a/gcc/value-range.h
+++ b/gcc/value-range.h
@@ -593,6 +593,8 @@ extern void dump_value_range (FILE *, const vrange *);
 extern bool vrp_val_is_min (const_tree);
 extern bool vrp_val_is_max (const_tree);
 extern bool vrp_operand_equal_p (const_tree, const_tree);
+inline void real_max_representable (REAL_VALUE_TYPE *r, const_tree type);
+inline void real_min_representable (REAL_VALUE_TYPE *r, const_tree type);
 
 inline value_range_kind
 vrange::kind () const
@@ -1009,7 +1011,15 @@ vrp_val_max (const_tree type)
       return wide_int_to_tree (const_cast<tree> (type), max);
     }
   if (frange::supports_p (type))
-    return build_real (const_cast <tree> (type), dconstinf);
+    {
+      if (flag_finite_math_only)
+	{
+	  REAL_VALUE_TYPE r;
+	  real_max_representable (&r, type);
+	  return build_real (const_cast <tree> (type), r);
+	}
+      return build_real (const_cast <tree> (type), dconstinf);
+    }
   return NULL_TREE;
 }
 
@@ -1023,7 +1033,15 @@ vrp_val_min (const_tree type)
   if (POINTER_TYPE_P (type))
     return build_zero_cst (const_cast<tree> (type));
   if (frange::supports_p (type))
-    return build_real (const_cast <tree> (type), dconstninf);
+    {
+      if (flag_finite_math_only)
+	{
+	  REAL_VALUE_TYPE r;
+	  real_min_representable (&r, type);
+	  return build_real (const_cast <tree> (type), r);
+	}
+      return build_real (const_cast <tree> (type), dconstninf);
+    }
   return NULL_TREE;
 }
 
@@ -1073,8 +1091,8 @@ frange::set_varying (tree type)
 {
   m_kind = VR_VARYING;
   m_type = type;
-  m_min = dconstninf;
-  m_max = dconstinf;
+  m_min = *TREE_REAL_CST_PTR (vrp_val_min (type));
+  m_max = *TREE_REAL_CST_PTR (vrp_val_max (type));
   m_pos_nan = true;
   m_neg_nan = true;
 }
@@ -1133,7 +1151,7 @@ frange::clear_nan ()
 // Set R to maximum representable value for TYPE.
 
 inline void
-real_max_representable (REAL_VALUE_TYPE *r, tree type)
+real_max_representable (REAL_VALUE_TYPE *r, const_tree type)
 {
   char buf[128];
   get_max_float (REAL_MODE_FORMAT (TYPE_MODE (type)),
@@ -1145,7 +1163,7 @@ real_max_representable (REAL_VALUE_TYPE *r, tree type)
 // Set R to minimum representable value for TYPE.
 
 inline void
-real_min_representable (REAL_VALUE_TYPE *r, tree type)
+real_min_representable (REAL_VALUE_TYPE *r, const_tree type)
 {
   real_max_representable (r, type);
   *r = real_value_negate (r);
-- 
2.37.1

Re: [PATCH] frange: drop endpoints to min/max representable numbers for -ffinite-math-only.

[Richard Biener]

On Thu, Sep 22, 2022 at 6:49 PM Aldy Hernandez <aldyh@redhat.com> wrote:
>
> Similarly to how we drop NANs to UNDEFINED when -ffinite-math-only, I
> think we can drop the numbers outside of the min/max representable
> numbers to the representable number.
>
> This means the endpoings to VR_VARYING for -ffinite-math-only can now
> be the min/max representable, instead of -INF and +INF.
>
> Saturating in the setter means that the upcoming implementation for
> binary operators no longer have to worry about doing the right
> thing for -ffinite-math-only.  If the range goes outside the limits,
> it'll get chopped down.
>
> How does this look?
>
> Tested on x86-64 Linux.
>
> gcc/ChangeLog:
>
>         * range-op-float.cc (build_le): Use vrp_val_*.
>         (build_lt): Same.
>         (build_ge): Same.
>         (build_gt): Same.
>         * value-range.cc (frange::set): Chop ranges outside of the
>         representable numbers for -ffinite-math-only.
>         (frange::normalize_kind): Use vrp_val*.
>         (frange::verify_range): Same.
>         (frange::set_nonnegative): Same.
>         (range_tests_floats): Remove tests that depend on -INF and +INF.
>         * value-range.h (real_max_representable): Add prototype.
>         (real_min_representable): Same.
>         (vrp_val_max): Set max representable number for
>         -ffinite-math-only.
>         (vrp_val_min): Same but for min.
>         (frange::set_varying): Use vrp_val*.
> ---
>  gcc/range-op-float.cc | 12 +++++++----
>  gcc/value-range.cc    | 46 ++++++++++++++++++++-----------------------
>  gcc/value-range.h     | 30 ++++++++++++++++++++++------
>  3 files changed, 53 insertions(+), 35 deletions(-)
>
> diff --git a/gcc/range-op-float.cc b/gcc/range-op-float.cc
> index 2bd3dc9253f..15ba19c2deb 100644
> --- a/gcc/range-op-float.cc
> +++ b/gcc/range-op-float.cc
> @@ -232,7 +232,8 @@ build_le (frange &r, tree type, const frange &val)
>  {
>    gcc_checking_assert (!val.known_isnan ());
>
> -  r.set (type, dconstninf, val.upper_bound ());
> +  REAL_VALUE_TYPE ninf = *TREE_REAL_CST_PTR (vrp_val_min (type));
> +  r.set (type, ninf, val.upper_bound ());
>
>    // Add both zeros if there's the possibility of zero equality.
>    frange_add_zeros (r, type);
> @@ -257,7 +258,8 @@ build_lt (frange &r, tree type, const frange &val)
>        return false;
>      }
>    // We only support closed intervals.
> -  r.set (type, dconstninf, val.upper_bound ());
> +  REAL_VALUE_TYPE ninf = *TREE_REAL_CST_PTR (vrp_val_min (type));
> +  r.set (type, ninf, val.upper_bound ());
>    return true;
>  }
>
> @@ -268,7 +270,8 @@ build_ge (frange &r, tree type, const frange &val)
>  {
>    gcc_checking_assert (!val.known_isnan ());
>
> -  r.set (type, val.lower_bound (), dconstinf);
> +  REAL_VALUE_TYPE inf = *TREE_REAL_CST_PTR (vrp_val_max (type));
> +  r.set (type, val.lower_bound (), inf);
>
>    // Add both zeros if there's the possibility of zero equality.
>    frange_add_zeros (r, type);
> @@ -294,7 +297,8 @@ build_gt (frange &r, tree type, const frange &val)
>      }
>
>    // We only support closed intervals.
> -  r.set (type, val.lower_bound (), dconstinf);
> +  REAL_VALUE_TYPE inf = *TREE_REAL_CST_PTR (vrp_val_max (type));
> +  r.set (type, val.lower_bound (), inf);
>    return true;
>  }
>
> diff --git a/gcc/value-range.cc b/gcc/value-range.cc
> index 7e8028eced2..e57d60e1bac 100644
> --- a/gcc/value-range.cc
> +++ b/gcc/value-range.cc
> @@ -338,6 +338,18 @@ frange::set (tree min, tree max, value_range_kind kind)
>        m_neg_nan = false;
>      }
>
> +  // For -ffinite-math-only we can drop ranges outside the
> +  // representable numbers to min/max for the type.
> +  if (flag_finite_math_only)
> +    {
> +      REAL_VALUE_TYPE min_repr = *TREE_REAL_CST_PTR (vrp_val_min (m_type));
> +      REAL_VALUE_TYPE max_repr = *TREE_REAL_CST_PTR (vrp_val_max (m_type));
> +      if (real_less (&m_min, &min_repr))
> +       m_min = min_repr;
> +      if (real_less (&max_repr, &m_max))
> +       m_max = max_repr;

I think you want to re-formulate that in terms of real_isinf() and
change those to the max representable values.

> +    }
> +
>    // Check for swapped ranges.
>    gcc_checking_assert (tree_compare (LE_EXPR, min, max));
>
> @@ -371,8 +383,8 @@ bool
>  frange::normalize_kind ()
>  {
>    if (m_kind == VR_RANGE
> -      && real_isinf (&m_min, 1)
> -      && real_isinf (&m_max, 0))
> +      && vrp_val_is_min (build_real (m_type, m_min))
> +      && vrp_val_is_max (build_real (m_type, m_max)))

I think this suggests that keeping +-Inf might be the better choice.
At least you want to change vrp_val_is_min/max to take a REAL_VALUE_TYPE
to avoid building four tree nodes for just this compare?

>      {
>        if (m_pos_nan && m_neg_nan)
>         {
> @@ -385,8 +397,8 @@ frange::normalize_kind ()
>        if (!m_pos_nan || !m_neg_nan)
>         {
>           m_kind = VR_RANGE;
> -         m_min = dconstninf;
> -         m_max = dconstinf;
> +         m_min = *TREE_REAL_CST_PTR (vrp_val_min (m_type));
> +         m_max = *TREE_REAL_CST_PTR (vrp_val_max (m_type));
>           return true;
>         }
>      }
> @@ -706,8 +718,8 @@ frange::verify_range ()
>      case VR_VARYING:
>        gcc_checking_assert (m_type);
>        gcc_checking_assert (m_pos_nan && m_neg_nan);
> -      gcc_checking_assert (real_isinf (&m_min, 1));
> -      gcc_checking_assert (real_isinf (&m_max, 0));
> +      gcc_checking_assert (vrp_val_is_min (build_real (m_type, m_min)));
> +      gcc_checking_assert (vrp_val_is_max (build_real (m_type, m_max)));
>        return;
>      case VR_RANGE:
>        gcc_checking_assert (m_type);
> @@ -732,7 +744,8 @@ frange::verify_range ()
>    // If all the properties are clear, we better not span the entire
>    // domain, because that would make us varying.
>    if (m_pos_nan && m_neg_nan)
> -    gcc_checking_assert (!real_isinf (&m_min, 1) || !real_isinf (&m_max, 0));
> +    gcc_checking_assert (!vrp_val_is_min (build_real (m_type, m_min))
> +                        || !vrp_val_is_max (build_real (m_type, m_max)));
>  }
>
>  // We can't do much with nonzeros yet.
> @@ -779,7 +792,7 @@ frange::zero_p () const
>  void
>  frange::set_nonnegative (tree type)
>  {
> -  set (type, dconst0, dconstinf);
> +  set (type, dconst0, *TREE_REAL_CST_PTR (vrp_val_max (type)));
>
>    // Set +NAN as the only possibility.
>    if (HONOR_NANS (type))
> @@ -3886,23 +3899,6 @@ range_tests_floats ()
>    r0.clear_nan ();
>    ASSERT_FALSE (r0.varying_p ());
>
> -  // The endpoints of a VARYING are +-INF.
> -  r0.set_varying (float_type_node);
> -  ASSERT_TRUE (real_identical (&r0.lower_bound (), &dconstninf));
> -  ASSERT_TRUE (real_identical (&r0.upper_bound (), &dconstinf));
> -
> -  // The maximum representable range for a type is still a subset of VARYING.
> -  REAL_VALUE_TYPE q, r;
> -  real_min_representable (&q, float_type_node);
> -  real_max_representable (&r, float_type_node);
> -  r0 = frange (float_type_node, q, r);
> -  // r0 is not a varying, because it does not include -INF/+INF.
> -  ASSERT_FALSE (r0.varying_p ());
> -  // The upper bound of r0 must be less than +INF.
> -  ASSERT_TRUE (real_less (&r0.upper_bound (), &dconstinf));
> -  // The lower bound of r0 must be greater than -INF.
> -  ASSERT_TRUE (real_less (&dconstninf, &r0.lower_bound ()));
> -
>    // For most architectures, where float and double are different
>    // sizes, having the same endpoints does not necessarily mean the
>    // ranges are equal.
> diff --git a/gcc/value-range.h b/gcc/value-range.h
> index 3668b331187..74879b76b40 100644
> --- a/gcc/value-range.h
> +++ b/gcc/value-range.h
> @@ -593,6 +593,8 @@ extern void dump_value_range (FILE *, const vrange *);
>  extern bool vrp_val_is_min (const_tree);
>  extern bool vrp_val_is_max (const_tree);
>  extern bool vrp_operand_equal_p (const_tree, const_tree);
> +inline void real_max_representable (REAL_VALUE_TYPE *r, const_tree type);
> +inline void real_min_representable (REAL_VALUE_TYPE *r, const_tree type);
>
>  inline value_range_kind
>  vrange::kind () const
> @@ -1009,7 +1011,15 @@ vrp_val_max (const_tree type)
>        return wide_int_to_tree (const_cast<tree> (type), max);
>      }
>    if (frange::supports_p (type))
> -    return build_real (const_cast <tree> (type), dconstinf);
> +    {
> +      if (flag_finite_math_only)
> +       {
> +         REAL_VALUE_TYPE r;
> +         real_max_representable (&r, type);
> +         return build_real (const_cast <tree> (type), r);
> +       }
> +      return build_real (const_cast <tree> (type), dconstinf);

Overall there's too many places affected IMHO.  The above also shows
we're building trees a lot - note that REAL_CST are _not_ shared, so
each time you call build_real a new REAL_CST tree node is allocated
(and later GCed if no longer used).

I thought vrp_val_min/max are legacy by now ...

That said, using trees for the range representation might not be the
very best thing to do, it's unnecessarily wrapping things.

> +    }
>    return NULL_TREE;
>  }
>
> @@ -1023,7 +1033,15 @@ vrp_val_min (const_tree type)
>    if (POINTER_TYPE_P (type))
>      return build_zero_cst (const_cast<tree> (type));
>    if (frange::supports_p (type))
> -    return build_real (const_cast <tree> (type), dconstninf);
> +    {
> +      if (flag_finite_math_only)
> +       {
> +         REAL_VALUE_TYPE r;
> +         real_min_representable (&r, type);
> +         return build_real (const_cast <tree> (type), r);
> +       }
> +      return build_real (const_cast <tree> (type), dconstninf);
> +    }
>    return NULL_TREE;
>  }
>
> @@ -1073,8 +1091,8 @@ frange::set_varying (tree type)
>  {
>    m_kind = VR_VARYING;
>    m_type = type;
> -  m_min = dconstninf;
> -  m_max = dconstinf;
> +  m_min = *TREE_REAL_CST_PTR (vrp_val_min (type));
> +  m_max = *TREE_REAL_CST_PTR (vrp_val_max (type));
>    m_pos_nan = true;
>    m_neg_nan = true;
>  }
> @@ -1133,7 +1151,7 @@ frange::clear_nan ()
>  // Set R to maximum representable value for TYPE.
>
>  inline void
> -real_max_representable (REAL_VALUE_TYPE *r, tree type)
> +real_max_representable (REAL_VALUE_TYPE *r, const_tree type)
>  {
>    char buf[128];
>    get_max_float (REAL_MODE_FORMAT (TYPE_MODE (type)),
> @@ -1145,7 +1163,7 @@ real_max_representable (REAL_VALUE_TYPE *r, tree type)
>  // Set R to minimum representable value for TYPE.
>
>  inline void
> -real_min_representable (REAL_VALUE_TYPE *r, tree type)
> +real_min_representable (REAL_VALUE_TYPE *r, const_tree type)
>  {
>    real_max_representable (r, type);
>    *r = real_value_negate (r);
> --
> 2.37.1
>

Re: [PATCH] frange: drop endpoints to min/max representable numbers for -ffinite-math-only.

[Aldy Hernandez]

[-- Attachment #1: Type: text/plain, Size: 11485 bytes --]

Ughhh, my bad.  I had reworked this as soon as Jakub said we couldn't
cache the min/max in TYPE_MIN/MAX_VALUE, but forgot to send it.  And
yes...the incessant wrapping was very annoying.  It's all fixed.

Let me know what you think.
Aldy

On Fri, Sep 23, 2022 at 9:04 AM Richard Biener
<richard.guenther@gmail.com> wrote:
>
> On Thu, Sep 22, 2022 at 6:49 PM Aldy Hernandez <aldyh@redhat.com> wrote:
> >
> > Similarly to how we drop NANs to UNDEFINED when -ffinite-math-only, I
> > think we can drop the numbers outside of the min/max representable
> > numbers to the representable number.
> >
> > This means the endpoings to VR_VARYING for -ffinite-math-only can now
> > be the min/max representable, instead of -INF and +INF.
> >
> > Saturating in the setter means that the upcoming implementation for
> > binary operators no longer have to worry about doing the right
> > thing for -ffinite-math-only.  If the range goes outside the limits,
> > it'll get chopped down.
> >
> > How does this look?
> >
> > Tested on x86-64 Linux.
> >
> > gcc/ChangeLog:
> >
> >         * range-op-float.cc (build_le): Use vrp_val_*.
> >         (build_lt): Same.
> >         (build_ge): Same.
> >         (build_gt): Same.
> >         * value-range.cc (frange::set): Chop ranges outside of the
> >         representable numbers for -ffinite-math-only.
> >         (frange::normalize_kind): Use vrp_val*.
> >         (frange::verify_range): Same.
> >         (frange::set_nonnegative): Same.
> >         (range_tests_floats): Remove tests that depend on -INF and +INF.
> >         * value-range.h (real_max_representable): Add prototype.
> >         (real_min_representable): Same.
> >         (vrp_val_max): Set max representable number for
> >         -ffinite-math-only.
> >         (vrp_val_min): Same but for min.
> >         (frange::set_varying): Use vrp_val*.
> > ---
> >  gcc/range-op-float.cc | 12 +++++++----
> >  gcc/value-range.cc    | 46 ++++++++++++++++++++-----------------------
> >  gcc/value-range.h     | 30 ++++++++++++++++++++++------
> >  3 files changed, 53 insertions(+), 35 deletions(-)
> >
> > diff --git a/gcc/range-op-float.cc b/gcc/range-op-float.cc
> > index 2bd3dc9253f..15ba19c2deb 100644
> > --- a/gcc/range-op-float.cc
> > +++ b/gcc/range-op-float.cc
> > @@ -232,7 +232,8 @@ build_le (frange &r, tree type, const frange &val)
> >  {
> >    gcc_checking_assert (!val.known_isnan ());
> >
> > -  r.set (type, dconstninf, val.upper_bound ());
> > +  REAL_VALUE_TYPE ninf = *TREE_REAL_CST_PTR (vrp_val_min (type));
> > +  r.set (type, ninf, val.upper_bound ());
> >
> >    // Add both zeros if there's the possibility of zero equality.
> >    frange_add_zeros (r, type);
> > @@ -257,7 +258,8 @@ build_lt (frange &r, tree type, const frange &val)
> >        return false;
> >      }
> >    // We only support closed intervals.
> > -  r.set (type, dconstninf, val.upper_bound ());
> > +  REAL_VALUE_TYPE ninf = *TREE_REAL_CST_PTR (vrp_val_min (type));
> > +  r.set (type, ninf, val.upper_bound ());
> >    return true;
> >  }
> >
> > @@ -268,7 +270,8 @@ build_ge (frange &r, tree type, const frange &val)
> >  {
> >    gcc_checking_assert (!val.known_isnan ());
> >
> > -  r.set (type, val.lower_bound (), dconstinf);
> > +  REAL_VALUE_TYPE inf = *TREE_REAL_CST_PTR (vrp_val_max (type));
> > +  r.set (type, val.lower_bound (), inf);
> >
> >    // Add both zeros if there's the possibility of zero equality.
> >    frange_add_zeros (r, type);
> > @@ -294,7 +297,8 @@ build_gt (frange &r, tree type, const frange &val)
> >      }
> >
> >    // We only support closed intervals.
> > -  r.set (type, val.lower_bound (), dconstinf);
> > +  REAL_VALUE_TYPE inf = *TREE_REAL_CST_PTR (vrp_val_max (type));
> > +  r.set (type, val.lower_bound (), inf);
> >    return true;
> >  }
> >
> > diff --git a/gcc/value-range.cc b/gcc/value-range.cc
> > index 7e8028eced2..e57d60e1bac 100644
> > --- a/gcc/value-range.cc
> > +++ b/gcc/value-range.cc
> > @@ -338,6 +338,18 @@ frange::set (tree min, tree max, value_range_kind kind)
> >        m_neg_nan = false;
> >      }
> >
> > +  // For -ffinite-math-only we can drop ranges outside the
> > +  // representable numbers to min/max for the type.
> > +  if (flag_finite_math_only)
> > +    {
> > +      REAL_VALUE_TYPE min_repr = *TREE_REAL_CST_PTR (vrp_val_min (m_type));
> > +      REAL_VALUE_TYPE max_repr = *TREE_REAL_CST_PTR (vrp_val_max (m_type));
> > +      if (real_less (&m_min, &min_repr))
> > +       m_min = min_repr;
> > +      if (real_less (&max_repr, &m_max))
> > +       m_max = max_repr;
>
> I think you want to re-formulate that in terms of real_isinf() and
> change those to the max representable values.
>
> > +    }
> > +
> >    // Check for swapped ranges.
> >    gcc_checking_assert (tree_compare (LE_EXPR, min, max));
> >
> > @@ -371,8 +383,8 @@ bool
> >  frange::normalize_kind ()
> >  {
> >    if (m_kind == VR_RANGE
> > -      && real_isinf (&m_min, 1)
> > -      && real_isinf (&m_max, 0))
> > +      && vrp_val_is_min (build_real (m_type, m_min))
> > +      && vrp_val_is_max (build_real (m_type, m_max)))
>
> I think this suggests that keeping +-Inf might be the better choice.
> At least you want to change vrp_val_is_min/max to take a REAL_VALUE_TYPE
> to avoid building four tree nodes for just this compare?
>
> >      {
> >        if (m_pos_nan && m_neg_nan)
> >         {
> > @@ -385,8 +397,8 @@ frange::normalize_kind ()
> >        if (!m_pos_nan || !m_neg_nan)
> >         {
> >           m_kind = VR_RANGE;
> > -         m_min = dconstninf;
> > -         m_max = dconstinf;
> > +         m_min = *TREE_REAL_CST_PTR (vrp_val_min (m_type));
> > +         m_max = *TREE_REAL_CST_PTR (vrp_val_max (m_type));
> >           return true;
> >         }
> >      }
> > @@ -706,8 +718,8 @@ frange::verify_range ()
> >      case VR_VARYING:
> >        gcc_checking_assert (m_type);
> >        gcc_checking_assert (m_pos_nan && m_neg_nan);
> > -      gcc_checking_assert (real_isinf (&m_min, 1));
> > -      gcc_checking_assert (real_isinf (&m_max, 0));
> > +      gcc_checking_assert (vrp_val_is_min (build_real (m_type, m_min)));
> > +      gcc_checking_assert (vrp_val_is_max (build_real (m_type, m_max)));
> >        return;
> >      case VR_RANGE:
> >        gcc_checking_assert (m_type);
> > @@ -732,7 +744,8 @@ frange::verify_range ()
> >    // If all the properties are clear, we better not span the entire
> >    // domain, because that would make us varying.
> >    if (m_pos_nan && m_neg_nan)
> > -    gcc_checking_assert (!real_isinf (&m_min, 1) || !real_isinf (&m_max, 0));
> > +    gcc_checking_assert (!vrp_val_is_min (build_real (m_type, m_min))
> > +                        || !vrp_val_is_max (build_real (m_type, m_max)));
> >  }
> >
> >  // We can't do much with nonzeros yet.
> > @@ -779,7 +792,7 @@ frange::zero_p () const
> >  void
> >  frange::set_nonnegative (tree type)
> >  {
> > -  set (type, dconst0, dconstinf);
> > +  set (type, dconst0, *TREE_REAL_CST_PTR (vrp_val_max (type)));
> >
> >    // Set +NAN as the only possibility.
> >    if (HONOR_NANS (type))
> > @@ -3886,23 +3899,6 @@ range_tests_floats ()
> >    r0.clear_nan ();
> >    ASSERT_FALSE (r0.varying_p ());
> >
> > -  // The endpoints of a VARYING are +-INF.
> > -  r0.set_varying (float_type_node);
> > -  ASSERT_TRUE (real_identical (&r0.lower_bound (), &dconstninf));
> > -  ASSERT_TRUE (real_identical (&r0.upper_bound (), &dconstinf));
> > -
> > -  // The maximum representable range for a type is still a subset of VARYING.
> > -  REAL_VALUE_TYPE q, r;
> > -  real_min_representable (&q, float_type_node);
> > -  real_max_representable (&r, float_type_node);
> > -  r0 = frange (float_type_node, q, r);
> > -  // r0 is not a varying, because it does not include -INF/+INF.
> > -  ASSERT_FALSE (r0.varying_p ());
> > -  // The upper bound of r0 must be less than +INF.
> > -  ASSERT_TRUE (real_less (&r0.upper_bound (), &dconstinf));
> > -  // The lower bound of r0 must be greater than -INF.
> > -  ASSERT_TRUE (real_less (&dconstninf, &r0.lower_bound ()));
> > -
> >    // For most architectures, where float and double are different
> >    // sizes, having the same endpoints does not necessarily mean the
> >    // ranges are equal.
> > diff --git a/gcc/value-range.h b/gcc/value-range.h
> > index 3668b331187..74879b76b40 100644
> > --- a/gcc/value-range.h
> > +++ b/gcc/value-range.h
> > @@ -593,6 +593,8 @@ extern void dump_value_range (FILE *, const vrange *);
> >  extern bool vrp_val_is_min (const_tree);
> >  extern bool vrp_val_is_max (const_tree);
> >  extern bool vrp_operand_equal_p (const_tree, const_tree);
> > +inline void real_max_representable (REAL_VALUE_TYPE *r, const_tree type);
> > +inline void real_min_representable (REAL_VALUE_TYPE *r, const_tree type);
> >
> >  inline value_range_kind
> >  vrange::kind () const
> > @@ -1009,7 +1011,15 @@ vrp_val_max (const_tree type)
> >        return wide_int_to_tree (const_cast<tree> (type), max);
> >      }
> >    if (frange::supports_p (type))
> > -    return build_real (const_cast <tree> (type), dconstinf);
> > +    {
> > +      if (flag_finite_math_only)
> > +       {
> > +         REAL_VALUE_TYPE r;
> > +         real_max_representable (&r, type);
> > +         return build_real (const_cast <tree> (type), r);
> > +       }
> > +      return build_real (const_cast <tree> (type), dconstinf);
>
> Overall there's too many places affected IMHO.  The above also shows
> we're building trees a lot - note that REAL_CST are _not_ shared, so
> each time you call build_real a new REAL_CST tree node is allocated
> (and later GCed if no longer used).
>
> I thought vrp_val_min/max are legacy by now ...
>
> That said, using trees for the range representation might not be the
> very best thing to do, it's unnecessarily wrapping things.
>
> > +    }
> >    return NULL_TREE;
> >  }
> >
> > @@ -1023,7 +1033,15 @@ vrp_val_min (const_tree type)
> >    if (POINTER_TYPE_P (type))
> >      return build_zero_cst (const_cast<tree> (type));
> >    if (frange::supports_p (type))
> > -    return build_real (const_cast <tree> (type), dconstninf);
> > +    {
> > +      if (flag_finite_math_only)
> > +       {
> > +         REAL_VALUE_TYPE r;
> > +         real_min_representable (&r, type);
> > +         return build_real (const_cast <tree> (type), r);
> > +       }
> > +      return build_real (const_cast <tree> (type), dconstninf);
> > +    }
> >    return NULL_TREE;
> >  }
> >
> > @@ -1073,8 +1091,8 @@ frange::set_varying (tree type)
> >  {
> >    m_kind = VR_VARYING;
> >    m_type = type;
> > -  m_min = dconstninf;
> > -  m_max = dconstinf;
> > +  m_min = *TREE_REAL_CST_PTR (vrp_val_min (type));
> > +  m_max = *TREE_REAL_CST_PTR (vrp_val_max (type));
> >    m_pos_nan = true;
> >    m_neg_nan = true;
> >  }
> > @@ -1133,7 +1151,7 @@ frange::clear_nan ()
> >  // Set R to maximum representable value for TYPE.
> >
> >  inline void
> > -real_max_representable (REAL_VALUE_TYPE *r, tree type)
> > +real_max_representable (REAL_VALUE_TYPE *r, const_tree type)
> >  {
> >    char buf[128];
> >    get_max_float (REAL_MODE_FORMAT (TYPE_MODE (type)),
> > @@ -1145,7 +1163,7 @@ real_max_representable (REAL_VALUE_TYPE *r, tree type)
> >  // Set R to minimum representable value for TYPE.
> >
> >  inline void
> > -real_min_representable (REAL_VALUE_TYPE *r, tree type)
> > +real_min_representable (REAL_VALUE_TYPE *r, const_tree type)
> >  {
> >    real_max_representable (r, type);
> >    *r = real_value_negate (r);
> > --
> > 2.37.1
> >
>

[-- Attachment #2: 0003-frange-drop-endpoints-to-min-max-representable-numbe.patch --]
[-- Type: text/x-patch, Size: 10557 bytes --]

From ed143e0c8e08b9192439ff4b7892a91ffa9bfd12 Mon Sep 17 00:00:00 2001
From: Aldy Hernandez <aldyh@redhat.com>
Date: Thu, 22 Sep 2022 18:20:39 +0200
Subject: [PATCH] frange: drop endpoints to min/max representable numbers for
 -ffinite-math-only.

Similarly to how we drop NANs to UNDEFINED when -ffinite-math-only, I
think we can drop the numbers outside of the min/max representable
numbers to the representable number.

This means the endpoings to VR_VARYING for -ffinite-math-only can now
be the min/max representable, instead of -INF and +INF.

Saturating in the setter means that the upcoming implementation for
binary operators no longer have to worry about doing the right
thing for -ffinite-math-only.  If the range goes outside the limits,
it'll get chopped down.

Tested on x86-64 Linux.

gcc/ChangeLog:

	* range-op-float.cc (build_le): Use vrp_val_*.
	(build_lt): Same.
	(build_ge): Same.
	(build_gt): Same.
	* value-range.cc (frange::set): Chop ranges outside of the
	representable numbers for -ffinite-math-only.
	(frange::normalize_kind): Use vrp_val*.
	(frange::verify_range): Same.
	(frange::set_nonnegative): Same.
	(range_tests_floats): Remove tests that depend on -INF and +INF.
	* value-range.h (real_max_representable): Add prototype.
	(real_min_representable): Same.
	(vrp_val_max): Set max representable number for
	-ffinite-math-only.
	(vrp_val_min): Same but for min.
	(frange::set_varying): Use vrp_val*.
---
 gcc/range-op-float.cc | 18 ++++++-----
 gcc/value-range.cc    | 46 ++++++++++++--------------
 gcc/value-range.h     | 75 ++++++++++++++++++++++++++++++++++++-------
 3 files changed, 94 insertions(+), 45 deletions(-)

diff --git a/gcc/range-op-float.cc b/gcc/range-op-float.cc
index 2bd3dc9253f..84f1ebe5a61 100644
--- a/gcc/range-op-float.cc
+++ b/gcc/range-op-float.cc
@@ -190,8 +190,7 @@ frelop_early_resolve (irange &r, tree type,
 static inline void
 frange_drop_inf (frange &r, tree type)
 {
-  REAL_VALUE_TYPE max;
-  real_max_representable (&max, type);
+  REAL_VALUE_TYPE max = real_max_representable (type);
   frange tmp (type, r.lower_bound (), max);
   r.intersect (tmp);
 }
@@ -202,8 +201,7 @@ frange_drop_inf (frange &r, tree type)
 static inline void
 frange_drop_ninf (frange &r, tree type)
 {
-  REAL_VALUE_TYPE min;
-  real_min_representable (&min, type);
+  REAL_VALUE_TYPE min = real_min_representable (type);
   frange tmp (type, min, r.upper_bound ());
   r.intersect (tmp);
 }
@@ -232,7 +230,8 @@ build_le (frange &r, tree type, const frange &val)
 {
   gcc_checking_assert (!val.known_isnan ());
 
-  r.set (type, dconstninf, val.upper_bound ());
+  REAL_VALUE_TYPE ninf = frange_val_min (type);
+  r.set (type, ninf, val.upper_bound ());
 
   // Add both zeros if there's the possibility of zero equality.
   frange_add_zeros (r, type);
@@ -257,7 +256,8 @@ build_lt (frange &r, tree type, const frange &val)
       return false;
     }
   // We only support closed intervals.
-  r.set (type, dconstninf, val.upper_bound ());
+  REAL_VALUE_TYPE ninf = frange_val_min (type);
+  r.set (type, ninf, val.upper_bound ());
   return true;
 }
 
@@ -268,7 +268,8 @@ build_ge (frange &r, tree type, const frange &val)
 {
   gcc_checking_assert (!val.known_isnan ());
 
-  r.set (type, val.lower_bound (), dconstinf);
+  REAL_VALUE_TYPE inf = frange_val_max (type);
+  r.set (type, val.lower_bound (), inf);
 
   // Add both zeros if there's the possibility of zero equality.
   frange_add_zeros (r, type);
@@ -294,7 +295,8 @@ build_gt (frange &r, tree type, const frange &val)
     }
 
   // We only support closed intervals.
-  r.set (type, val.lower_bound (), dconstinf);
+  REAL_VALUE_TYPE inf = frange_val_max (type);
+  r.set (type, val.lower_bound (), inf);
   return true;
 }
 
diff --git a/gcc/value-range.cc b/gcc/value-range.cc
index 7e8028eced2..43905ba4901 100644
--- a/gcc/value-range.cc
+++ b/gcc/value-range.cc
@@ -338,6 +338,18 @@ frange::set (tree min, tree max, value_range_kind kind)
       m_neg_nan = false;
     }
 
+  // For -ffinite-math-only we can drop ranges outside the
+  // representable numbers to min/max for the type.
+  if (flag_finite_math_only)
+    {
+      REAL_VALUE_TYPE min_repr = frange_val_min (m_type);
+      REAL_VALUE_TYPE max_repr = frange_val_max (m_type);
+      if (real_less (&m_min, &min_repr))
+	m_min = min_repr;
+      if (real_less (&max_repr, &m_max))
+	m_max = max_repr;
+    }
+
   // Check for swapped ranges.
   gcc_checking_assert (tree_compare (LE_EXPR, min, max));
 
@@ -371,8 +383,8 @@ bool
 frange::normalize_kind ()
 {
   if (m_kind == VR_RANGE
-      && real_isinf (&m_min, 1)
-      && real_isinf (&m_max, 0))
+      && frange_val_is_min (m_min, m_type)
+      && frange_val_is_max (m_max, m_type))
     {
       if (m_pos_nan && m_neg_nan)
 	{
@@ -385,8 +397,8 @@ frange::normalize_kind ()
       if (!m_pos_nan || !m_neg_nan)
 	{
 	  m_kind = VR_RANGE;
-	  m_min = dconstninf;
-	  m_max = dconstinf;
+	  m_min = frange_val_min (m_type);
+	  m_max = frange_val_max (m_type);
 	  return true;
 	}
     }
@@ -706,8 +718,8 @@ frange::verify_range ()
     case VR_VARYING:
       gcc_checking_assert (m_type);
       gcc_checking_assert (m_pos_nan && m_neg_nan);
-      gcc_checking_assert (real_isinf (&m_min, 1));
-      gcc_checking_assert (real_isinf (&m_max, 0));
+      gcc_checking_assert (frange_val_is_min (m_min, m_type));
+      gcc_checking_assert (frange_val_is_max (m_max, m_type));
       return;
     case VR_RANGE:
       gcc_checking_assert (m_type);
@@ -732,7 +744,8 @@ frange::verify_range ()
   // If all the properties are clear, we better not span the entire
   // domain, because that would make us varying.
   if (m_pos_nan && m_neg_nan)
-    gcc_checking_assert (!real_isinf (&m_min, 1) || !real_isinf (&m_max, 0));
+    gcc_checking_assert (!frange_val_is_min (m_min, m_type)
+			 || !frange_val_is_max (m_max, m_type));
 }
 
 // We can't do much with nonzeros yet.
@@ -779,7 +792,7 @@ frange::zero_p () const
 void
 frange::set_nonnegative (tree type)
 {
-  set (type, dconst0, dconstinf);
+  set (type, dconst0, frange_val_max (type));
 
   // Set +NAN as the only possibility.
   if (HONOR_NANS (type))
@@ -3886,23 +3899,6 @@ range_tests_floats ()
   r0.clear_nan ();
   ASSERT_FALSE (r0.varying_p ());
 
-  // The endpoints of a VARYING are +-INF.
-  r0.set_varying (float_type_node);
-  ASSERT_TRUE (real_identical (&r0.lower_bound (), &dconstninf));
-  ASSERT_TRUE (real_identical (&r0.upper_bound (), &dconstinf));
-
-  // The maximum representable range for a type is still a subset of VARYING.
-  REAL_VALUE_TYPE q, r;
-  real_min_representable (&q, float_type_node);
-  real_max_representable (&r, float_type_node);
-  r0 = frange (float_type_node, q, r);
-  // r0 is not a varying, because it does not include -INF/+INF.
-  ASSERT_FALSE (r0.varying_p ());
-  // The upper bound of r0 must be less than +INF.
-  ASSERT_TRUE (real_less (&r0.upper_bound (), &dconstinf));
-  // The lower bound of r0 must be greater than -INF.
-  ASSERT_TRUE (real_less (&dconstninf, &r0.lower_bound ()));
-
   // For most architectures, where float and double are different
   // sizes, having the same endpoints does not necessarily mean the
   // ranges are equal.
diff --git a/gcc/value-range.h b/gcc/value-range.h
index 3668b331187..413e54bda6f 100644
--- a/gcc/value-range.h
+++ b/gcc/value-range.h
@@ -593,6 +593,8 @@ extern void dump_value_range (FILE *, const vrange *);
 extern bool vrp_val_is_min (const_tree);
 extern bool vrp_val_is_max (const_tree);
 extern bool vrp_operand_equal_p (const_tree, const_tree);
+inline REAL_VALUE_TYPE frange_val_min (const_tree type);
+inline REAL_VALUE_TYPE frange_val_max (const_tree type);
 
 inline value_range_kind
 vrange::kind () const
@@ -1009,7 +1011,10 @@ vrp_val_max (const_tree type)
       return wide_int_to_tree (const_cast<tree> (type), max);
     }
   if (frange::supports_p (type))
-    return build_real (const_cast <tree> (type), dconstinf);
+    {
+      REAL_VALUE_TYPE r = frange_val_max (type);
+      return build_real (const_cast <tree> (type), r);
+    }
   return NULL_TREE;
 }
 
@@ -1023,7 +1028,10 @@ vrp_val_min (const_tree type)
   if (POINTER_TYPE_P (type))
     return build_zero_cst (const_cast<tree> (type));
   if (frange::supports_p (type))
-    return build_real (const_cast <tree> (type), dconstninf);
+    {
+      REAL_VALUE_TYPE r = frange_val_min (type);
+      return build_real (const_cast <tree> (type), r);
+    }
   return NULL_TREE;
 }
 
@@ -1073,8 +1081,8 @@ frange::set_varying (tree type)
 {
   m_kind = VR_VARYING;
   m_type = type;
-  m_min = dconstninf;
-  m_max = dconstinf;
+  m_min = frange_val_min (type);
+  m_max = frange_val_max (type);
   m_pos_nan = true;
   m_neg_nan = true;
 }
@@ -1132,23 +1140,66 @@ frange::clear_nan ()
 
 // Set R to maximum representable value for TYPE.
 
-inline void
-real_max_representable (REAL_VALUE_TYPE *r, tree type)
+inline REAL_VALUE_TYPE
+real_max_representable (const_tree type)
 {
+  REAL_VALUE_TYPE r;
   char buf[128];
   get_max_float (REAL_MODE_FORMAT (TYPE_MODE (type)),
 		 buf, sizeof (buf), false);
-  int res = real_from_string (r, buf);
+  int res = real_from_string (&r, buf);
   gcc_checking_assert (!res);
+  return r;
 }
 
-// Set R to minimum representable value for TYPE.
+// Return the minimum representable value for TYPE.
 
-inline void
-real_min_representable (REAL_VALUE_TYPE *r, tree type)
+inline REAL_VALUE_TYPE
+real_min_representable (const_tree type)
+{
+  REAL_VALUE_TYPE r = real_max_representable (type);
+  r = real_value_negate (&r);
+  return r;
+}
+
+// Return the minimum value for TYPE.
+
+inline REAL_VALUE_TYPE
+frange_val_min (const_tree type)
+{
+  if (flag_finite_math_only)
+    return real_min_representable (type);
+  else
+    return dconstninf;
+}
+
+// Return the maximum value for TYPE.
+
+inline REAL_VALUE_TYPE
+frange_val_max (const_tree type)
+{
+  if (flag_finite_math_only)
+    return real_max_representable (type);
+  else
+    return dconstinf;
+}
+
+// Return TRUE if R is the minimum value for TYPE.
+
+inline bool
+frange_val_is_min (const REAL_VALUE_TYPE &r, const_tree type)
+{
+  REAL_VALUE_TYPE min = frange_val_min (type);
+  return real_identical (&min, &r);
+}
+
+// Return TRUE if R is the max value for TYPE.
+
+inline bool
+frange_val_is_max (const REAL_VALUE_TYPE &r, const_tree type)
 {
-  real_max_representable (r, type);
-  *r = real_value_negate (r);
+  REAL_VALUE_TYPE max = frange_val_max (type);
+  return real_identical (&max, &r);
 }
 
 // Build a signless NAN of type TYPE.
-- 
2.37.1

Re: [PATCH] frange: drop endpoints to min/max representable numbers for -ffinite-math-only.

[Richard Biener]

On Fri, Sep 23, 2022 at 9:21 AM Aldy Hernandez <aldyh@redhat.com> wrote:
>
> Ughhh, my bad.  I had reworked this as soon as Jakub said we couldn't
> cache the min/max in TYPE_MIN/MAX_VALUE, but forgot to send it.  And
> yes...the incessant wrapping was very annoying.  It's all fixed.
>
> Let me know what you think.

Much better.

Thanks,
Richard.

> Aldy
>
> On Fri, Sep 23, 2022 at 9:04 AM Richard Biener
> <richard.guenther@gmail.com> wrote:
> >
> > On Thu, Sep 22, 2022 at 6:49 PM Aldy Hernandez <aldyh@redhat.com> wrote:
> > >
> > > Similarly to how we drop NANs to UNDEFINED when -ffinite-math-only, I
> > > think we can drop the numbers outside of the min/max representable
> > > numbers to the representable number.
> > >
> > > This means the endpoings to VR_VARYING for -ffinite-math-only can now
> > > be the min/max representable, instead of -INF and +INF.
> > >
> > > Saturating in the setter means that the upcoming implementation for
> > > binary operators no longer have to worry about doing the right
> > > thing for -ffinite-math-only.  If the range goes outside the limits,
> > > it'll get chopped down.
> > >
> > > How does this look?
> > >
> > > Tested on x86-64 Linux.
> > >
> > > gcc/ChangeLog:
> > >
> > >         * range-op-float.cc (build_le): Use vrp_val_*.
> > >         (build_lt): Same.
> > >         (build_ge): Same.
> > >         (build_gt): Same.
> > >         * value-range.cc (frange::set): Chop ranges outside of the
> > >         representable numbers for -ffinite-math-only.
> > >         (frange::normalize_kind): Use vrp_val*.
> > >         (frange::verify_range): Same.
> > >         (frange::set_nonnegative): Same.
> > >         (range_tests_floats): Remove tests that depend on -INF and +INF.
> > >         * value-range.h (real_max_representable): Add prototype.
> > >         (real_min_representable): Same.
> > >         (vrp_val_max): Set max representable number for
> > >         -ffinite-math-only.
> > >         (vrp_val_min): Same but for min.
> > >         (frange::set_varying): Use vrp_val*.
> > > ---
> > >  gcc/range-op-float.cc | 12 +++++++----
> > >  gcc/value-range.cc    | 46 ++++++++++++++++++++-----------------------
> > >  gcc/value-range.h     | 30 ++++++++++++++++++++++------
> > >  3 files changed, 53 insertions(+), 35 deletions(-)
> > >
> > > diff --git a/gcc/range-op-float.cc b/gcc/range-op-float.cc
> > > index 2bd3dc9253f..15ba19c2deb 100644
> > > --- a/gcc/range-op-float.cc
> > > +++ b/gcc/range-op-float.cc
> > > @@ -232,7 +232,8 @@ build_le (frange &r, tree type, const frange &val)
> > >  {
> > >    gcc_checking_assert (!val.known_isnan ());
> > >
> > > -  r.set (type, dconstninf, val.upper_bound ());
> > > +  REAL_VALUE_TYPE ninf = *TREE_REAL_CST_PTR (vrp_val_min (type));
> > > +  r.set (type, ninf, val.upper_bound ());
> > >
> > >    // Add both zeros if there's the possibility of zero equality.
> > >    frange_add_zeros (r, type);
> > > @@ -257,7 +258,8 @@ build_lt (frange &r, tree type, const frange &val)
> > >        return false;
> > >      }
> > >    // We only support closed intervals.
> > > -  r.set (type, dconstninf, val.upper_bound ());
> > > +  REAL_VALUE_TYPE ninf = *TREE_REAL_CST_PTR (vrp_val_min (type));
> > > +  r.set (type, ninf, val.upper_bound ());
> > >    return true;
> > >  }
> > >
> > > @@ -268,7 +270,8 @@ build_ge (frange &r, tree type, const frange &val)
> > >  {
> > >    gcc_checking_assert (!val.known_isnan ());
> > >
> > > -  r.set (type, val.lower_bound (), dconstinf);
> > > +  REAL_VALUE_TYPE inf = *TREE_REAL_CST_PTR (vrp_val_max (type));
> > > +  r.set (type, val.lower_bound (), inf);
> > >
> > >    // Add both zeros if there's the possibility of zero equality.
> > >    frange_add_zeros (r, type);
> > > @@ -294,7 +297,8 @@ build_gt (frange &r, tree type, const frange &val)
> > >      }
> > >
> > >    // We only support closed intervals.
> > > -  r.set (type, val.lower_bound (), dconstinf);
> > > +  REAL_VALUE_TYPE inf = *TREE_REAL_CST_PTR (vrp_val_max (type));
> > > +  r.set (type, val.lower_bound (), inf);
> > >    return true;
> > >  }
> > >
> > > diff --git a/gcc/value-range.cc b/gcc/value-range.cc
> > > index 7e8028eced2..e57d60e1bac 100644
> > > --- a/gcc/value-range.cc
> > > +++ b/gcc/value-range.cc
> > > @@ -338,6 +338,18 @@ frange::set (tree min, tree max, value_range_kind kind)
> > >        m_neg_nan = false;
> > >      }
> > >
> > > +  // For -ffinite-math-only we can drop ranges outside the
> > > +  // representable numbers to min/max for the type.
> > > +  if (flag_finite_math_only)
> > > +    {
> > > +      REAL_VALUE_TYPE min_repr = *TREE_REAL_CST_PTR (vrp_val_min (m_type));
> > > +      REAL_VALUE_TYPE max_repr = *TREE_REAL_CST_PTR (vrp_val_max (m_type));
> > > +      if (real_less (&m_min, &min_repr))
> > > +       m_min = min_repr;
> > > +      if (real_less (&max_repr, &m_max))
> > > +       m_max = max_repr;
> >
> > I think you want to re-formulate that in terms of real_isinf() and
> > change those to the max representable values.
> >
> > > +    }
> > > +
> > >    // Check for swapped ranges.
> > >    gcc_checking_assert (tree_compare (LE_EXPR, min, max));
> > >
> > > @@ -371,8 +383,8 @@ bool
> > >  frange::normalize_kind ()
> > >  {
> > >    if (m_kind == VR_RANGE
> > > -      && real_isinf (&m_min, 1)
> > > -      && real_isinf (&m_max, 0))
> > > +      && vrp_val_is_min (build_real (m_type, m_min))
> > > +      && vrp_val_is_max (build_real (m_type, m_max)))
> >
> > I think this suggests that keeping +-Inf might be the better choice.
> > At least you want to change vrp_val_is_min/max to take a REAL_VALUE_TYPE
> > to avoid building four tree nodes for just this compare?
> >
> > >      {
> > >        if (m_pos_nan && m_neg_nan)
> > >         {
> > > @@ -385,8 +397,8 @@ frange::normalize_kind ()
> > >        if (!m_pos_nan || !m_neg_nan)
> > >         {
> > >           m_kind = VR_RANGE;
> > > -         m_min = dconstninf;
> > > -         m_max = dconstinf;
> > > +         m_min = *TREE_REAL_CST_PTR (vrp_val_min (m_type));
> > > +         m_max = *TREE_REAL_CST_PTR (vrp_val_max (m_type));
> > >           return true;
> > >         }
> > >      }
> > > @@ -706,8 +718,8 @@ frange::verify_range ()
> > >      case VR_VARYING:
> > >        gcc_checking_assert (m_type);
> > >        gcc_checking_assert (m_pos_nan && m_neg_nan);
> > > -      gcc_checking_assert (real_isinf (&m_min, 1));
> > > -      gcc_checking_assert (real_isinf (&m_max, 0));
> > > +      gcc_checking_assert (vrp_val_is_min (build_real (m_type, m_min)));
> > > +      gcc_checking_assert (vrp_val_is_max (build_real (m_type, m_max)));
> > >        return;
> > >      case VR_RANGE:
> > >        gcc_checking_assert (m_type);
> > > @@ -732,7 +744,8 @@ frange::verify_range ()
> > >    // If all the properties are clear, we better not span the entire
> > >    // domain, because that would make us varying.
> > >    if (m_pos_nan && m_neg_nan)
> > > -    gcc_checking_assert (!real_isinf (&m_min, 1) || !real_isinf (&m_max, 0));
> > > +    gcc_checking_assert (!vrp_val_is_min (build_real (m_type, m_min))
> > > +                        || !vrp_val_is_max (build_real (m_type, m_max)));
> > >  }
> > >
> > >  // We can't do much with nonzeros yet.
> > > @@ -779,7 +792,7 @@ frange::zero_p () const
> > >  void
> > >  frange::set_nonnegative (tree type)
> > >  {
> > > -  set (type, dconst0, dconstinf);
> > > +  set (type, dconst0, *TREE_REAL_CST_PTR (vrp_val_max (type)));
> > >
> > >    // Set +NAN as the only possibility.
> > >    if (HONOR_NANS (type))
> > > @@ -3886,23 +3899,6 @@ range_tests_floats ()
> > >    r0.clear_nan ();
> > >    ASSERT_FALSE (r0.varying_p ());
> > >
> > > -  // The endpoints of a VARYING are +-INF.
> > > -  r0.set_varying (float_type_node);
> > > -  ASSERT_TRUE (real_identical (&r0.lower_bound (), &dconstninf));
> > > -  ASSERT_TRUE (real_identical (&r0.upper_bound (), &dconstinf));
> > > -
> > > -  // The maximum representable range for a type is still a subset of VARYING.
> > > -  REAL_VALUE_TYPE q, r;
> > > -  real_min_representable (&q, float_type_node);
> > > -  real_max_representable (&r, float_type_node);
> > > -  r0 = frange (float_type_node, q, r);
> > > -  // r0 is not a varying, because it does not include -INF/+INF.
> > > -  ASSERT_FALSE (r0.varying_p ());
> > > -  // The upper bound of r0 must be less than +INF.
> > > -  ASSERT_TRUE (real_less (&r0.upper_bound (), &dconstinf));
> > > -  // The lower bound of r0 must be greater than -INF.
> > > -  ASSERT_TRUE (real_less (&dconstninf, &r0.lower_bound ()));
> > > -
> > >    // For most architectures, where float and double are different
> > >    // sizes, having the same endpoints does not necessarily mean the
> > >    // ranges are equal.
> > > diff --git a/gcc/value-range.h b/gcc/value-range.h
> > > index 3668b331187..74879b76b40 100644
> > > --- a/gcc/value-range.h
> > > +++ b/gcc/value-range.h
> > > @@ -593,6 +593,8 @@ extern void dump_value_range (FILE *, const vrange *);
> > >  extern bool vrp_val_is_min (const_tree);
> > >  extern bool vrp_val_is_max (const_tree);
> > >  extern bool vrp_operand_equal_p (const_tree, const_tree);
> > > +inline void real_max_representable (REAL_VALUE_TYPE *r, const_tree type);
> > > +inline void real_min_representable (REAL_VALUE_TYPE *r, const_tree type);
> > >
> > >  inline value_range_kind
> > >  vrange::kind () const
> > > @@ -1009,7 +1011,15 @@ vrp_val_max (const_tree type)
> > >        return wide_int_to_tree (const_cast<tree> (type), max);
> > >      }
> > >    if (frange::supports_p (type))
> > > -    return build_real (const_cast <tree> (type), dconstinf);
> > > +    {
> > > +      if (flag_finite_math_only)
> > > +       {
> > > +         REAL_VALUE_TYPE r;
> > > +         real_max_representable (&r, type);
> > > +         return build_real (const_cast <tree> (type), r);
> > > +       }
> > > +      return build_real (const_cast <tree> (type), dconstinf);
> >
> > Overall there's too many places affected IMHO.  The above also shows
> > we're building trees a lot - note that REAL_CST are _not_ shared, so
> > each time you call build_real a new REAL_CST tree node is allocated
> > (and later GCed if no longer used).
> >
> > I thought vrp_val_min/max are legacy by now ...
> >
> > That said, using trees for the range representation might not be the
> > very best thing to do, it's unnecessarily wrapping things.
> >
> > > +    }
> > >    return NULL_TREE;
> > >  }
> > >
> > > @@ -1023,7 +1033,15 @@ vrp_val_min (const_tree type)
> > >    if (POINTER_TYPE_P (type))
> > >      return build_zero_cst (const_cast<tree> (type));
> > >    if (frange::supports_p (type))
> > > -    return build_real (const_cast <tree> (type), dconstninf);
> > > +    {
> > > +      if (flag_finite_math_only)
> > > +       {
> > > +         REAL_VALUE_TYPE r;
> > > +         real_min_representable (&r, type);
> > > +         return build_real (const_cast <tree> (type), r);
> > > +       }
> > > +      return build_real (const_cast <tree> (type), dconstninf);
> > > +    }
> > >    return NULL_TREE;
> > >  }
> > >
> > > @@ -1073,8 +1091,8 @@ frange::set_varying (tree type)
> > >  {
> > >    m_kind = VR_VARYING;
> > >    m_type = type;
> > > -  m_min = dconstninf;
> > > -  m_max = dconstinf;
> > > +  m_min = *TREE_REAL_CST_PTR (vrp_val_min (type));
> > > +  m_max = *TREE_REAL_CST_PTR (vrp_val_max (type));
> > >    m_pos_nan = true;
> > >    m_neg_nan = true;
> > >  }
> > > @@ -1133,7 +1151,7 @@ frange::clear_nan ()
> > >  // Set R to maximum representable value for TYPE.
> > >
> > >  inline void
> > > -real_max_representable (REAL_VALUE_TYPE *r, tree type)
> > > +real_max_representable (REAL_VALUE_TYPE *r, const_tree type)
> > >  {
> > >    char buf[128];
> > >    get_max_float (REAL_MODE_FORMAT (TYPE_MODE (type)),
> > > @@ -1145,7 +1163,7 @@ real_max_representable (REAL_VALUE_TYPE *r, tree type)
> > >  // Set R to minimum representable value for TYPE.
> > >
> > >  inline void
> > > -real_min_representable (REAL_VALUE_TYPE *r, tree type)
> > > +real_min_representable (REAL_VALUE_TYPE *r, const_tree type)
> > >  {
> > >    real_max_representable (r, type);
> > >    *r = real_value_negate (r);
> > > --
> > > 2.37.1
> > >
> >

Re: [PATCH] frange: dump hex values when dumping FP numbers.

[Toon Moene]

If it's not too cumbersome, I suggest dumping both.

In my neck-of-the-woods (meteorology) I have seen this done just to 
ensure that algorithms that are supposed to be bit-reproducable actually 
are - and that it can be checked visually.

Kind regards,
Toon.

On 9/22/22 18:49, Aldy Hernandez via Gcc-patches wrote:

> It has been suggested that if we start bumping numbers by an ULP when
> calculating open ranges (for example the numbers less than 3.0) that
> dumping these will become increasingly harder to read, and instead we
> should opt for the hex representation.  I still find the floating
> point representation easier to read for most numbers, but perhaps we
> could have both?
> 
> With this patch this is the representation for [15.0, 20.0]:
> 
>       [frange] float [1.5e+1 (0x0.fp+4), 2.0e+1 (0x0.ap+5)]
> 
> Would you find this useful, or should we stick to the hex
> representation only (or something altogether different)?
> 
> Tested on x86-64 Linux.
> 
> gcc/ChangeLog:
> 
> 	* value-range-pretty-print.cc (vrange_printer::print_real_value): New.
> 	(vrange_printer::visit): Call print_real_value.
> 	* value-range-pretty-print.h: New print_real_value.
> ---
>   gcc/value-range-pretty-print.cc | 16 ++++++++++++----
>   gcc/value-range-pretty-print.h  |  1 +
>   2 files changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/gcc/value-range-pretty-print.cc b/gcc/value-range-pretty-print.cc
> index eb7442229ba..51be037c254 100644
> --- a/gcc/value-range-pretty-print.cc
> +++ b/gcc/value-range-pretty-print.cc
> @@ -117,6 +117,16 @@ vrange_printer::print_irange_bitmasks (const irange &r) const
>     pp_string (pp, buf);
>   }
>   
> +void
> +vrange_printer::print_real_value (tree type, const REAL_VALUE_TYPE &r) const
> +{
> +  char s[60];
> +  tree t = build_real (type, r);
> +  dump_generic_node (pp, t, 0, TDF_NONE, false);
> +  real_to_hexadecimal (s, &r, sizeof (s), 0, 1);
> +  pp_printf (pp, " (%s)", s);
> +}
> +
>   // Print an frange.
>   
>   void
> @@ -141,11 +151,9 @@ vrange_printer::visit (const frange &r) const
>     bool has_endpoints = !r.known_isnan ();
>     if (has_endpoints)
>       {
> -      dump_generic_node (pp,
> -			 build_real (type, r.lower_bound ()), 0, TDF_NONE, false);
> +      print_real_value (type, r.lower_bound ());
>         pp_string (pp, ", ");
> -      dump_generic_node (pp,
> -			 build_real (type, r.upper_bound ()), 0, TDF_NONE, false);
> +      print_real_value (type, r.upper_bound ());
>       }
>     pp_character (pp, ']');
>     print_frange_nan (r);
> diff --git a/gcc/value-range-pretty-print.h b/gcc/value-range-pretty-print.h
> index 20c26598fe7..a9ae5a7b4cc 100644
> --- a/gcc/value-range-pretty-print.h
> +++ b/gcc/value-range-pretty-print.h
> @@ -32,6 +32,7 @@ private:
>     void print_irange_bound (const wide_int &w, tree type) const;
>     void print_irange_bitmasks (const irange &) const;
>     void print_frange_nan (const frange &) const;
> +  void print_real_value (tree type, const REAL_VALUE_TYPE &r) const;
>   
>     pretty_printer *pp;
>   };

-- 
Toon Moene - e-mail: toon@moene.org - phone: +31 346 214290
Saturnushof 14, 3738 XG  Maartensdijk, The Netherlands

Re: [PATCH] frange: dump hex values when dumping FP numbers.

[Jeff Law]

On 9/22/22 10:49, Aldy Hernandez via Gcc-patches wrote:
> It has been suggested that if we start bumping numbers by an ULP when
> calculating open ranges (for example the numbers less than 3.0) that
> dumping these will become increasingly harder to read, and instead we
> should opt for the hex representation.  I still find the floating
> point representation easier to read for most numbers, but perhaps we
> could have both?
>
> With this patch this is the representation for [15.0, 20.0]:
>
>       [frange] float [1.5e+1 (0x0.fp+4), 2.0e+1 (0x0.ap+5)]
>
> Would you find this useful, or should we stick to the hex
> representation only (or something altogether different)?
>
> Tested on x86-64 Linux.
>
> gcc/ChangeLog:
>
> 	* value-range-pretty-print.cc (vrange_printer::print_real_value): New.
> 	(vrange_printer::visit): Call print_real_value.
> 	* value-range-pretty-print.h: New print_real_value.

The big advantage of the hex representation is you can feed that back 
into the compiler trivially and be confident the bit pattern hasn't 
changed.   I've found it invaluable when doing deep FP analysis.


jeff

Re: [PATCH] frange: dump hex values when dumping FP numbers.

[Jakub Jelinek]

On Thu, Sep 22, 2022 at 06:49:10PM +0200, Aldy Hernandez wrote:
> It has been suggested that if we start bumping numbers by an ULP when
> calculating open ranges (for example the numbers less than 3.0) that
> dumping these will become increasingly harder to read, and instead we
> should opt for the hex representation.  I still find the floating
> point representation easier to read for most numbers, but perhaps we
> could have both?
> 
> With this patch this is the representation for [15.0, 20.0]:
> 
>      [frange] float [1.5e+1 (0x0.fp+4), 2.0e+1 (0x0.ap+5)]
> 
> Would you find this useful, or should we stick to the hex
> representation only (or something altogether different)?

I think dumping both is the way to go, but real_to_hexadecimal doesn't
do anything useful with decimal floats, so that part should be
guarded on !DECIMAL_FLOAT_TYPE_P (type).

Why do you build a tree + dump_generic_node for decimal instead of
real_to_decimal_for_mode ?
The former I think calls:
            char string[100];
            real_to_decimal (string, &d, sizeof (string), 0, 1);
so perhaps:
  char s[100];
  real_to_decimal_for_mode (s, &r, sizeof (string), 0, 1, TYPE_MODE (type));
  pp_string (pp, "%s", s);
  if (!DECIMAL_FLOAT_TYPE_P (type))
    {
      real_to_hexadecimal (s, &r, sizeof (s), 0, 1);
      pp_printf (pp, " (%s)", s);
    }
?

	Jakub

Re: [PATCH] frange: dump hex values when dumping FP numbers.

[Aldy Hernandez]

[-- Attachment #1: Type: text/plain, Size: 1873 bytes --]

On Thu, Sep 22, 2022 at 11:04 PM Jakub Jelinek <jakub@redhat.com> wrote:
>
> On Thu, Sep 22, 2022 at 06:49:10PM +0200, Aldy Hernandez wrote:
> > It has been suggested that if we start bumping numbers by an ULP when
> > calculating open ranges (for example the numbers less than 3.0) that
> > dumping these will become increasingly harder to read, and instead we
> > should opt for the hex representation.  I still find the floating
> > point representation easier to read for most numbers, but perhaps we
> > could have both?
> >
> > With this patch this is the representation for [15.0, 20.0]:
> >
> >      [frange] float [1.5e+1 (0x0.fp+4), 2.0e+1 (0x0.ap+5)]
> >
> > Would you find this useful, or should we stick to the hex
> > representation only (or something altogether different)?
>
> I think dumping both is the way to go, but real_to_hexadecimal doesn't
> do anything useful with decimal floats, so that part should be
> guarded on !DECIMAL_FLOAT_TYPE_P (type).

Remember that decimal floats are not supported for frange, but I
suppose it's good form to guard the print for when we do.

>
> Why do you build a tree + dump_generic_node for decimal instead of
> real_to_decimal_for_mode ?

Honestly, cause I was too lazy.  That code is inherited from irange
and I figured it's just a dumping routine, but I'm happy to fix it.

> The former I think calls:
>             char string[100];
>             real_to_decimal (string, &d, sizeof (string), 0, 1);
> so perhaps:
>   char s[100];
>   real_to_decimal_for_mode (s, &r, sizeof (string), 0, 1, TYPE_MODE (type));
>   pp_string (pp, "%s", s);
>   if (!DECIMAL_FLOAT_TYPE_P (type))
>     {
>       real_to_hexadecimal (s, &r, sizeof (s), 0, 1);
>       pp_printf (pp, " (%s)", s);
>     }
> ?

Thanks.

I'm retesting the following and will commit if it succeeds since we
seem to have overwhelming consensus :).
Aldy

[-- Attachment #2: 0001-frange-dump-hex-values-when-dumping-FP-numbers.patch --]
[-- Type: text/x-patch, Size: 2834 bytes --]

From 2f052904412bbe5821ee310067ad76b52980d8e3 Mon Sep 17 00:00:00 2001
From: Aldy Hernandez <aldyh@redhat.com>
Date: Thu, 22 Sep 2022 18:07:03 +0200
Subject: [PATCH] frange: dump hex values when dumping FP numbers.

It has been suggested that if we start bumping numbers by an ULP when
calculating open ranges (for example the numbers less than 3.0) that
dumping these will become increasingly harder to read, and instead we
should opt for the hex representation.  I still find the floating
point representation easier to read for most numbers, but perhaps we
could have both?

With this patch this is the representation for [15.0, 20.0]:

     [frange] float [1.5e+1 (0x0.fp+4), 2.0e+1 (0x0.ap+5)]

Would you find this useful, or should we stick to the hex
representation only?

Tested on x86-64 Linux.

gcc/ChangeLog:

	* value-range-pretty-print.cc (vrange_printer::print_real_value): New.
	(vrange_printer::visit): Call print_real_value.
	* value-range-pretty-print.h: New print_real_value.
---
 gcc/value-range-pretty-print.cc | 19 +++++++++++++++----
 gcc/value-range-pretty-print.h  |  1 +
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/gcc/value-range-pretty-print.cc b/gcc/value-range-pretty-print.cc
index eb7442229ba..8cbe97b76fd 100644
--- a/gcc/value-range-pretty-print.cc
+++ b/gcc/value-range-pretty-print.cc
@@ -117,6 +117,19 @@ vrange_printer::print_irange_bitmasks (const irange &r) const
   pp_string (pp, buf);
 }
 
+void
+vrange_printer::print_real_value (tree type, const REAL_VALUE_TYPE &r) const
+{
+  char s[100];
+  real_to_decimal_for_mode (s, &r, sizeof (s), 0, 1, TYPE_MODE (type));
+  pp_string (pp, s);
+  if (!DECIMAL_FLOAT_TYPE_P (type))
+    {
+      real_to_hexadecimal (s, &r, sizeof (s), 0, 1);
+      pp_printf (pp, " (%s)", s);
+    }
+}
+
 // Print an frange.
 
 void
@@ -141,11 +154,9 @@ vrange_printer::visit (const frange &r) const
   bool has_endpoints = !r.known_isnan ();
   if (has_endpoints)
     {
-      dump_generic_node (pp,
-			 build_real (type, r.lower_bound ()), 0, TDF_NONE, false);
+      print_real_value (type, r.lower_bound ());
       pp_string (pp, ", ");
-      dump_generic_node (pp,
-			 build_real (type, r.upper_bound ()), 0, TDF_NONE, false);
+      print_real_value (type, r.upper_bound ());
     }
   pp_character (pp, ']');
   print_frange_nan (r);
diff --git a/gcc/value-range-pretty-print.h b/gcc/value-range-pretty-print.h
index 20c26598fe7..a9ae5a7b4cc 100644
--- a/gcc/value-range-pretty-print.h
+++ b/gcc/value-range-pretty-print.h
@@ -32,6 +32,7 @@ private:
   void print_irange_bound (const wide_int &w, tree type) const;
   void print_irange_bitmasks (const irange &) const;
   void print_frange_nan (const frange &) const;
+  void print_real_value (tree type, const REAL_VALUE_TYPE &r) const;
 
   pretty_printer *pp;
 };
-- 
2.37.1