On Sat, Jul 9, 2022 at 6:16 PM Jeff Law via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > > > On 7/6/2022 11:10 AM, Aldy Hernandez via Gcc-patches wrote: > > Currently SSA_NAME_RANGE_INFO only handles integer ranges, and loses > > half the precision in the process because its use of legacy > > value_range's. This patch rewrites all the SSA_NAME_RANGE_INFO > > (nonzero bits included) to use the recently contributed > > vrange_storage. With it, we'll be able to efficiently save any ranges > > supported by ranger in GC memory. Presently this will only be > > irange's, but shortly we'll add floating ranges and others to the mix. > > > > As per the discussion with the trailing_wide_ints adjustments and > > vrange_storage, we'll be able to save integer ranges with a maximum of > > 5 sub-ranges. This could be adjusted later if more sub-ranges are > > needed (unlikely). > > > > Since this is a behavior changing patch, I would like to take a few > > days for discussion, and commit early next week if all goes well. > > > > A few notes. > > > > First, we get rid of the SSA_NAME_ANTI_RANGE_P bit in the SSA_NAME > > since we store full resolution ranges. Perhaps it could be re-used > > for something else. > > > > The range_info_def struct is gone in favor of an opaque type handled > > by vrange_storage. It currently supports irange, but will support > > frange, prange, etc, in due time. > > > > From the looks of it, set_range_info was an update operation despite > > its name, as we improved the nonzero bits with each call, even though > > we clobbered the ranges. Presumably this was because doing a proper > > intersect of ranges lost information with the anti-range hack. We no > > longer have this limitation so now we formalize both set_range_info > > and set_nonzero_bits to an update operation. After all, we should > > never be losing information, but enhancing it whenever possible. This > > means, that if folks' finger-memory is not offended, as a follow-up, > > I'd like to rename set_nonzero_bits and set_range_info to update_*. > > > > I have kept the same global API we had in tree-ssanames.h, with the > > caveat that all set operations are now update as discussed above. > > > > There is a 2% performance penalty for evrp and a 3% penalty for VRP > > that is coincidentally in line with a previous improvement of the same > > amount in the vrange abstraction patchset. Interestingly, this > > penalty is mostly due to the wide int to tree dance we keep doing with > > irange and legacy. In a first draft of this patch where I was > > streaming trees directly, there was actually a small improvement > > instead. I hope to get some of the gain back when we move irange's to > > wide-ints, though I'm not in a hurry ;-). > > > > Tested and benchmarked on x86-64 Linux. I will also test on ppc64le > > before the final commit. > > > > Comments welcome. > > > > gcc/ChangeLog: > > > > * gimple-range.cc (gimple_ranger::export_global_ranges): Remove > > verification against legacy value_range. > > * tree-core.h (struct range_info_def): Remove. > > (struct irange_storage_slot): New. > > (struct tree_base): Remove SSA_NAME_ANTI_RANGE_P documentation. > > (struct tree_ssa_name): Add vrange_storage support. > > * tree-ssanames.cc (range_info_p): New. > > (range_info_fits_p): New. > > (range_info_alloc): New. > > (range_info_free): New. > > (range_info_get_range): New. > > (range_info_set_range): New. > > (set_range_info_raw): Remove. > > (set_range_info): Adjust to use vrange_storage. > > (set_nonzero_bits): Same. > > (get_nonzero_bits): Same. > > (duplicate_ssa_name_range_info): Remove overload taking > > value_range_kind. > > Rewrite tree overload to use vrange_storage. > > (duplicate_ssa_name_fn): Adjust to use vrange_storage. > > * tree-ssanames.h (struct range_info_def): Remove. > > (set_range_info): Adjust prototype to take vrange. > > * tree-vrp.cc (vrp_asserts::remove_range_assertions): Call > > duplicate_ssa_name_range_info. > > * tree.h (SSA_NAME_ANTI_RANGE_P): Remove. > > (SSA_NAME_RANGE_TYPE): Remove. > > * value-query.cc (get_ssa_name_range_info): Adjust to use > > vrange_storage. > > (update_global_range): Use int_range_max. > > (get_range_global): Remove as_a<irange>. > I'll be so happy once we don't have to keep doing the conversions > between the types. > > Anti-ranges no more!
Yeah, it took a little longer than the 6 weeks Andrew had estimated originally :-P. Note that anti range kinda sorta still exist in two forms: a) If you use value_range, as it still uses the legacy stuff underneath. But any new consumers (evrp, DOM, etc), all pass an int_range<N> or an int_range_max, so anyone who cares about ranges should never see an anti range. Later this cycle value_range will be typedefed to what is now Value_Range, which is an infinite precision range that works for all types the ranger supports. So anti-ranges here will die a quick death. b) There are some passes which still use the deprecated irange::kind(). This method will return VR_ANTI_RANGE if the range looks like this [-MIN, 123][567,+MAX]. But kind() is just a convenience function so that passes that have yet to be converted can still pretend they see anti-ranges. Underneath a non-legacy irange has no concept of an anti-range. Currently, I see the following passes still using the anti-range nonsense: gimple-array-bounds.cc gimple-ssa-warn-restrict.cc ipa-fnsummary.cc ipa-prop.cc pointer-query.cc tree-ssa-strlen.cc I don't understand the ipa-* stuff, so I never touched it. OTOH, the middle end warnings always break when you improve ranges so I left them alone. Aldy > > I've got no real concerns here. So unless someone objects, your plan is OK. > > jeff >
