Implement {get,set}_range_info() variants that work with value_range's

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

Implement {get,set}_range_info() variants that work with value_range's

Aldy Hernandez-2
get/set_range_info() currently returns the extremes of a range.  I have
implemented overloaded variants that return a proper range.  In the
future we should use actual ranges throughout, and not depend on range
extremes, as depending on this behavior could causes us to lose precision.

I am also including changes to size_must_be_zero_p() to show how we
should be using the range API, as opposed to performing error prone
ad-hoc calculations on ranges and anti-ranges.

Martin, I'm not saying your code is wrong.  There are numerous other
places in the compiler where we manipulate ranges/anti-ranges directly,
all of which should be adapted in the future.  Everywhere there is a
mention of VR_RANGE/VR_ANTI_RANGE in the compiler is suspect.  We should
ideally be using intersect/union/may_contain_p/null_p, etc.

OK pending another round of tests?  (As I had tested this patch along
with a whole slew of other upcoming changes ;-)).

Aldy

curr.patch (4K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Implement {get,set}_range_info() variants that work with value_range's

Richard Biener-2
On Thu, Nov 8, 2018 at 12:52 PM Aldy Hernandez <[hidden email]> wrote:
>
> get/set_range_info() currently returns the extremes of a range.  I have
> implemented overloaded variants that return a proper range.  In the
> future we should use actual ranges throughout, and not depend on range
> extremes, as depending on this behavior could causes us to lose precision.
>
> I am also including changes to size_must_be_zero_p() to show how we
> should be using the range API, as opposed to performing error prone
> ad-hoc calculations on ranges and anti-ranges.

Yeah, I've been talking this all along but not being heard...

> Martin, I'm not saying your code is wrong.  There are numerous other
> places in the compiler where we manipulate ranges/anti-ranges directly,
> all of which should be adapted in the future.  Everywhere there is a
> mention of VR_RANGE/VR_ANTI_RANGE in the compiler is suspect.  We should
> ideally be using intersect/union/may_contain_p/null_p, etc.

null_p is a bad name btw, I just confused it with empty_p ... (which we have
as undefined_p).  contains_only_zero_p would be less confusing.

> OK pending another round of tests?  (As I had tested this patch along
> with a whole slew of other upcoming changes ;-)).

OK.

Richard.

>
> Aldy
Reply | Threaded
Open this post in threaded view
|

Re: Implement {get,set}_range_info() variants that work with value_range's

Aldy Hernandez-2


On 11/8/18 8:59 AM, Richard Biener wrote:

> On Thu, Nov 8, 2018 at 12:52 PM Aldy Hernandez <[hidden email]> wrote:
>>
>> get/set_range_info() currently returns the extremes of a range.  I have
>> implemented overloaded variants that return a proper range.  In the
>> future we should use actual ranges throughout, and not depend on range
>> extremes, as depending on this behavior could causes us to lose precision.
>>
>> I am also including changes to size_must_be_zero_p() to show how we
>> should be using the range API, as opposed to performing error prone
>> ad-hoc calculations on ranges and anti-ranges.
>
> Yeah, I've been talking this all along but not being heard...

My girlfriend says I don't listen.  It could be related.

>> Martin, I'm not saying your code is wrong.  There are numerous other
>> places in the compiler where we manipulate ranges/anti-ranges directly,
>> all of which should be adapted in the future.  Everywhere there is a
>> mention of VR_RANGE/VR_ANTI_RANGE in the compiler is suspect.  We should
>> ideally be using intersect/union/may_contain_p/null_p, etc.
>
> null_p is a bad name btw, I just confused it with empty_p ... (which we have
> as undefined_p).  contains_only_zero_p would be less confusing.

Yes, a horrible name.  I noticed so as I debugged precisely this bit.
How about zero_p?

Aldy
Reply | Threaded
Open this post in threaded view
|

Re: Implement {get,set}_range_info() variants that work with value_range's

Richard Biener-2
On Thu, Nov 8, 2018 at 3:05 PM Aldy Hernandez <[hidden email]> wrote:

