| Message ID | 2fc9b482-d119-8e29-91dc-078664dbeaea@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | handle symbolics when comparing ranges | expand |
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
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?
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
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.
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