diff mbox series

Return true from operator== for two identical ranges containing NAN.

Message ID 20230418085255.252125-1-aldyh@redhat.com
State New
Headers show
Series Return true from operator== for two identical ranges containing NAN. | expand

Commit Message

Aldy Hernandez April 18, 2023, 8:52 a.m. UTC
[Andrew, we talked about this a few months ago.  Just making sure we're
on the same page so I can push it.  Also, a heads-up for Jakub.]

The == operator for ranges signifies that two ranges contain the same
thing, not that they are ultimately equal.  So [2,4] == [2,4], even
though one may be a 2 and the other may be a 3.  Similarly with two
VARYING ranges.

There is an oversight in frange::operator== where we are returning
false for two identical NANs.  This is causing us to never cache NANs
in sbr_sparse_bitmap::set_bb_range.

---
 gcc/value-range.cc | 10 ----------
 1 file changed, 10 deletions(-)

Comments

Andrew MacLeod April 18, 2023, 1:20 p.m. UTC | #1
On 4/18/23 04:52, Aldy Hernandez wrote:
> [Andrew, we talked about this a few months ago.  Just making sure we're
> on the same page so I can push it.  Also, a heads-up for Jakub.]
>
> The == operator for ranges signifies that two ranges contain the same
> thing, not that they are ultimately equal.  So [2,4] == [2,4], even
> though one may be a 2 and the other may be a 3.  Similarly with two
> VARYING ranges.
>
> There is an oversight in frange::operator== where we are returning
> false for two identical NANs.  This is causing us to never cache NANs
> in sbr_sparse_bitmap::set_bb_range.

yes, this is correct.

Andrew
Aldy Hernandez April 18, 2023, 1:23 p.m. UTC | #2
Committed with a changelog.

Thanks.

On Tue, Apr 18, 2023, 15:20 Andrew MacLeod <amacleod@redhat.com> wrote:

>
> On 4/18/23 04:52, Aldy Hernandez wrote:
> > [Andrew, we talked about this a few months ago.  Just making sure we're
> > on the same page so I can push it.  Also, a heads-up for Jakub.]
> >
> > The == operator for ranges signifies that two ranges contain the same
> > thing, not that they are ultimately equal.  So [2,4] == [2,4], even
> > though one may be a 2 and the other may be a 3.  Similarly with two
> > VARYING ranges.
> >
> > There is an oversight in frange::operator== where we are returning
> > false for two identical NANs.  This is causing us to never cache NANs
> > in sbr_sparse_bitmap::set_bb_range.
>
> yes, this is correct.
>
> Andrew
>
>
diff mbox series

Patch

diff --git a/gcc/value-range.cc b/gcc/value-range.cc
index ec826c2fe1b..963330eed79 100644
--- a/gcc/value-range.cc
+++ b/gcc/value-range.cc
@@ -629,9 +629,6 @@  frange::operator== (const frange &src) const
       if (varying_p ())
 	return types_compatible_p (m_type, src.m_type);
 
-      if (known_isnan () || src.known_isnan ())
-	return false;
-
       return (real_identical (&m_min, &src.m_min)
 	      && real_identical (&m_max, &src.m_max)
 	      && m_pos_nan == src.m_pos_nan
@@ -3749,13 +3746,6 @@  range_tests_nan ()
       ASSERT_TRUE (r0.maybe_isnan ());
     }
 
-  // NAN ranges are not equal to each other.
-  r0.set_nan (float_type_node);
-  r1 = r0;
-  ASSERT_FALSE (r0 == r1);
-  ASSERT_FALSE (r0 == r0);
-  ASSERT_TRUE (r0 != r0);
-
   // [5,6] U NAN = [5,6] NAN.
   r0 = frange_float ("5", "6");
   r0.clear_nan ();