>
>
>
> On 11/8/18 8:59 AM, Richard Biener wrote:
> > On Thu, Nov 8, 2018 at 12:52 PM Aldy Hernandez <[hidden email]> wrote:
> >>
> >> get/set_range_info() currently returns the extremes of a range.  I have
> >> implemented overloaded variants that return a proper range.  In the
> >> future we should use actual ranges throughout, and not depend on range
> >> extremes, as depending on this behavior could causes us to lose precision.
> >>
> >> I am also including changes to size_must_be_zero_p() to show how we
> >> should be using the range API, as opposed to performing error prone
> >> ad-hoc calculations on ranges and anti-ranges.
> >
> > Yeah, I've been talking this all along but not being heard...
>
> My girlfriend says I don't listen.  It could be related.
>
> >> Martin, I'm not saying your code is wrong.  There are numerous other
> >> places in the compiler where we manipulate ranges/anti-ranges directly,
> >> all of which should be adapted in the future.  Everywhere there is a
> >> mention of VR_RANGE/VR_ANTI_RANGE in the compiler is suspect.  We should
> >> ideally be using intersect/union/may_contain_p/null_p, etc.
> >
> > null_p is a bad name btw, I just confused it with empty_p ... (which we have
> > as undefined_p).  contains_only_zero_p would be less confusing.
>
> Yes, a horrible name.  I noticed so as I debugged precisely this bit.
> How about zero_p?

Probably the same ambiguous connotation?  But yes, way better than null_p.

Richard.

> Aldy
Reply | Threaded
Open this post in threaded view
|

Re: Implement {get,set}_range_info() variants that work with value_range's

Aldy Hernandez-2


On 11/8/18 9:41 AM, Richard Biener wrote:

> On Thu, Nov 8, 2018 at 3:05 PM Aldy Hernandez <[hidden email]> wrote:
>>
>>
>>
>> On 11/8/18 8:59 AM, Richard Biener wrote:
>>> On Thu, Nov 8, 2018 at 12:52 PM Aldy Hernandez <[hidden email]> wrote:
>>>>
>>>> get/set_range_info() currently returns the extremes of a range.  I have
>>>> implemented overloaded variants that return a proper range.  In the
>>>> future we should use actual ranges throughout, and not depend on range
>>>> extremes, as depending on this behavior could causes us to lose precision.
>>>>
>>>> I am also including changes to size_must_be_zero_p() to show how we
>>>> should be using the range API, as opposed to performing error prone
>>>> ad-hoc calculations on ranges and anti-ranges.
>>>
>>> Yeah, I've been talking this all along but not being heard...
>>
>> My girlfriend says I don't listen.  It could be related.
>>
>>>> Martin, I'm not saying your code is wrong.  There are numerous other
>>>> places in the compiler where we manipulate ranges/anti-ranges directly,
>>>> all of which should be adapted in the future.  Everywhere there is a
>>>> mention of VR_RANGE/VR_ANTI_RANGE in the compiler is suspect.  We should
>>>> ideally be using intersect/union/may_contain_p/null_p, etc.
>>>
>>> null_p is a bad name btw, I just confused it with empty_p ... (which we have
>>> as undefined_p).  contains_only_zero_p would be less confusing.
>>
>> Yes, a horrible name.  I noticed so as I debugged precisely this bit.
>> How about zero_p?
>
> Probably the same ambiguous connotation?  But yes, way better than null_p.

Well...for starters I started with the nomenclature already in VRP which
was range_is_null.

Also, how is zero_p() ambiguous?  We want to know if the range is [0,
0].  That reads pretty obvious to me.

Aldy
Reply | Threaded
Open this post in threaded view
|

Re: Implement {get,set}_range_info() variants that work with value_range's

Jeff Law
On 11/8/18 7:44 AM, Aldy Hernandez wrote:

>
>
> On 11/8/18 9:41 AM, Richard Biener wrote:
>> On Thu, Nov 8, 2018 at 3:05 PM Aldy Hernandez <[hidden email]> wrote:
>>>
>>>
>>>
>>> On 11/8/18 8:59 AM, Richard Biener wrote:
>>>> On Thu, Nov 8, 2018 at 12:52 PM Aldy Hernandez <[hidden email]>
>>>> wrote:
>>>>>
>>>>> get/set_range_info() currently returns the extremes of a range.  I
>>>>> have
>>>>> implemented overloaded variants that return a proper range.  In the
>>>>> future we should use actual ranges throughout, and not depend on range
>>>>> extremes, as depending on this behavior could causes us to lose
>>>>> precision.
>>>>>
>>>>> I am also including changes to size_must_be_zero_p() to show how we
>>>>> should be using the range API, as opposed to performing error prone
>>>>> ad-hoc calculations on ranges and anti-ranges.
>>>>
>>>> Yeah, I've been talking this all along but not being heard...
>>>
>>> My girlfriend says I don't listen.  It could be related.
>>>
>>>>> Martin, I'm not saying your code is wrong.  There are numerous other
>>>>> places in the compiler where we manipulate ranges/anti-ranges
>>>>> directly,
>>>>> all of which should be adapted in the future.  Everywhere there is a
>>>>> mention of VR_RANGE/VR_ANTI_RANGE in the compiler is suspect.  We
>>>>> should
>>>>> ideally be using intersect/union/may_contain_p/null_p, etc.
>>>>
>>>> null_p is a bad name btw, I just confused it with empty_p ... (which
>>>> we have
>>>> as undefined_p).  contains_only_zero_p would be less confusing.
>>>
>>> Yes, a horrible name.  I noticed so as I debugged precisely this bit.
>>> How about zero_p?
>>
>> Probably the same ambiguous connotation?  But yes, way better than
>> null_p.
>
> Well...for starters I started with the nomenclature already in VRP which
> was range_is_null.
Probably due to using it for NULL pointer test elimination.  But yea, in
retrospect it's an awful name.

