public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [COMMITTED] Minor fixes to frange.
@ 2022-09-14 15:08 Aldy Hernandez
  2022-09-14 15:08 ` [COMMITTED] Provide cleaner set_nan(), clear_nan(), and update_nan() methods Aldy Hernandez
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Aldy Hernandez @ 2022-09-14 15:08 UTC (permalink / raw)
  To: GCC patches; +Cc: Andrew MacLeod, Aldy Hernandez

Following are a series of cleanups to the frange code in preparation
for a much more invasive patch rewriting the NAN and sign tracking
bits.  Please be patient, as I'm trying to break everything up into
small chunks instead of dropping a mondo patch removing the NAN and
sign tristate handling.

No functional changes.

Regstrapped on x86-64 Linux, plus I ran selftests for
-ffinite-math-only.

gcc/ChangeLog:

	* value-query.cc (range_query::get_tree_range): Remove check for overflow.
	* value-range-pretty-print.cc (vrange_printer::visit): Move read
	of type until after undefined_p is checked.
	* value-range.cc (frange::set): Remove asserts for REAL_CST.
	(frange::contains_p): Tidy up.
	(range_tests_nan):  Add comment.
	* value-range.h (frange::type): Check for undefined_p.
	(frange::set_undefined): Remove set of endpoints.
---
 gcc/value-query.cc              |  3 ---
 gcc/value-range-pretty-print.cc |  3 +--
 gcc/value-range.cc              | 15 +++++----------
 gcc/value-range.h               |  3 +--
 4 files changed, 7 insertions(+), 17 deletions(-)

diff --git a/gcc/value-query.cc b/gcc/value-query.cc
index ad80db780c2..de83f469be4 100644
--- a/gcc/value-query.cc
+++ b/gcc/value-query.cc
@@ -217,9 +217,6 @@ range_query::get_tree_range (vrange &r, tree expr, gimple *stmt)
 
     case REAL_CST:
       {
-	if (TREE_OVERFLOW_P (expr))
-	  expr = drop_tree_overflow (expr);
-
 	frange &f = as_a <frange> (r);
 	f.set (expr, expr);
 
diff --git a/gcc/value-range-pretty-print.cc b/gcc/value-range-pretty-print.cc
index 93e18d3c1b6..b124e46cb9e 100644
--- a/gcc/value-range-pretty-print.cc
+++ b/gcc/value-range-pretty-print.cc
@@ -122,14 +122,13 @@ vrange_printer::print_irange_bitmasks (const irange &r) const
 void
 vrange_printer::visit (const frange &r) const
 {
-  tree type = r.type ();
-
   pp_string (pp, "[frange] ");
   if (r.undefined_p ())
     {
       pp_string (pp, "UNDEFINED");
       return;
     }
+  tree type = r.type ();
   dump_generic_node (pp, type, 0, TDF_NONE, false);
   pp_string (pp, " ");
   if (r.varying_p ())
diff --git a/gcc/value-range.cc b/gcc/value-range.cc
index 6f0609959b3..d40a4ebf657 100644
--- a/gcc/value-range.cc
+++ b/gcc/value-range.cc
@@ -357,15 +357,11 @@ frange::set_signbit (fp_prop::kind k)
 void
 frange::set (tree min, tree max, value_range_kind kind)
 {
-  gcc_checking_assert (TREE_CODE (min) == REAL_CST);
-  gcc_checking_assert (TREE_CODE (max) == REAL_CST);
-
   if (kind == VR_UNDEFINED)
     {
       set_undefined ();
       return;
     }
-
   // Treat VR_ANTI_RANGE and VR_VARYING as varying.
   if (kind != VR_RANGE)
     {
@@ -401,7 +397,6 @@ frange::set (tree min, tree max, value_range_kind kind)
   gcc_checking_assert (is_nan || tree_compare (LE_EXPR, min, max));
 
   normalize_kind ();
-
   if (flag_checking)
     verify_range ();
 }
@@ -612,17 +607,17 @@ frange::operator== (const frange &src) const
 bool
 frange::contains_p (tree cst) const
 {
+  gcc_checking_assert (m_kind != VR_ANTI_RANGE);
+  const REAL_VALUE_TYPE *rv = TREE_REAL_CST_PTR (cst);
+
   if (undefined_p ())
     return false;
 
   if (varying_p ())
     return true;
 
-  gcc_checking_assert (m_kind == VR_RANGE);
 
-  const REAL_VALUE_TYPE *rv = TREE_REAL_CST_PTR (cst);
-  if (real_compare (GE_EXPR, rv, &m_min)
-      && real_compare (LE_EXPR, rv, &m_max))
+  if (real_compare (GE_EXPR, rv, &m_min) && real_compare (LE_EXPR, rv, &m_max))
     {
       if (HONOR_SIGNED_ZEROS (m_type) && real_iszero (rv))
 	{
@@ -3652,7 +3647,7 @@ range_tests_nan ()
   ASSERT_FALSE (r0 == r0);
   ASSERT_TRUE (r0 != r0);
 
-  // [5,6] U NAN.
+  // [5,6] U NAN = [5,6] NAN.
   r0 = frange_float ("5", "6");
   r0.set_nan (fp_prop::NO);
   r1 = frange_nan (float_type_node);
diff --git a/gcc/value-range.h b/gcc/value-range.h
index f9a01ee7a05..0ba0193bc1f 100644
--- a/gcc/value-range.h
+++ b/gcc/value-range.h
@@ -1141,6 +1141,7 @@ frange::frange (tree min, tree max, value_range_kind kind)
 inline tree
 frange::type () const
 {
+  gcc_checking_assert (!undefined_p ());
   return m_type;
 }
 
@@ -1160,8 +1161,6 @@ frange::set_undefined ()
   m_kind = VR_UNDEFINED;
   m_type = NULL;
   m_props.set_undefined ();
-  memset (&m_min, 0, sizeof (m_min));
-  memset (&m_max, 0, sizeof (m_max));
 }
 
 // Set R to maximum representable value for TYPE.
-- 
2.37.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [COMMITTED] Provide cleaner set_nan(), clear_nan(), and update_nan() methods.
  2022-09-14 15:08 [COMMITTED] Minor fixes to frange Aldy Hernandez
@ 2022-09-14 15:08 ` Aldy Hernandez
  2022-09-14 15:08 ` [COMMITTED] Use frange::set_nan() from the generic frange::set() Aldy Hernandez
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Aldy Hernandez @ 2022-09-14 15:08 UTC (permalink / raw)
  To: GCC patches; +Cc: Andrew MacLeod, Aldy Hernandez

set_* has a very specific meaning for irange's and friends.  Methods
prefixed with set_* are setters clobbering the existing range.  As
such, the current set_nan() method is confusing in that it's not
actually setting a range to a NAN, but twiddling the NAN flags for an
existing frange.

This patch replaces set_nan() with an update_nan() to set the flag,
and clear_nan() to clear it.  This makes the code clearer, and though
the confusing tristate is still there, it will be removed in upcoming
patches.

Also, there is now an actual set_nan() method to set the range to a
NAN.  This replaces two out of class functions doing the same thing.
In future patches I will also add the ability to create a NAN with a
specific sign, but doing so now would be confusing because we're not
tracking NAN signs.

We should also submit set_signbit to the same fate, but it's about to
get removed.

No functional changes.

Regstrapped on x86-64 Linux, plus I ran selftests for
-ffinite-math-only.

gcc/ChangeLog:

	* range-op-float.cc (frange_set_nan): Remove.
	(build_lt): Use set_nan, update_nan, clear_nan.
	(build_gt): Same.
	(foperator_equal::op1_range): Same.
	(foperator_not_equal::op1_range): Same.
	(foperator_lt::op1_range): Same.
	(foperator_lt::op2_range): Same.
	(foperator_le::op1_range): Same.
	(foperator_le::op2_range): Same.
	(foperator_gt::op1_range): Same.
	(foperator_gt::op2_range): Same.
	(foperator_ge::op1_range): Same.
	(foperator_ge::op2_range): Same.
	(foperator_unordered::op1_range): Same.
	(foperator_ordered::op1_range): Same.
	* value-query.cc (range_query::get_tree_range): Same.
	* value-range.cc (frange::set_nan): Same.
	(frange::update_nan): Same.
	(frange::union_): Same.
	(frange::intersect): Same.
	(range_tests_nan): Same.
	(range_tests_signed_zeros): Same.
	(range_tests_signbit): Same.
	(range_tests_floats): Same.
	* value-range.h (class frange): Add update_nan and clear_nan.
	(frange::set_nan): New.
---
 gcc/range-op-float.cc | 46 ++++++++++++++------------------------
 gcc/value-query.cc    |  4 ++--
 gcc/value-range.cc    | 52 +++++++++++++++++++++----------------------
 gcc/value-range.h     | 11 ++++-----
 4 files changed, 51 insertions(+), 62 deletions(-)

diff --git a/gcc/range-op-float.cc b/gcc/range-op-float.cc
index 0f928b6c098..f979ca597cb 100644
--- a/gcc/range-op-float.cc
+++ b/gcc/range-op-float.cc
@@ -150,18 +150,6 @@ range_operator_float::op1_op2_relation (const irange &lhs ATTRIBUTE_UNUSED) cons
   return VREL_VARYING;
 }
 
-// Set R to [NAN, NAN].
-
-static inline void
-frange_set_nan (frange &r, tree type)
-{
-  REAL_VALUE_TYPE rv;
-  bool res = real_nan (&rv, "", 1, TYPE_MODE (type));
-  if (flag_checking)
-    gcc_assert (res);
-  r.set (type, rv, rv);
-}
-
 // Return TRUE if OP1 is known to be free of NANs.
 
 static inline bool
@@ -248,7 +236,7 @@ build_lt (frange &r, tree type, const REAL_VALUE_TYPE &val)
   if (real_isinf (&val, 1))
     {
       if (HONOR_NANS (type))
-	frange_set_nan (r, type);
+	r.set_nan (type);
       else
 	r.set_undefined ();
       return false;
@@ -286,7 +274,7 @@ build_gt (frange &r, tree type, const REAL_VALUE_TYPE &val)
   if (real_isinf (&val, 0))
     {
       if (HONOR_NANS (type))
-	frange_set_nan (r, type);
+	r.set_nan (type);
       else
 	r.set_undefined ();
       return false;
@@ -392,14 +380,14 @@ foperator_equal::op1_range (frange &r, tree type,
       if (HONOR_SIGNED_ZEROS (type) && r.contains_p (build_zero_cst (type)))
 	r.set_signbit (fp_prop::VARYING);
       // The TRUE side of op1 == op2 implies op1 is !NAN.
-      r.set_nan (fp_prop::NO);
+      r.clear_nan ();
       break;
 
     case BRS_FALSE:
       r.set_varying (type);
       // The FALSE side of op1 == op1 implies op1 is a NAN.
       if (rel == VREL_EQ)
-	frange_set_nan (r, type);
+	r.set_nan (type);
       // If the result is false, the only time we know anything is
       // if OP2 is a constant.
       else if (op2.singleton_p ()
@@ -496,7 +484,7 @@ foperator_not_equal::op1_range (frange &r, tree type,
       if (HONOR_SIGNED_ZEROS (type) && r.contains_p (build_zero_cst (type)))
 	r.set_signbit (fp_prop::VARYING);
       // The FALSE side of op1 != op2 implies op1 is !NAN.
-      r.set_nan (fp_prop::NO);
+      r.clear_nan ();
       break;
 
     default:
@@ -563,7 +551,7 @@ foperator_lt::op1_range (frange &r,
     case BRS_TRUE:
       if (build_lt (r, type, op2.upper_bound ()))
 	{
-	  r.set_nan (fp_prop::NO);
+	  r.clear_nan ();
 	  // x < y implies x is not +INF.
 	  frange_drop_inf (r, type);
 	}
@@ -591,7 +579,7 @@ foperator_lt::op2_range (frange &r,
     case BRS_TRUE:
       if (build_gt (r, type, op1.lower_bound ()))
 	{
-	  r.set_nan (fp_prop::NO);
+	  r.clear_nan ();
 	  // x < y implies y is not -INF.
 	  frange_drop_ninf (r, type);
 	}
@@ -664,7 +652,7 @@ foperator_le::op1_range (frange &r,
     {
     case BRS_TRUE:
       if (build_le (r, type, op2.upper_bound ()))
-	r.set_nan (fp_prop::NO);
+	r.clear_nan ();
       break;
 
     case BRS_FALSE:
@@ -688,7 +676,7 @@ foperator_le::op2_range (frange &r,
     {
     case BRS_TRUE:
       if (build_ge (r, type, op1.lower_bound ()))
-	r.set_nan (fp_prop::NO);
+	r.clear_nan ();
       break;
 
     case BRS_FALSE:
@@ -759,7 +747,7 @@ foperator_gt::op1_range (frange &r,
     case BRS_TRUE:
       if (build_gt (r, type, op2.lower_bound ()))
 	{
-	  r.set_nan (fp_prop::NO);
+	  r.clear_nan ();
 	  // x > y implies x is not -INF.
 	  frange_drop_ninf (r, type);
 	}
@@ -787,7 +775,7 @@ foperator_gt::op2_range (frange &r,
     case BRS_TRUE:
       if (build_lt (r, type, op1.upper_bound ()))
 	{
-	  r.set_nan (fp_prop::NO);
+	  r.clear_nan ();
 	  // x > y implies y is not +INF.
 	  frange_drop_inf (r, type);
 	}
@@ -860,7 +848,7 @@ foperator_ge::op1_range (frange &r,
     {
     case BRS_TRUE:
       build_ge (r, type, op2.lower_bound ());
-      r.set_nan (fp_prop::NO);
+      r.clear_nan ();
       break;
 
     case BRS_FALSE:
@@ -887,7 +875,7 @@ foperator_ge::op2_range (frange &r, tree type,
 
     case BRS_TRUE:
       build_le (r, type, op1.upper_bound ());
-      r.set_nan (fp_prop::NO);
+      r.clear_nan ();
       break;
 
     default:
@@ -948,13 +936,13 @@ foperator_unordered::op1_range (frange &r, tree type,
       // Since at least one operand must be NAN, if one of them is
       // not, the other must be.
       if (!op2.maybe_nan ())
-	frange_set_nan (r, type);
+	r.set_nan (type);
       break;
 
     case BRS_FALSE:
       r.set_varying (type);
       // A false UNORDERED means both operands are !NAN.
-      r.set_nan (fp_prop::NO);
+      r.clear_nan ();
       break;
 
     default:
@@ -1011,14 +999,14 @@ foperator_ordered::op1_range (frange &r, tree type,
     case BRS_TRUE:
       r.set_varying (type);
       // The TRUE side of op1 ORDERED op2 implies op1 is !NAN.
-      r.set_nan (fp_prop::NO);
+      r.clear_nan ();
       break;
 
     case BRS_FALSE:
       r.set_varying (type);
       // The FALSE side of op1 ORDERED op1 implies op1 is !NAN.
       if (rel == VREL_EQ)
-	r.set_nan (fp_prop::NO);
+	r.clear_nan ();
       break;
 
     default:
diff --git a/gcc/value-query.cc b/gcc/value-query.cc
index de83f469be4..ea6e4b979ad 100644
--- a/gcc/value-query.cc
+++ b/gcc/value-query.cc
@@ -223,9 +223,9 @@ range_query::get_tree_range (vrange &r, tree expr, gimple *stmt)
 	// Singletons from the tree world have known properties.
 	REAL_VALUE_TYPE *rv = TREE_REAL_CST_PTR (expr);
 	if (real_isnan (rv))
-	  f.set_nan (fp_prop::YES);
+	  f.update_nan (fp_prop::YES);
 	else
-	  f.set_nan (fp_prop::NO);
+	  f.clear_nan ();
 	if (real_isneg (rv))
 	  f.set_signbit (fp_prop::YES);
 	else
diff --git a/gcc/value-range.cc b/gcc/value-range.cc
index d40a4ebf657..dd827421aca 100644
--- a/gcc/value-range.cc
+++ b/gcc/value-range.cc
@@ -270,7 +270,7 @@ tree_compare (tree_code code, tree op1, tree op2)
 // Set the NAN property.  Adjust the range if appopriate.
 
 void
-frange::set_nan (fp_prop::kind k)
+frange::update_nan (fp_prop::kind k)
 {
   if (k == fp_prop::YES)
     {
@@ -280,7 +280,7 @@ frange::set_nan (fp_prop::kind k)
 	  return;
 	}
       gcc_checking_assert (!undefined_p ());
-      *this = frange_nan (m_type);
+      set_nan (m_type);
       return;
     }
 
@@ -474,7 +474,7 @@ frange::union_ (const vrange &v)
       *this = r;
       m_props = save;
       m_props.union_ (r.m_props);
-      set_nan (fp_prop::VARYING);
+      update_nan (fp_prop::VARYING);
       if (flag_checking)
 	verify_range ();
       return true;
@@ -482,7 +482,7 @@ frange::union_ (const vrange &v)
   if (r.known_nan ())
     {
       m_props.union_ (r.m_props);
-      set_nan (fp_prop::VARYING);
+      update_nan (fp_prop::VARYING);
       if (flag_checking)
 	verify_range ();
       return true;
@@ -531,7 +531,7 @@ frange::intersect (const vrange &v)
       if (m_props == r.m_props)
 	return false;
 
-      *this = frange_nan (m_type);
+      set_nan (m_type);
       return true;
     }
   // ?? Perhaps the intersection of a NAN and anything is a NAN ??.
@@ -3634,14 +3634,14 @@ range_tests_nan ()
       r1 = frange_float ("10", "12");
       r0 = r1;
       ASSERT_EQ (r0, r1);
-      r0.set_nan (fp_prop::NO);
+      r0.clear_nan ();
       ASSERT_NE (r0, r1);
-      r0.set_nan (fp_prop::YES);
+      r0.clear_nan ();
       ASSERT_NE (r0, r1);
     }
 
   // NAN ranges are not equal to each other.
-  r0 = frange_nan (float_type_node);
+  r0.set_nan (float_type_node);
   r1 = r0;
   ASSERT_FALSE (r0 == r1);
   ASSERT_FALSE (r0 == r0);
@@ -3649,8 +3649,8 @@ range_tests_nan ()
 
   // [5,6] U NAN = [5,6] NAN.
   r0 = frange_float ("5", "6");
-  r0.set_nan (fp_prop::NO);
-  r1 = frange_nan (float_type_node);
+  r0.clear_nan ();
+  r1.set_nan (float_type_node);
   r0.union_ (r1);
   real_from_string (&q, "5");
   real_from_string (&r, "6");
@@ -3659,34 +3659,34 @@ range_tests_nan ()
   ASSERT_TRUE (r0.maybe_nan ());
 
   // NAN U NAN = NAN
-  r0 = frange_nan (float_type_node);
-  r1 = frange_nan (float_type_node);
+  r0.set_nan (float_type_node);
+  r1.set_nan (float_type_node);
   r0.union_ (r1);
   ASSERT_TRUE (real_isnan (&r0.lower_bound ()));
   ASSERT_TRUE (real_isnan (&r1.upper_bound ()));
   ASSERT_TRUE (r0.known_nan ());
 
   // [INF, INF] ^ NAN = VARYING
-  r0 = frange_nan (float_type_node);
+  r0.set_nan (float_type_node);
   r1 = frange_float ("+Inf", "+Inf");
   r0.intersect (r1);
   ASSERT_TRUE (r0.varying_p ());
 
   // NAN ^ NAN = NAN
-  r0 = frange_nan (float_type_node);
-  r1 = frange_nan (float_type_node);
+  r0.set_nan (float_type_node);
+  r1.set_nan (float_type_node);
   r0.intersect (r1);
   ASSERT_TRUE (r0.known_nan ());
 
   // VARYING ^ NAN = NAN.
-  r0 = frange_nan (float_type_node);
+  r0.set_nan (float_type_node);
   r1.set_varying (float_type_node);
   r0.intersect (r1);
   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);
+  r0.update_nan (fp_prop::YES);
   ASSERT_TRUE (r0.known_nan ());
   ASSERT_TRUE (real_isnan (&r0.lower_bound ()));
   ASSERT_TRUE (real_isnan (&r0.upper_bound ()));
@@ -3736,7 +3736,7 @@ range_tests_signed_zeros ()
   ASSERT_TRUE (r0.zero_p () && !r0.known_signbit (signbit));
 
   // NAN U [5,6] should be [5,6] with no sign info.
-  r0 = frange_nan (float_type_node);
+  r0.set_nan (float_type_node);
   r1 = frange_float ("5", "6");
   r0.union_ (r1);
   real_from_string (&q, "5");
@@ -3762,7 +3762,7 @@ range_tests_signbit ()
   // the signbit property set.
   r0 = frange_float ("-5", "10");
   r0.set_signbit (fp_prop::YES);
-  r0.set_nan (fp_prop::NO);
+  r0.clear_nan ();
   ASSERT_TRUE (r0.known_signbit (signbit) && signbit);
   r1 = frange_float ("-5", "0");
   ASSERT_TRUE (real_identical (&r0.lower_bound (), &r1.lower_bound ()));
@@ -3770,20 +3770,20 @@ range_tests_signbit ()
 
   // Negative numbers should have the SIGNBIT set.
   r0 = frange_float ("-5", "-1");
-  r0.set_nan (fp_prop::NO);
+  r0.clear_nan ();
   ASSERT_TRUE (r0.known_signbit (signbit) && signbit);
   // Positive numbers should have the SIGNBIT clear.
   r0 = frange_float ("1", "10");
-  r0.set_nan (fp_prop::NO);
+  r0.clear_nan ();
   ASSERT_TRUE (r0.known_signbit (signbit) && !signbit);
   // Numbers containing zero should have an unknown SIGNBIT.
   r0 = frange_float ("0", "10");
-  r0.set_nan (fp_prop::NO);
+  r0.clear_nan ();
   ASSERT_TRUE (!r0.known_signbit (signbit));
   // Numbers spanning both positive and negative should have an
   // unknown SIGNBIT.
   r0 = frange_float ("-10", "10");
-  r0.set_nan (fp_prop::NO);
+  r0.clear_nan ();
   ASSERT_TRUE (!r0.known_signbit (signbit));
   r0.set_varying (float_type_node);
   ASSERT_TRUE (!r0.known_signbit (signbit));
@@ -3791,12 +3791,12 @@ range_tests_signbit ()
   // Ignore signbit changes when the sign bit is obviously known from
   // the range.
   r0 = frange_float ("5", "10");
-  r0.set_nan (fp_prop::NO);
+  r0.clear_nan ();
   r0.set_signbit (fp_prop::VARYING);
   ASSERT_TRUE (r0.known_signbit (signbit) && !signbit);
   r0 = frange_float ("-5", "-1");
   r0.set_signbit (fp_prop::NO);
-  r0.set_nan (fp_prop::NO);
+  r0.clear_nan ();
   ASSERT_TRUE (r0.undefined_p ());
 }
 
@@ -3817,7 +3817,7 @@ range_tests_floats ()
   if (r0.maybe_nan ())
     ASSERT_TRUE (r0.varying_p ());
   // ...unless it has some special property...
-  r0.set_nan (fp_prop::NO);
+  r0.clear_nan ();
   ASSERT_FALSE (r0.varying_p ());
 
   // The endpoints of a VARYING are +-INF.
diff --git a/gcc/value-range.h b/gcc/value-range.h
index 0ba0193bc1f..6e896eb9ab5 100644
--- a/gcc/value-range.h
+++ b/gcc/value-range.h
@@ -348,6 +348,7 @@ public:
   virtual void set (tree, tree, value_range_kind = VR_RANGE) override;
   void set (tree type, const REAL_VALUE_TYPE &, const REAL_VALUE_TYPE &,
 	    value_range_kind = VR_RANGE);
+  void set_nan (tree type);
   virtual void set_varying (tree type) override;
   virtual void set_undefined () override;
   virtual bool union_ (const vrange &) override;
@@ -376,7 +377,8 @@ public:
   bool known_signbit (bool &signbit) const;
 
   // Accessors for FP properties.
-  void set_nan (fp_prop::kind f);
+  void update_nan (fp_prop::kind f);
+  void clear_nan () { update_nan (fp_prop::NO); }
   void set_signbit (fp_prop::kind);
 private:
   fp_prop get_nan () const { return m_props.get_nan (); }
@@ -1186,13 +1188,12 @@ real_min_representable (REAL_VALUE_TYPE *r, tree type)
 
 // Build a NAN of type TYPE.
 
-inline frange
-frange_nan (tree type)
+inline void
+frange::set_nan (tree type)
 {
   REAL_VALUE_TYPE r;
-
   gcc_assert (real_nan (&r, "", 1, TYPE_MODE (type)));
-  return frange (type, r, r);
+  set (type, r, r);
 }
 
 // Return TRUE if range is known to be finite.
-- 
2.37.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [COMMITTED] Use frange::set_nan() from the generic frange::set().
  2022-09-14 15:08 [COMMITTED] Minor fixes to frange Aldy Hernandez
  2022-09-14 15:08 ` [COMMITTED] Provide cleaner set_nan(), clear_nan(), and update_nan() methods Aldy Hernandez
