handle symbolics when comparing ranges
diff mbox series

Message ID 2fc9b482-d119-8e29-91dc-078664dbeaea@redhat.com
State New
Headers show
Series
  • handle symbolics when comparing ranges
Related show

Commit Message

Aldy Hernandez Nov. 4, 2019, 10:23 p.m. UTC
value_range_base::operator== was originally lifted from a world where 
symbolics didn't exist (the ranger branch), but symbolics do exist in 
mainline.

Although this isn't causing a problem yet, as soon as someone tries to 
compare non numeric ranges, we'll die.  Using operand_equal_p simplifies 
the comparison code drastically.

I suppose if/when we get multiple sub-ranges in value_range_base, we'll 
have to adapt this function again to compare things piece wise.  For 
now, let's keep things simple.

OK pending tests?

Comments

Andrew MacLeod Nov. 4, 2019, 10:45 p.m. UTC | #1
On 11/4/19 5:23 PM, Aldy Hernandez wrote:
> value_range_base::operator== was originally lifted from a world where 
> symbolics didn't exist (the ranger branch), but symbolics do exist in 
> mainline.
>
> Although this isn't causing a problem yet, as soon as someone tries to 
> compare non numeric ranges, we'll die.  Using operand_equal_p 
> simplifies the comparison code drastically.
>
> I suppose if/when we get multiple sub-ranges in value_range_base, 
> we'll have to adapt this function again to compare things piece wise.  
> For now, let's keep things simple.
>
> OK pending tests?
Oh, we brought over the multiple sub-range bits to value_range_base... 
yeah we can remove that and just check for operand equality.  we'll deal 
with multiple subranges when thats a thing.


Is this any different than just calling value_range_base::equal_p()?

Andrew
Aldy Hernandez Nov. 4, 2019, 11:05 p.m. UTC | #2
On 11/4/19 11:45 PM, Andrew MacLeod wrote:
> On 11/4/19 5:23 PM, Aldy Hernandez wrote:
>> value_range_base::operator== was originally lifted from a world where 
>> symbolics didn't exist (the ranger branch), but symbolics do exist in 
>> mainline.
>>
>> Although this isn't causing a problem yet, as soon as someone tries to 
>> compare non numeric ranges, we'll die.  Using operand_equal_p 
>> simplifies the comparison code drastically.
>>
>> I suppose if/when we get multiple sub-ranges in value_range_base, 
>> we'll have to adapt this function again to compare things piece wise. 
>> For now, let's keep things simple.
>>
>> OK pending tests?
> Oh, we brought over the multiple sub-range bits to value_range_base... 
> yeah we can remove that and just check for operand equality.  we'll deal 
> with multiple subranges when thats a thing.
> 
> 
> Is this any different than just calling value_range_base::equal_p()?

Ooops, indeed, that's the same thing.

Adjusted.

OK pending tests?
Andrew MacLeod Nov. 5, 2019, 2:55 a.m. UTC | #3
On 11/4/19 6:05 PM, Aldy Hernandez wrote:
>
>
> On 11/4/19 11:45 PM, Andrew MacLeod wrote:
>> On 11/4/19 5:23 PM, Aldy Hernandez wrote:
>>> value_range_base::operator== was originally lifted from a world 
>>> where symbolics didn't exist (the ranger branch), but symbolics do 
>>> exist in mainline.
>>>
>>> Although this isn't causing a problem yet, as soon as someone tries 
>>> to compare non numeric ranges, we'll die.  Using operand_equal_p 
>>> simplifies the comparison code drastically.
>>>
>>> I suppose if/when we get multiple sub-ranges in value_range_base, 
>>> we'll have to adapt this function again to compare things piece 
>>> wise. For now, let's keep things simple.
>>>
>>> OK pending tests?
>> Oh, we brought over the multiple sub-range bits to 
>> value_range_base... yeah we can remove that and just check for 
>> operand equality.  we'll deal with multiple subranges when thats a 
>> thing.
>>
>>
>> Is this any different than just calling value_range_base::equal_p()?
>
> Ooops, indeed, that's the same thing.
>
> Adjusted.
>
> OK pending tests?
yeah, approved.

Andrew
Aldy Hernandez Nov. 5, 2019, 3:34 a.m. UTC | #4
On 11/5/19 3:55 AM, Andrew MacLeod wrote:
> On 11/4/19 6:05 PM, Aldy Hernandez wrote:
>>
>>
>> On 11/4/19 11:45 PM, Andrew MacLeod wrote:
>>> On 11/4/19 5:23 PM, Aldy Hernandez wrote:
>>>> value_range_base::operator== was originally lifted from a world 
>>>> where symbolics didn't exist (the ranger branch), but symbolics do 
>>>> exist in mainline.
>>>>
>>>> Although this isn't causing a problem yet, as soon as someone tries 
>>>> to compare non numeric ranges, we'll die.  Using operand_equal_p 
>>>> simplifies the comparison code drastically.
>>>>
>>>> I suppose if/when we get multiple sub-ranges in value_range_base, 
>>>> we'll have to adapt this function again to compare things piece 
>>>> wise. For now, let's keep things simple.
>>>>
>>>> OK pending tests?
>>> Oh, we brought over the multiple sub-range bits to 
>>> value_range_base... yeah we can remove that and just check for 
>>> operand equality.  we'll deal with multiple subranges when thats a 
>>> thing.
>>>
>>>
>>> Is this any different than just calling value_range_base::equal_p()?
>>
>> Ooops, indeed, that's the same thing.
>>
>> Adjusted.
>>
>> OK pending tests?
> yeah, approved.

Final committed patch attached.  I had to remove the now unused 
range_compatible_p function, as it's no longer necessary.

Thanks.

Patch
diff mbox series

commit d1177659bd26ac8122410dee092f5ca427b4558b
Author: Aldy Hernandez <aldyh@redhat.com>
Date:   Mon Nov 4 21:20:26 2019 +0100

    Use operand_equal_p in value_range_base::operator== so we can handle
    symbolics without dying.

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 6fbbf87e294..3ebe7fd4348 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,8 @@ 
+2019-11-04  Aldy Hernandez  <aldyh@redhat.com>
+
+	* tree-vrp.c (value_range_base::operator==): Use operand_equal_p
+	instead of wide-int's, to properly handle symbolics.
+
 2019-11-04  Aldy Hernandez  <aldyh@redhat.com>
 
 	* tree-vrp.c (value_range_base::set): Do not special case pointers.
diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
index 452895bfc24..e683339f3cd 100644
--- a/gcc/tree-vrp.c
+++ b/gcc/tree-vrp.c
@@ -6336,16 +6336,12 @@  value_range_base::operator== (const value_range_base &r) const
   if (undefined_p ())
     return r.undefined_p ();
 
-  if (num_pairs () != r.num_pairs ()
-      || !range_compatible_p (type (), r.type ()))
+  if (!range_compatible_p (type (), r.type ()))
     return false;
 
-  for (unsigned p = 0; p < num_pairs (); p++)
-    if (wi::ne_p (lower_bound (p), r.lower_bound (p))
-	|| wi::ne_p (upper_bound (p), r.upper_bound (p)))
-      return false;
-
-  return true;
+  return (m_kind == r.m_kind
+	  && operand_equal_p (m_min, r.m_min, 0)
+	  && operand_equal_p (m_max, r.m_max, 0));
 }
 
 /* Visit all arguments for PHI node PHI that flow through executable