>
> Also, how is zero_p() ambiguous?  We want to know if the range is [0,
> 0].  That reads pretty obvious to me.
Seems reasonable to me.  THe question is will we push that name into all
the other places that use "null" when discussing ranges like ~[0,0] or
ranges that ultimately include [0,0].  Not all of these are in [E]VRP IIRC.

jeff
Reply | Threaded
Open this post in threaded view
|

Re: Implement {get,set}_range_info() variants that work with value_range's

Martin Sebor-2
In reply to this post by Aldy Hernandez-2
On 11/08/2018 04:52 AM, Aldy Hernandez wrote:

> get/set_range_info() currently returns the extremes of a range.  I have
> implemented overloaded variants that return a proper range.  In the
> future we should use actual ranges throughout, and not depend on range
> extremes, as depending on this behavior could causes us to lose precision.
>
> I am also including changes to size_must_be_zero_p() to show how we
> should be using the range API, as opposed to performing error prone
> ad-hoc calculations on ranges and anti-ranges.
>
> Martin, I'm not saying your code is wrong.  There are numerous other
> places in the compiler where we manipulate ranges/anti-ranges directly,
> all of which should be adapted in the future.  Everywhere there is a
> mention of VR_RANGE/VR_ANTI_RANGE in the compiler is suspect.  We should
> ideally be using intersect/union/may_contain_p/null_p, etc.
>
> OK pending another round of tests?  (As I had tested this patch along
> with a whole slew of other upcoming changes ;-)).

I'm all for simplification! :)

I'd love it if the API made it possible to write these expressions
using native operators and intuitive names, without having to worry
about details like what types they are represented in or how to
convert between them.  It would be great if a function like
size_must_be_zero_p could be coded along the lines similar to:

   return intersect (size, value_range (ssize_type_node) - 0) == 0;

It shouldn't have to twiddle bits to compute the wide_int value
of SSIZE_MAX or wrestle with convertibility warts by calling
build_int_cst and and wide_int_to_tree.

Martin

PS I think we should be able to get close to the syntax above
with a few value_range ctors:

   value_range::value_range (tree type_or_expression);

   template <class IntegerType>
   value_range::value_range (const IntegerType&);

a non-member minus operator to compute the difference:

   value_range operator- (const value_range&, const value_range&);

a non-member intersect function like this:

   value_range intersect (const value_range&, const value_range&);

and a non-member equality operator:

   bool operator== (const value_range&, const value_range&);

With the above, because a tree and any integer (including wide_int,
offset_int, and all the other flavors du jour) implicitly convert
to a value_range (possibly representing a single value),
the subtraction, intersection, and equality would apply to any and
all of those types as well without callers having to spell out or
even know what representation they're dealing with.
Reply | Threaded
Open this post in threaded view
|

Re: Implement {get,set}_range_info() variants that work with value_range's

Aldy Hernandez-2


On 11/8/18 4:47 PM, Martin Sebor wrote:

> On 11/08/2018 04:52 AM, Aldy Hernandez wrote:
>> get/set_range_info() currently returns the extremes of a range.  I have
>> implemented overloaded variants that return a proper range.  In the
>> future we should use actual ranges throughout, and not depend on range
>> extremes, as depending on this behavior could causes us to lose
>> precision.
>>
>> I am also including changes to size_must_be_zero_p() to show how we
>> should be using the range API, as opposed to performing error prone
>> ad-hoc calculations on ranges and anti-ranges.
>>
>> Martin, I'm not saying your code is wrong.  There are numerous other
>> places in the compiler where we manipulate ranges/anti-ranges directly,
>> all of which should be adapted in the future.  Everywhere there is a
>> mention of VR_RANGE/VR_ANTI_RANGE in the compiler is suspect.  We should
>> ideally be using intersect/union/may_contain_p/null_p, etc.
>>
>> OK pending another round of tests?  (As I had tested this patch along
>> with a whole slew of other upcoming changes ;-)).
>
> I'm all for simplification! :)
>
> I'd love it if the API made it possible to write these expressions
> using native operators and intuitive names, without having to worry
> about details like what types they are represented in or how to
> convert between them.  It would be great if a function like
> size_must_be_zero_p could be coded along the lines similar to:
>
>    return intersect (size, value_range (ssize_type_node) - 0) == 0;
>
> It shouldn't have to twiddle bits to compute the wide_int value
> of SSIZE_MAX or wrestle with convertibility warts by calling
> build_int_cst and and wide_int_to_tree.
>
> Martin
>
> PS I think we should be able to get close to the syntax above
> with a few value_range ctors:
>
>    value_range::value_range (tree type_or_expression);

