Re: [PATCH V4] Optimize '(X - N * M) / N' to 'X / N - M' if valid

[Jiufu Guo <guojiufu@linux.ibm.com>]

Hi,

Richard Biener <rguenther@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.