On Tue, May 25, 2021 at 12:53 PM Aldy Hernandez <al...@redhat.com> wrote: > > On 5/25/21 11:46 AM, Richard Biener wrote: > > On Tue, May 25, 2021 at 11:36 AM Aldy Hernandez <al...@redhat.com> wrote: > >> > >> > >> > >> On 5/25/21 10:57 AM, Richard Biener wrote: > >>> On Mon, May 24, 2021 at 6:44 PM Aldy Hernandez via Gcc-patches > >>> <gcc-patches@gcc.gnu.org> wrote: > >>>> > >>>> > >>>> > >>>> On 5/21/21 1:39 PM, Aldy Hernandez wrote: > >>>>> This patch provides a generic API for accessing global ranges. It is > >>>>> meant to replace get_range_info() and get_ptr_nonnull() with one > >>>>> common interface. It uses the same API as the ranger (class > >>>>> range_query), so there will now be one API for accessing local and > >>>>> global ranges alike. > >>>>> > >>>>> Follow-up patches will convert all users of get_range_info and > >>>>> get_ptr_nonnull to this API. > >>>>> > >>>>> For get_range_info, instead of: > >>>>> > >>>>> if (!POINTER_TYPE_P (TREE_TYPE (name)) && SSA_NAME_RANGE_INFO > >>>>> (name)) > >>>>> get_range_info (name, vr); > >>>>> > >>>>> You can now do: > >>>>> > >>>>> RANGE_QUERY (cfun)->range_of_expr (vr, name, [stmt]); > >>>> > >>>> BTW, we're not wed to the idea of putting the current range object in > >>>> cfun. The important thing is that the API is consistent across, not > >>>> where it lives. > >>> > >>> If the range object is specific for a function (and thus cannot handle > >>> multiple functions in IPA mode) then struct function looks like the > >>> correct > >>> place. Accessing that unconditionally via 'cfun' sounds bad though > >>> because > >>> that disallows use from IPA passes. > >> > >> The default range object can either be the "global_ranges" object > >> (get_range_info / get_ptr_nonnull wrapper) or a ranger. So, the former > >> is global in nature and not tied to any function, and the latter is tied > >> to the gimple IL in a function. > >> > >> What we want is a mechanism from which a pass can query the range of an > >> SSA (or expression) at a statement or edge, etc agnostically. If a > >> ranger is activated, use that, otherwise use the global information. > >> > >> For convenience we wanted a mechanism in which we didn't have to pass an > >> object between functions in a pass (be it a ranger or a struct > >> function). Back when I tried to convert some passes to a ranger, it was > >> a pain to pass a ranger object around, and having to pass struct > >> function would be similarly painful. > >> > >> ISTM, that most converted passes in this patchset already use cfun > >> throughout. For that matter, even the two IPA ones (ipa-fnsummary and > >> ipa-prop) use cfun throughout (by first calling push_cfun (node->decl)). > >> > >> How about I use fun if easily accessible in a pass, otherwise cfun? I'm > >> trying to avoid having to pass around a struct function in passes that > >> require surgery to do so (especially when they're already using cfun). > >> > >> Basically, we want minimal changes to clients for ease of use. > > > > I think it's fine to not fix "endusers", esp. if they already use 'cfun' > > and fixing would be a lot mechanical work. What we need to avoid > > is implicit uses of cfun via APIs we introduce because that makes > > a pass/API that is "cfun" clean, eventually even working on explicit > > struct function (and thus IPA safe) no longer so and depend on "cfun" > > without that being visible. > > Sounds reasonable. > > I have removed the use of cfun in get_global_range_query(), so no users > of GLOBAL_RANGE_QUERY will implicitly use it. > > I have verified that all uses of cfun are in passes that already have > cfun uses, or in the case of -Wrestrict in a pass that requires > shuffling things around to avoid cfun. Besides, -Wrestrict is not an > IPA pass. > > Note that there are 3 of uses of the following idiom: > > + if (cfun) > + RANGE_QUERY (cfun)->range_of_expr (vr, t); > + else > + GLOBAL_RANGE_QUERY->range_of_expr (vr, t); > > This is for three functions in fold-const.c, gimple-fold.c, and > tree-dfa.c that may or may not be called with a cfun. We'd like these > functions to pick up the current available range object. But if doing > so is problematic, I can change it to just use GLOBAL_RANGE_QUERY.
I think that's OK for now. It means that for hypothetical IPA passes using range queries that they'd get GLOBAL_RANGE_QUERY instead of the per-function one when running into those functions. But as you maybe figured there's quite some cfun/current_function_decl uses in "infrastructure" that has the same issue - we're just trying to reduce that. Richard. > Aldy
