On 6/11/19 12:17 PM, Martin Sebor wrote:
On 6/11/19 9:05 AM, Aldy Hernandez wrote:
On 6/11/19 9:45 AM, Richard Biener wrote:
On Tue, Jun 11, 2019 at 12:40 PM Aldy Hernandez <al...@redhat.com>
wrote:
This patch cleans up the various contains, may_contain, and
value_inside_range variants we have throughout, in favor of one--
contains_p. There should be no changes in functionality.
I have added a note to range_includes_zero_p, perhaps as a personal
question than anything else. This function was/is returning true for
UNDEFINED. From a semantic sense, that doesn't make sense. UNDEFINED
is really the empty set. Is the functionality wrong, or should we call
this function something else? Either way, I'm fine removing the
comment
but I'm genuinely curious.
So this affects for example this hunk:
- if (vr && !range_includes_p (vr, 1))
+ if (vr && (!vr->contains_p (build_one_cst (TREE_TYPE (name)))
+ && !vr->undefined_p ()))
{
I think it's arbitrary how we compute X in UNDEFINED and I'm fine
with changing the affected predicates to return false. This means
not testing for !vr->undefined_p here.
Excellent.
Note I very much dislike the build_one_cst () call here so please
provide an overload hiding this.
Good idea. I love it.
Not sure why you keep range_includes_zero_p.
I wasn't sure if there was some subtle reason why we were including
undefined.
OK pending tests?
Should the second overload:
+ bool contains_p (tree) const;
+ bool contains_p (int) const;
take something like HOST_WIDE_INT or even one of those poly_ints
like build_int_cst does? (With the former, contains_p (0) will
likely be ambiguous since 0 is int and HOST_WIDE_INT is long).
We have a type, so there should be no confusion:
+ return contains_p (build_int_cst (type (), val));
(UNDEFINED and VARYING don't have a type, so they are special cased prior).
Aldy