Would this create the range for [type, type] or [-MIN, type] or what?

>
>    template <class IntegerType>
>    value_range::value_range (const IntegerType&);
>
> a non-member minus operator to compute the difference:
>
>    value_range operator- (const value_range&, const value_range&);
>
> a non-member intersect function like this:
>
>    value_range intersect (const value_range&, const value_range&);
>
> and a non-member equality operator:
>
>    bool operator== (const value_range&, const value_range&);
>
> With the above, because a tree and any integer (including wide_int,
> offset_int, and all the other flavors du jour) implicitly convert
> to a value_range (possibly representing a single value),
> the subtraction, intersection, and equality would apply to any and
> all of those types as well without callers having to spell out or
> even know what representation they're dealing with.

I like it.

Aldy
Reply | Threaded
Open this post in threaded view
|

Re: Implement {get,set}_range_info() variants that work with value_range's

Martin Sebor-2
On 11/09/2018 03:02 AM, Aldy Hernandez wrote:

>
>
> On 11/8/18 4:47 PM, Martin Sebor wrote:
>> On 11/08/2018 04:52 AM, Aldy Hernandez wrote:
>>> get/set_range_info() currently returns the extremes of a range.  I have
>>> implemented overloaded variants that return a proper range.  In the
>>> future we should use actual ranges throughout, and not depend on range
>>> extremes, as depending on this behavior could causes us to lose
>>> precision.
>>>
>>> I am also including changes to size_must_be_zero_p() to show how we
>>> should be using the range API, as opposed to performing error prone
>>> ad-hoc calculations on ranges and anti-ranges.
>>>
>>> Martin, I'm not saying your code is wrong.  There are numerous other
>>> places in the compiler where we manipulate ranges/anti-ranges directly,
>>> all of which should be adapted in the future.  Everywhere there is a
>>> mention of VR_RANGE/VR_ANTI_RANGE in the compiler is suspect.  We should
>>> ideally be using intersect/union/may_contain_p/null_p, etc.
>>>
>>> OK pending another round of tests?  (As I had tested this patch along
>>> with a whole slew of other upcoming changes ;-)).
>>
>> I'm all for simplification! :)
>>
>> I'd love it if the API made it possible to write these expressions
>> using native operators and intuitive names, without having to worry
>> about details like what types they are represented in or how to
>> convert between them.  It would be great if a function like
>> size_must_be_zero_p could be coded along the lines similar to:
>>
>>    return intersect (size, value_range (ssize_type_node) - 0) == 0;
>>
>> It shouldn't have to twiddle bits to compute the wide_int value
>> of SSIZE_MAX or wrestle with convertibility warts by calling
>> build_int_cst and and wide_int_to_tree.
>>
>> Martin
>>
>> PS I think we should be able to get close to the syntax above
>> with a few value_range ctors:
>>
>>    value_range::value_range (tree type_or_expression);
>
> Would this create the range for [type, type] or [-MIN, type] or what?

For a TYPE it would create [TYPE_MIN, TYPE_MAX].

>>    template <class IntegerType>
>>    value_range::value_range (const IntegerType&);
>>
>> a non-member minus operator to compute the difference:
>>
>>    value_range operator- (const value_range&, const value_range&);
>>
>> a non-member intersect function like this:
>>
>>    value_range intersect (const value_range&, const value_range&);
>>
>> and a non-member equality operator:
>>
>>    bool operator== (const value_range&, const value_range&);
>>
>> With the above, because a tree and any integer (including wide_int,
>> offset_int, and all the other flavors du jour) implicitly convert
>> to a value_range (possibly representing a single value),
>> the subtraction, intersection, and equality would apply to any and
>> all of those types as well without callers having to spell out or
>> even know what representation they're dealing with.
>
> I like it.

I'm glad!  When can we get it? ;-)  Seriously, if this is
something we would like to see I'm happy to help put it
together.

Martin