@ 2022-09-14 15:08 ` Aldy Hernandez
  2022-09-14 15:08 ` [COMMITTED] Pass full range to build_* in range-op-float.cc Aldy Hernandez
  2022-09-14 15:08 ` [COMMITTED] frange: add both zeros to ranges when there's the possiblity of equality Aldy Hernandez
  3 siblings, 0 replies; 5+ messages in thread
From: Aldy Hernandez @ 2022-09-14 15:08 UTC (permalink / raw)
  To: GCC patches; +Cc: Andrew MacLeod, Aldy Hernandez

This patch cleans up the frange::set() code by passing all things NAN
to frange::set_nan().

No functional changes.

Regstrapped on x86-64 Linux, plus I ran selftests for
-ffinite-math-only.

gcc/ChangeLog:

	* value-range.cc (frange::set): Use set_nan.
	* value-range.h (frange::set_nan): Inline code originally in
	set().
---
 gcc/value-range.cc | 27 ++++++++++++++-------------
 gcc/value-range.h  |  9 ++++++++-
 2 files changed, 22 insertions(+), 14 deletions(-)

diff --git a/gcc/value-range.cc b/gcc/value-range.cc
index dd827421aca..d759fcf178c 100644
--- a/gcc/value-range.cc
+++ b/gcc/value-range.cc
@@ -369,32 +369,33 @@ frange::set (tree min, tree max, value_range_kind kind)
       return;
     }
 
+  // Handle NANs.
+  if (real_isnan (TREE_REAL_CST_PTR (min)) || real_isnan (TREE_REAL_CST_PTR (max)))
+    {
+      gcc_checking_assert (real_identical (TREE_REAL_CST_PTR (min),
+					   TREE_REAL_CST_PTR (max)));
+      tree type = TREE_TYPE (min);
+      set_nan (type);
+      return;
+    }
+
   m_kind = kind;
   m_type = TREE_TYPE (min);
   m_props.set_varying ();
   m_min = *TREE_REAL_CST_PTR (min);
   m_max = *TREE_REAL_CST_PTR (max);
 
-  bool is_nan = (real_isnan (TREE_REAL_CST_PTR (min))
-		 || real_isnan (TREE_REAL_CST_PTR (max)));
-
-  // Ranges with a NAN and a non-NAN endpoint are nonsensical.
-  gcc_checking_assert (!is_nan || operand_equal_p (min, max));
-
-  // Set NAN property if we're absolutely sure.
-  if (is_nan && operand_equal_p (min, max))
-    m_props.nan_set_yes ();
-  else if (!HONOR_NANS (m_type))
-    m_props.nan_set_no ();
-
   // Set SIGNBIT property for positive and negative ranges.
   if (real_less (&m_max, &dconst0))
     m_props.signbit_set_yes ();
   else if (real_less (&dconst0, &m_min))
     m_props.signbit_set_no ();
 
+  if (!HONOR_NANS (m_type))
+    m_props.nan_set_no ();
+
   // Check for swapped ranges.
-  gcc_checking_assert (is_nan || tree_compare (LE_EXPR, min, max));
+  gcc_checking_assert (tree_compare (LE_EXPR, min, max));
 
   normalize_kind ();
   if (flag_checking)
diff --git a/gcc/value-range.h b/gcc/value-range.h
index 6e896eb9ab5..4392de84c8b 100644
--- a/gcc/value-range.h
+++ b/gcc/value-range.h
@@ -1193,7 +1193,14 @@ frange::set_nan (tree type)
 {
   REAL_VALUE_TYPE r;
   gcc_assert (real_nan (&r, "", 1, TYPE_MODE (type)));
-  set (type, r, r);
+  m_kind = VR_RANGE;
+  m_type = type;
+  m_min = r;
+  m_max = r;
+  m_props.set_varying ();
+  m_props.nan_set_yes ();
+  if (flag_checking)
+    verify_range ();
 }
 
 // Return TRUE if range is known to be finite.
-- 
2.37.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [COMMITTED] Pass full range to build_* in range-op-float.cc
  2022-09-14 15:08 [COMMITTED] Minor fixes to frange Aldy Hernandez
  2022-09-14 15:08 ` [COMMITTED] Provide cleaner set_nan(), clear_nan(), and update_nan() methods Aldy Hernandez
  2022-09-14 15:08 ` [COMMITTED] Use frange::set_nan() from the generic frange::set() Aldy Hernandez
@ 2022-09-14 15:08 ` Aldy Hernandez
  2022-09-14 15:08 ` [COMMITTED] frange: add both zeros to ranges when there's the possiblity of equality Aldy Hernandez
  3 siblings, 0 replies; 5+ messages in thread
From: Aldy Hernandez @ 2022-09-14 15:08 UTC (permalink / raw)
  To: GCC patches; +Cc: Andrew MacLeod, Aldy Hernandez

The build_<relop> helper functions in range-op-float.cc take the
actual value from the operand's endpoint, but this value could be
deduced from the operand itself therefore cleaning up the call site.
This also reduces the potential of mistakenly passing the wrong bound.

No functional changes.

Regstrapped on x86-64 Linux, plus I ran selftests for
-ffinite-math-only.

gcc/ChangeLog:

	* range-op-float.cc (build_le): Accept frange instead of number.
	(build_lt): Same.
	(build_ge): Same.
	(build_gt): Same.
	(foperator_lt::op1_range): Pass full range to build_*.
	(foperator_lt::op2_range): Same.
	(foperator_le::op1_range): Same.
	(foperator_le::op2_range): Same.
	(foperator_gt::op1_range): Same.
	(foperator_gt::op2_range): Same.
	(foperator_ge::op1_range): Same.
	(foperator_ge::op2_range): Same.
---
 gcc/range-op-float.cc | 72 +++++++++++++++++++++----------------------
 1 file changed, 36 insertions(+), 36 deletions(-)

diff --git a/gcc/range-op-float.cc b/gcc/range-op-float.cc
index f979ca597cb..8f3e5241313 100644
--- a/gcc/range-op-float.cc
+++ b/gcc/range-op-float.cc
@@ -208,32 +208,32 @@ frange_drop_ninf (frange &r, tree type)
   r.intersect (tmp);
 }
 
-// (X <= VAL) produces the range of [-INF, VAL].
+// Build a range that is <= VAL and store it in R.
 
 static bool
-build_le (frange &r, tree type, const REAL_VALUE_TYPE &val)
+build_le (frange &r, tree type, const frange &val)
 {
-  if (real_isnan (&val))
+  if (val.known_nan ())
     {
       r.set_undefined ();
       return false;
     }
-  r.set (type, dconstninf, val);
+  r.set (type, dconstninf, val.upper_bound ());
   return true;
 }
 
-// (X < VAL) produces the range of [-INF, VAL).
+// Build a range that is < VAL and store it in R.
 
 static bool
-build_lt (frange &r, tree type, const REAL_VALUE_TYPE &val)
+build_lt (frange &r, tree type, const frange &val)
 {
-  if (real_isnan (&val))
+  if (val.known_nan ())
     {
       r.set_undefined ();
       return false;
     }
   // < -INF is outside the range.
-  if (real_isinf (&val, 1))
+  if (real_isinf (&val.upper_bound (), 1))
     {
       if (HONOR_NANS (type))
 	r.set_nan (type);
@@ -241,37 +241,37 @@ build_lt (frange &r, tree type, const REAL_VALUE_TYPE &val)
 	r.set_undefined ();
       return false;
     }
-  // Hijack LE because we only support closed intervals.
-  build_le (r, type, val);
+  // We only support closed intervals.
+  r.set (type, dconstninf, val.upper_bound ());
   return true;
 }
 
-// (X >= VAL) produces the range of [VAL, +INF].
+// Build a range that is >= VAL and store it in R.
 
 static bool
-build_ge (frange &r, tree type, const REAL_VALUE_TYPE &val)
+build_ge (frange &r, tree type, const frange &val)
 {
-  if (real_isnan (&val))
+  if (val.known_nan ())
     {
       r.set_undefined ();
       return false;
     }
-  r.set (type, val, dconstinf);
+  r.set (type, val.lower_bound (), dconstinf);
   return true;
 }
 
-// (X > VAL) produces the range of (VAL, +INF].
+// Build a range that is > VAL and store it in R.
 
 static bool
-build_gt (frange &r, tree type, const REAL_VALUE_TYPE &val)
+build_gt (frange &r, tree type, const frange &val)
 {
-  if (real_isnan (&val))
+  if (val.known_nan ())
     {
       r.set_undefined ();
       return false;
     }
   // > +INF is outside the range.
-  if (real_isinf (&val, 0))
+  if (real_isinf (&val.lower_bound (), 0))
     {
       if (HONOR_NANS (type))
 	r.set_nan (type);
@@ -280,8 +280,8 @@ build_gt (frange &r, tree type, const REAL_VALUE_TYPE &val)
       return false;
     }
 
-  // Hijack GE because we only support closed intervals.
-  build_ge (r, type, val);
+  // We only support closed intervals.
+  r.set (type, val.lower_bound (), dconstinf);
   return true;
 }
 
@@ -549,7 +549,7 @@ foperator_lt::op1_range (frange &r,
   switch (get_bool_state (r, lhs, type))
     {
     case BRS_TRUE:
-      if (build_lt (r, type, op2.upper_bound ()))
+      if (build_lt (r, type, op2))
 	{
 	  r.clear_nan ();
 	  // x < y implies x is not +INF.
@@ -558,7 +558,7 @@ foperator_lt::op1_range (frange &r,
       break;
 
     case BRS_FALSE:
-      build_ge (r, type, op2.lower_bound ());
+      build_ge (r, type, op2);
       break;
 
     default:
@@ -577,7 +577,7 @@ foperator_lt::op2_range (frange &r,
   switch (get_bool_state (r, lhs, type))
     {
     case BRS_TRUE:
-      if (build_gt (r, type, op1.lower_bound ()))
+      if (build_gt (r, type, op1))
 	{
 	  r.clear_nan ();
 	  // x < y implies y is not -INF.
@@ -586,7 +586,7 @@ foperator_lt::op2_range (frange &r,
       break;
 
     case BRS_FALSE:
-      build_le (r, type, op1.upper_bound ());
+      build_le (r, type, op1);
       break;
 
     default:
@@ -651,12 +651,12 @@ foperator_le::op1_range (frange &r,
   switch (get_bool_state (r, lhs, type))
     {
     case BRS_TRUE:
-      if (build_le (r, type, op2.upper_bound ()))
+      if (build_le (r, type, op2))
 	r.clear_nan ();
       break;
 
     case BRS_FALSE:
-      build_gt (r, type, op2.lower_bound ());
+      build_gt (r, type, op2);
       break;
 
     default:
@@ -675,12 +675,12 @@ foperator_le::op2_range (frange &r,
   switch (get_bool_state (r, lhs, type))
     {
     case BRS_TRUE:
-      if (build_ge (r, type, op1.lower_bound ()))
+      if (build_ge (r, type, op1))
 	r.clear_nan ();
       break;
 
     case BRS_FALSE:
-      build_lt (r, type, op1.upper_bound ());
+      build_lt (r, type, op1);
       break;
 
     default:
@@ -745,7 +745,7 @@ foperator_gt::op1_range (frange &r,
   switch (get_bool_state (r, lhs, type))
     {
     case BRS_TRUE:
-      if (build_gt (r, type, op2.lower_bound ()))
+      if (build_gt (r, type, op2))
 	{
 	  r.clear_nan ();
 	  // x > y implies x is not -INF.
@@ -754,7 +754,7 @@ foperator_gt::op1_range (frange &r,
       break;
 
     case BRS_FALSE:
-      build_le (r, type, op2.upper_bound ());
+      build_le (r, type, op2);
       break;
 
     default:
@@ -773,7 +773,7 @@ foperator_gt::op2_range (frange &r,
   switch (get_bool_state (r, lhs, type))
     {
     case BRS_TRUE:
-      if (build_lt (r, type, op1.upper_bound ()))
+      if (build_lt (r, type, op1))
 	{
 	  r.clear_nan ();
 	  // x > y implies y is not +INF.
@@ -782,7 +782,7 @@ foperator_gt::op2_range (frange &r,
       break;
 
     case BRS_FALSE:
-      build_ge (r, type, op1.lower_bound ());
+      build_ge (r, type, op1);
       break;
 
     default:
@@ -847,12 +847,12 @@ foperator_ge::op1_range (frange &r,
   switch (get_bool_state (r, lhs, type))
     {
     case BRS_TRUE:
-      build_ge (r, type, op2.lower_bound ());
+      build_ge (r, type, op2);
       r.clear_nan ();
       break;
 
     case BRS_FALSE:
-      build_lt (r, type, op2.upper_bound ());
+      build_lt (r, type, op2);
       break;
 
     default:
@@ -870,11 +870,11 @@ foperator_ge::op2_range (frange &r, tree type,
   switch (get_bool_state (r, lhs, type))
     {
     case BRS_FALSE:
-      build_gt (r, type, op1.lower_bound ());
+      build_gt (r, type, op1);
       break;
 
     case BRS_TRUE:
-      build_le (r, type, op1.upper_bound ());
+      build_le (r, type, op1);
       r.clear_nan ();
       break;
 
-- 
2.37.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [COMMITTED] frange: add both zeros to ranges when there's the possiblity of equality.
  2022-09-14 15:08 [COMMITTED] Minor fixes to frange Aldy Hernandez
                   ` (2 preceding siblings ...)
  2022-09-14 15:08 ` [COMMITTED] Pass full range to build_* in range-op-float.cc Aldy Hernandez
@ 2022-09-14 15:08 ` Aldy Hernandez
  3 siblings, 0 replies; 5+ messages in thread
