Hi,
Richard Biener <rguent...@suse.de> writes:
> On Fri, 14 Jul 2023, Andrew MacLeod wrote:
>
>>
>> On 7/14/23 09:37, Richard Biener wrote:
>> > On Fri, 14 Jul 2023, Aldy Hernandez wrote:
>> >
>> >> I don't know what you're trying to accomplish here, as I haven't been
>> >> following the PR, but adding all these helper functions to the ranger
>> >> header
>> >> file seems wrong, especially since there's only one use of them. I see
>> >> you're
>> >> tweaking the irange API, adding helper functions to range-op (which is
>> >> only
>> >> for code dealing with implementing range operators for tree codes), etc
>> >> etc.
>> >>
>> >> If you need these helper functions, I suggest you put them closer to their
>> >> uses (i.e. wherever the match.pd support machinery goes).
>> > Note I suggested the opposite beacuse I thought these kind of helpers
>> > are closer to value-range support than to match.pd.
>>
>>
>> probably vr-values.{cc.h} and the simply_using_ranges paradigm would be the
>> most sensible place to put these kinds of auxiliary routines?
>>
>>
>> >
>> > But I take away from your answer that there's nothing close in the
>> > value-range machinery that answers the question whether A op B may
>> > overflow?
>>
>> we dont track it in ranges themselves. During calculation of a range we
>> obviously know, but propagating that generally when we rarely care doesn't
>> seem worthwhile. The very first generation of irange 6 years ago had an
>> overflow_p() flag, but it was removed as not being worth keeping. easier
>> to simply ask the question when it matters
>>
>> As the routines show, it pretty easy to figure out when the need arises so I
>> think that should suffice. At least for now,
>>
>> Should we decide we would like it in general, it wouldnt be hard to add to
>> irange. wi_fold() cuurently returns null, it could easily return a bool
>> indicating if an overflow happened, and wi_fold_in_parts and fold_range would
>> simply OR the results all together of the compoent wi_fold() calls. It would
>> require updating/audfiting a number of range-op entries and adding an
>> overflowed_p() query to irange.
>
> Ah, yeah - the folding APIs would be a good fit I guess. I was
> also looking to have the "new" helpers to be somewhat consistent
> with the ranger API.
>
> So if we had a fold_range overload with either an output argument
> or a flag that makes it return false on possible overflow that
> would work I guess? Since we have a virtual class setup we
> might be able to provide a default failing method and implement
> workers for plus and mult (as needed for this patch) as the need
> arises?
Thanks for your comments!
Here is a concern. The patterns in match.pd may be supported by
'vrp' passes. At that time, the range info would be computed (via
the value-range machinery) and cached for each SSA_NAME. In the
patterns, when range_of_expr is called for a capture, the range
info is retrieved from the cache, and no need to fold_range again.
This means the overflow info may also need to be cached together
with other range info. There may be additional memory and time
cost.
BR,
Jeff (Jiufu Guo)
>
> Thanks,
> Richard.
