From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ed1-x52d.google.com (mail-ed1-x52d.google.com [IPv6:2a00:1450:4864:20::52d]) by sourceware.org (Postfix) with ESMTPS id E74983894402 for ; Tue, 25 May 2021 11:06:11 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org E74983894402 Received: by mail-ed1-x52d.google.com with SMTP id o5so26682660edc.5 for ; Tue, 25 May 2021 04:06:11 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=5XSd2CnIgAEa3vJKDe9NlFQ801CIkYAKYQviI0oJxas=; b=HFLuMyfYm1JbdroVzotescN/7fZTH9wNWeZDoMd+45wo4KLlcst84bB7156s5/Szib tLirIpNdQ50U4FJRo4V5+TxYzjDJvnZr4mrBtlgM9J5iHiirT5MTuJdNO1Cq/78ibKoN 8fUVUoIAW6wDUC26PK3x9svvmCFzNxOp2EXeJpEj+N2b7wuAX1OxH8i0Q9Uni+F37B2Q d3LHckRiefqtzGUCVm6263Gxl6R0crZrDXkSx1NZbYKjk5aiT8ybelOWVEHiMOKhQL+l 6TVE6N8XEewE/x6Wb6rH8nEgJOmGmvXp1f35JNYTtjYaIW7zx9d8EdOfKkZQmpXq/NoD +/0g== X-Gm-Message-State: AOAM530Gd5410smPQqY7qTjk6vDKIJhPItiffZa7JeHgtqOg35C35eU6 apNbzRDEDxl/CDZCYMWhW8ufgvOOQUNa+tLin7I= X-Google-Smtp-Source: ABdhPJzEIM3k6ukuCvGPo3ZOl/ZW8LGGbiDCH7EbjJOT9I7Wjd0N4atHTOIWuBl7YQ0YGmm4GPnLTHg8xRIHqw/Pk7A= X-Received: by 2002:a05:6402:4256:: with SMTP id g22mr30367130edb.214.1621940770987; Tue, 25 May 2021 04:06:10 -0700 (PDT) MIME-Version: 1.0 References: <20210521113954.1148075-1-aldyh@redhat.com> <71bc0536-573b-a389-89f5-f13f5763d3df@redhat.com> <203d1cae-2844-7f58-a334-ce0e96cf52a2@redhat.com> In-Reply-To: From: Richard Biener Date: Tue, 25 May 2021 13:06:00 +0200 Message-ID: Subject: Re: [PATCH 1/5] Common API for accessing global and on-demand ranges. To: Aldy Hernandez Cc: GCC patches , Andrew MacLeod , Martin Sebor Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-2.7 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 25 May 2021 11:06:13 -0000 On Tue, May 25, 2021 at 12:53 PM Aldy Hernandez wrote: > > On 5/25/21 11:46 AM, Richard Biener wrote: > > On Tue, May 25, 2021 at 11:36 AM Aldy Hernandez 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 > >>> 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