From: Aldy Hernandez @ 2022-09-14 15:08 UTC (permalink / raw)
  To: GCC patches; +Cc: Andrew MacLeod, Aldy Hernandez

Every time there's equality at play, we must be careful that any
equality with zero matches both -0.0 and +0.0 when honoring signed
zeros.

We were doing this correctly for the == and != op1_range operators
(albeit inefficiently), but aren't doing it at all when building >=
and <=.  This fixes the oversight.

There is change in functionality here for the build_* functions.

This is the last "simple" patch I submit before overhauling NAN and
sign tracking.  And that will likely be after Cauldron because it will need
further testing (lapack, ppc64le, -ffinite-math-only, etc).

Regstrapped on x86-64 Linux, plus I ran selftests for
-ffinite-math-only.

gcc/ChangeLog:

	* range-op-float.cc (frange_add_zeros): New.
	(build_le): Call frange_add_zeros.
	(build_ge): Same.
	(foperator_equal::op1_range): Same.
	(foperator_not_equal::op1_range): Same.
---
 gcc/range-op-float.cc | 31 +++++++++++++++++++++++++------
 1 file changed, 25 insertions(+), 6 deletions(-)

diff --git a/gcc/range-op-float.cc b/gcc/range-op-float.cc
index 8f3e5241313..fbc14a730ad 100644
--- a/gcc/range-op-float.cc
+++ b/gcc/range-op-float.cc
@@ -208,6 +208,19 @@ frange_drop_ninf (frange &r, tree type)
   r.intersect (tmp);
 }
 
