| Message ID | b63b775a-d313-f9b1-74f9-58181708732b@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | implement value_range::domain_p() | expand |
On Thu, Nov 8, 2018 at 1:52 PM Aldy Hernandez <aldyh@redhat.com> wrote: > > I believe I've seen this idiom more than once. I know for sure I've > used it in our ssa-range branch :). I'll hunt for the other uses and > adjust accordingly. domain_p?! Isn't that the same as varying_p()? Also + if (m_kind == VR_RANGE) + { + tree type = TREE_TYPE (type ()); + value_range vr (VR_RANGE, TYPE_MIN_VALUE (type), TYPE_MAX_VALUE (type)); TYPE_MIN/MAX_VALUE is NULL for pointers + return equal_p (vr, /*ignore_equivs=*/true); But equivs essentially are making the range smaller! The range is the intersection of itself with all equiv ranges! Richard. + } > OK?
On 11/8/18 9:34 AM, Richard Biener wrote: > On Thu, Nov 8, 2018 at 1:52 PM Aldy Hernandez <aldyh@redhat.com> wrote: >> >> I believe I've seen this idiom more than once. I know for sure I've >> used it in our ssa-range branch :). I'll hunt for the other uses and >> adjust accordingly. > > domain_p?! Isn't that the same as varying_p()? Also Sigh, yes. If we kept normalized value_ranges around where [MIN,MAX] degraded into VR_VARYING, then yes. But alas, we have two ways of representing the entire domain. Don't look at me. That crap was already there :). Another approach would be to call ::set_and_canonicalize() before checking varying_p() and teach the canonicalize function that [MIN, MAX] is VR_VARYING. How does that sound? Aldy > > + if (m_kind == VR_RANGE) > + { > + tree type = TREE_TYPE (type ()); > + value_range vr (VR_RANGE, TYPE_MIN_VALUE (type), TYPE_MAX_VALUE (type)); > > TYPE_MIN/MAX_VALUE is NULL for pointers > > + return equal_p (vr, /*ignore_equivs=*/true); > > But equivs essentially are making the range smaller! The range > is the intersection of itself with all equiv ranges! > > Richard. > > + } > > >> OK?
On Thu, Nov 8, 2018 at 3:40 PM Aldy Hernandez <aldyh@redhat.com> wrote: > > > > On 11/8/18 9:34 AM, Richard Biener wrote: > > On Thu, Nov 8, 2018 at 1:52 PM Aldy Hernandez <aldyh@redhat.com> wrote: > >> > >> I believe I've seen this idiom more than once. I know for sure I've > >> used it in our ssa-range branch :). I'll hunt for the other uses and > >> adjust accordingly. > > > > domain_p?! Isn't that the same as varying_p()? Also > > Sigh, yes. If we kept normalized value_ranges around where [MIN,MAX] > degraded into VR_VARYING, then yes. But alas, we have two ways of > representing the entire domain. Don't look at me. That crap was > already there :). > > Another approach would be to call ::set_and_canonicalize() before > checking varying_p() and teach the canonicalize function that [MIN, MAX] > is VR_VARYING. How does that sound? But that's still broken for the case where it has equivalences. I fear that if you look at users we'll end up with three or more different varying_p/domain_p things all being subtly different... As said in the other thread we should see to separate equivs out of the way. > Aldy > > > > > + if (m_kind == VR_RANGE) > > + { > > + tree type = TREE_TYPE (type ()); > > + value_range vr (VR_RANGE, TYPE_MIN_VALUE (type), TYPE_MAX_VALUE (type)); > > > > TYPE_MIN/MAX_VALUE is NULL for pointers > > > > + return equal_p (vr, /*ignore_equivs=*/true); > > > > But equivs essentially are making the range smaller! The range > > is the intersection of itself with all equiv ranges! > > > > Richard. > > > > + } > > > > > >> OK?
On 11/8/18 9:53 AM, Richard Biener wrote: > On Thu, Nov 8, 2018 at 3:40 PM Aldy Hernandez <aldyh@redhat.com> wrote: >> >> >> >> On 11/8/18 9:34 AM, Richard Biener wrote: >>> On Thu, Nov 8, 2018 at 1:52 PM Aldy Hernandez <aldyh@redhat.com> wrote: >>>> >>>> I believe I've seen this idiom more than once. I know for sure I've >>>> used it in our ssa-range branch :). I'll hunt for the other uses and >>>> adjust accordingly. >>> >>> domain_p?! Isn't that the same as varying_p()? Also >> >> Sigh, yes. If we kept normalized value_ranges around where [MIN,MAX] >> degraded into VR_VARYING, then yes. But alas, we have two ways of >> representing the entire domain. Don't look at me. That crap was >> already there :). >> >> Another approach would be to call ::set_and_canonicalize() before >> checking varying_p() and teach the canonicalize function that [MIN, MAX] >> is VR_VARYING. How does that sound? > > But that's still broken for the case where it has equivalences. I fear that > if you look at users we'll end up with three or more different > varying_p/domain_p > things all being subtly different... Bah, I give up. I don't have time to look into possible subtleties wrt equivalences right now. I'll drop this patch. > > As said in the other thread we should see to separate equivs out > of the way. And as I meant to say in the other thread, I'll buy you many beers if you can do this for this release :). Aldy
On Thu, Nov 8, 2018 at 4:05 PM Aldy Hernandez <aldyh@redhat.com> wrote: > > > > On 11/8/18 9:53 AM, Richard Biener wrote: > > On Thu, Nov 8, 2018 at 3:40 PM Aldy Hernandez <aldyh@redhat.com> wrote: > >> > >> > >> > >> On 11/8/18 9:34 AM, Richard Biener wrote: > >>> On Thu, Nov 8, 2018 at 1:52 PM Aldy Hernandez <aldyh@redhat.com> wrote: > >>>> > >>>> I believe I've seen this idiom more than once. I know for sure I've > >>>> used it in our ssa-range branch :). I'll hunt for the other uses and > >>>> adjust accordingly. > >>> > >>> domain_p?! Isn't that the same as varying_p()? Also > >> > >> Sigh, yes. If we kept normalized value_ranges around where [MIN,MAX] > >> degraded into VR_VARYING, then yes. But alas, we have two ways of > >> representing the entire domain. Don't look at me. That crap was > >> already there :). > >> > >> Another approach would be to call ::set_and_canonicalize() before > >> checking varying_p() and teach the canonicalize function that [MIN, MAX] > >> is VR_VARYING. How does that sound? > > > > But that's still broken for the case where it has equivalences. I fear that > > if you look at users we'll end up with three or more different > > varying_p/domain_p > > things all being subtly different... > > Bah, I give up. I don't have time to look into possible subtleties wrt > equivalences right now. I'll drop this patch. > > > > > As said in the other thread we should see to separate equivs out > > of the way. > > And as I meant to say in the other thread, I'll buy you many beers if > you can do this for this release :). Well, yall made a mess out of the nicely contained VRP, and topped it with C++. Now it's ... a mess. And for whatever reason all the C++-ification and refactoring had to happen for GCC 9 :/ > Aldy
On 11/8/18 10:24 AM, Richard Biener wrote: > On Thu, Nov 8, 2018 at 4:05 PM Aldy Hernandez <aldyh@redhat.com> wrote: >> >> >> >> On 11/8/18 9:53 AM, Richard Biener wrote: >>> On Thu, Nov 8, 2018 at 3:40 PM Aldy Hernandez <aldyh@redhat.com> wrote: >>>> >>>> >>>> >>>> On 11/8/18 9:34 AM, Richard Biener wrote: >>>>> On Thu, Nov 8, 2018 at 1:52 PM Aldy Hernandez <aldyh@redhat.com> wrote: >>>>>> >>>>>> I believe I've seen this idiom more than once. I know for sure I've >>>>>> used it in our ssa-range branch :). I'll hunt for the other uses and >>>>>> adjust accordingly. >>>>> >>>>> domain_p?! Isn't that the same as varying_p()? Also >>>> >>>> Sigh, yes. If we kept normalized value_ranges around where [MIN,MAX] >>>> degraded into VR_VARYING, then yes. But alas, we have two ways of >>>> representing the entire domain. Don't look at me. That crap was >>>> already there :). >>>> >>>> Another approach would be to call ::set_and_canonicalize() before >>>> checking varying_p() and teach the canonicalize function that [MIN, MAX] >>>> is VR_VARYING. How does that sound? >>> >>> But that's still broken for the case where it has equivalences. I fear that >>> if you look at users we'll end up with three or more different >>> varying_p/domain_p >>> things all being subtly different... >> >> Bah, I give up. I don't have time to look into possible subtleties wrt >> equivalences right now. I'll drop this patch. >> >>> >>> As said in the other thread we should see to separate equivs out >>> of the way. >> >> And as I meant to say in the other thread, I'll buy you many beers if >> you can do this for this release :). > > Well, yall made a mess out of the nicely contained VRP, and topped > it with C++. > > Now it's ... a mess. You wish! VRP has been a mess for quite a long time. > > And for whatever reason all the C++-ification and refactoring had to happen > for GCC 9 :/ You're gonna absolutely love what we have in store for GCC 10 ;-). Aldy
On Thu, Nov 8, 2018 at 4:30 PM Aldy Hernandez <aldyh@redhat.com> wrote: > > > > On 11/8/18 10:24 AM, Richard Biener wrote: > > On Thu, Nov 8, 2018 at 4:05 PM Aldy Hernandez <aldyh@redhat.com> wrote: > >> > >> > >> > >> On 11/8/18 9:53 AM, Richard Biener wrote: > >>> On Thu, Nov 8, 2018 at 3:40 PM Aldy Hernandez <aldyh@redhat.com> wrote: > >>>> > >>>> > >>>> > >>>> On 11/8/18 9:34 AM, Richard Biener wrote: > >>>>> On Thu, Nov 8, 2018 at 1:52 PM Aldy Hernandez <aldyh@redhat.com> wrote: > >>>>>> > >>>>>> I believe I've seen this idiom more than once. I know for sure I've > >>>>>> used it in our ssa-range branch :). I'll hunt for the other uses and > >>>>>> adjust accordingly. > >>>>> > >>>>> domain_p?! Isn't that the same as varying_p()? Also > >>>> > >>>> Sigh, yes. If we kept normalized value_ranges around where [MIN,MAX] > >>>> degraded into VR_VARYING, then yes. But alas, we have two ways of > >>>> representing the entire domain. Don't look at me. That crap was > >>>> already there :). > >>>> > >>>> Another approach would be to call ::set_and_canonicalize() before > >>>> checking varying_p() and teach the canonicalize function that [MIN, MAX] > >>>> is VR_VARYING. How does that sound? > >>> > >>> But that's still broken for the case where it has equivalences. I fear that > >>> if you look at users we'll end up with three or more different > >>> varying_p/domain_p > >>> things all being subtly different... > >> > >> Bah, I give up. I don't have time to look into possible subtleties wrt > >> equivalences right now. I'll drop this patch. > >> > >>> > >>> As said in the other thread we should see to separate equivs out > >>> of the way. > >> > >> And as I meant to say in the other thread, I'll buy you many beers if > >> you can do this for this release :). > > > > Well, yall made a mess out of the nicely contained VRP, and topped > > it with C++. > > > > Now it's ... a mess. > > You wish! VRP has been a mess for quite a long time. > > > > > And for whatever reason all the C++-ification and refactoring had to happen > > for GCC 9 :/ > > You're gonna absolutely love what we have in store for GCC 10 ;-). I believe it when I see it. Oh wait - that was sarcastic! Richard. > Aldy
* tree-vrp.h (value_range::domain_p): New.
* tree-vrp.c (value_range::domain_p): New.
* tree-ssa-strlen.c (strlen_check_and_optimize_stmt): Use
value_range::domain_p instead of doing things ad-hoc.
diff --git a/gcc/tree-ssa-strlen.c b/gcc/tree-ssa-strlen.c
index 669c315dce2..ddb61e5a7f3 100644
--- a/gcc/tree-ssa-strlen.c
+++ b/gcc/tree-ssa-strlen.c
@@ -3687,19 +3687,16 @@ strlen_check_and_optimize_stmt (gimple_stmt_iterator *gsi, bool *cleanup_eh)
/* Reading a character before the final '\0'
character. Just set the value range to ~[0, 0]
if we don't have anything better. */
- wide_int min, max;
+ value_range vr;
+ get_range_info (lhs, vr);
tree type = TREE_TYPE (lhs);
- enum value_range_kind vr
- = get_range_info (lhs, &min, &max);
- if (vr == VR_VARYING
- || (vr == VR_RANGE
- && min == wi::min_value (TYPE_PRECISION (type),
- TYPE_SIGN (type))
- && max == wi::max_value (TYPE_PRECISION (type),
- TYPE_SIGN (type))))
- set_range_info (lhs, VR_ANTI_RANGE,
- wi::zero (TYPE_PRECISION (type)),
- wi::zero (TYPE_PRECISION (type)));
+ if (vr.domain_p ())
+ {
+ value_range vr (VR_ANTI_RANGE,
+ build_int_cst (type, 0),
+ build_int_cst (type, 0));
+ set_range_info (lhs, vr);
+ }
}
}
}
diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
index 3ccf2e491d6..e2126c21974 100644
--- a/gcc/tree-vrp.c
+++ b/gcc/tree-vrp.c
@@ -178,6 +178,22 @@ value_range::equal_p (const value_range &other, bool ignore_equivs) const
|| vrp_bitmap_equal_p (m_equiv, other.m_equiv)));
}
+/* Return TRUE if value_range spans the entire domain. */
+
+bool
+value_range::domain_p () const
+{
+ if (varying_p ())
+ return true;
+ if (m_kind == VR_RANGE)
+ {
+ tree type = TREE_TYPE (type ());
+ value_range vr (VR_RANGE, TYPE_MIN_VALUE (type), TYPE_MAX_VALUE (type));
+ return equal_p (vr, /*ignore_equivs=*/true);
+ }
+ return false;
+}
+
/* Return equality while ignoring equivalence bitmap. */
bool
diff --git a/gcc/tree-vrp.h b/gcc/tree-vrp.h
index c251329a195..c5811d50bbf 100644
--- a/gcc/tree-vrp.h
+++ b/gcc/tree-vrp.h
@@ -64,6 +64,7 @@ class GTY((for_user)) value_range
/* Misc methods. */
tree type () const;
bool null_p () const;
+ bool domain_p () const;
bool may_contain_p (tree) const;
bool singleton_p (tree *result = NULL) const;
void deep_copy (const value_range *);