+// If zero is in R, make sure both -0.0 and +0.0 are in the range.
+
+static inline void
+frange_add_zeros (frange &r, tree type)
+{
+  if (r.undefined_p () || r.known_nan ())
+    return;
+
+  if (HONOR_SIGNED_ZEROS (type)
+      && (real_iszero (&r.lower_bound ()) || real_iszero (&r.upper_bound ())))
+    r.set_signbit (fp_prop::VARYING);
+}
+
 // Build a range that is <= VAL and store it in R.
 
 static bool
@@ -219,6 +232,10 @@ build_le (frange &r, tree type, const frange &val)
       return false;
     }
   r.set (type, dconstninf, val.upper_bound ());
+
+  // Add both zeros if there's the possibility of zero equality.
+  frange_add_zeros (r, type);
+
   return true;
 }
 
@@ -257,6 +274,10 @@ build_ge (frange &r, tree type, const frange &val)
       return false;
     }
   r.set (type, val.lower_bound (), dconstinf);
+
+  // Add both zeros if there's the possibility of zero equality.
+  frange_add_zeros (r, type);
+
   return true;
 }
 
@@ -376,9 +397,8 @@ foperator_equal::op1_range (frange &r, tree type,
     case BRS_TRUE:
       // If it's true, the result is the same as OP2.
       r = op2;
-      // Make sure we don't copy the sign bit if we may have a zero.
-      if (HONOR_SIGNED_ZEROS (type) && r.contains_p (build_zero_cst (type)))
-	r.set_signbit (fp_prop::VARYING);
+      // Add both zeros if there's the possibility of zero equality.
+      frange_add_zeros (r, type);
       // The TRUE side of op1 == op2 implies op1 is !NAN.
       r.clear_nan ();
       break;
@@ -480,9 +500,8 @@ foperator_not_equal::op1_range (frange &r, tree type,
     case BRS_FALSE:
       // If it's false, the result is the same as OP2.
       r = op2;
-      // Make sure we don't copy the sign bit if we may have a zero.
-      if (HONOR_SIGNED_ZEROS (type) && r.contains_p (build_zero_cst (type)))
-	r.set_signbit (fp_prop::VARYING);
+      // Add both zeros if there's the possibility of zero equality.
+      frange_add_zeros (r, type);
       // The FALSE side of op1 != op2 implies op1 is !NAN.
       r.clear_nan ();
       break;
-- 
2.37.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2022-09-14 15:09 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-14 15:08 [COMMITTED] Minor fixes to frange Aldy Hernandez
2022-09-14 15:08 ` [COMMITTED] Provide cleaner set_nan(), clear_nan(), and update_nan() methods Aldy Hernandez
2022-09-14 15:08 ` [COMMITTED] Use frange::set_nan() from the generic frange::set() Aldy Hernandez
2022-09-14 15:08 ` [COMMITTED] Pass full range to build_* in range-op-float.cc Aldy Hernandez
2022-09-14 15:08 ` [COMMITTED] frange: add both zeros to ranges when there's the possiblity of equality Aldy Hernandez

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).