From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ed1-x532.google.com (mail-ed1-x532.google.com [IPv6:2a00:1450:4864:20::532]) by sourceware.org (Postfix) with ESMTPS id B7E6B3892463 for ; Mon, 8 Feb 2021 10:37:47 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org B7E6B3892463 Received: by mail-ed1-x532.google.com with SMTP id s3so17440349edi.7 for ; Mon, 08 Feb 2021 02:37:47 -0800 (PST) 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=YbvI24t1oSC7zugX6VGy4XHnlZGBaVrkVi/wi6xGc7g=; b=o9pgieVSumVRK5xSFLFkyXY6BA54yrTFc6vw7Upf/2ZUNLnSdJk3t/k19ZUGxL4y+3 nOD/IbD9DhanpP91qv4VR8htxtaaQsY1wZHOiOpeQTa8YCVWfvbCXyrXLl7gHh7aeqjj 4fcuOvSPtSRTCbzoe+K/8CJJ986l2zJV+5hT4MSsN2jXH0+EX2Mwdh9r24YxSoO3nOZx TX+8/ZCzu3HSq4EppVCMeasjj7HW1FyDakm5TaLbyeMBo4m3yVm+tX1PcvmRAHNPDdHL jJ0IqJeeaJau5ookusXzm43FbasTOsPDILP4sse85Uagto1h3vtrG6jR8HU9PDCph+LY HAcA== X-Gm-Message-State: AOAM530Kt9pq8yGFvyzd1U3VyCwTsNpPpTBCpwYYUBwvzKpFpjvqP2qX ElwmTo3D0FOukoDC5DJoW3JlkO20GRHgZLJGUYM= X-Google-Smtp-Source: ABdhPJx60EREkGQD1N/7TG2s3loip+qg+s4r+SI1qyq0Y99Bz7yKetPtGmL2P2+omRp/yKenHhhFaoEfsJF/lure4WI= X-Received: by 2002:aa7:c64a:: with SMTP id z10mr16324257edr.61.1612780666497; Mon, 08 Feb 2021 02:37:46 -0800 (PST) MIME-Version: 1.0 References: <0f27b6c3-f6ab-ec4f-52c6-6e544684f751@gmail.com> <729fe3f8-bf64-7c9a-9f1f-29d60e28bd45@gmail.com> <42b07e33-87de-2ead-2f29-35878373a910@gmail.com> <427adbd4-e128-b5bd-9c89-2399bbb7e2c3@gmail.com> <0542d4d0-235a-912b-4606-9945f5e78dc5@gmail.com> In-Reply-To: <0542d4d0-235a-912b-4606-9945f5e78dc5@gmail.com> From: Richard Biener Date: Mon, 8 Feb 2021 11:37:35 +0100 Message-ID: Subject: Re: [PATCH] document BLOCK_ABSTRACT_ORIGIN et al. To: Martin Sebor Cc: Jeff Law , Jakub Jelinek , gcc-patches Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-3.9 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: Mon, 08 Feb 2021 10:37:50 -0000 On Sat, Feb 6, 2021 at 8:11 PM Martin Sebor wrote: > > On 2/4/21 1:48 AM, Richard Biener wrote: > > On Wed, Feb 3, 2021 at 6:12 PM Martin Sebor wrote: > >> > >> On 2/3/21 5:01 AM, Richard Biener wrote: > >>> On Mon, Feb 1, 2021 at 5:20 PM Martin Sebor wrote: > >>>> > >>>> I have pushed the tree.h comments in g:6a2053773b8. I will wait > >>>> for an approval of the changes to the manual. > >>> > >>> Sorry for not looking earlier. > >> > >> Sorry, I thought you were fine with the text after your first review. > >> I'll adjust the tree.h comments when we're done, though I'd like to > >> think the example in the manual will do a lot more to help make it > >> clear than the comments in tree.h can. > >> > >>> > >>> +/* The scope enclosing the scope NODE, or FUNCTION_DECL for the "outermost" > >>> + function scope. Inlined functions are chained by this so that given > >>> + expression E and its TREE_BLOCK(E) B, BLOCK_SUPERCONTEXT(B) is the scope > >>> + in which E has been made or into which E has been inlined. */ > >>> > >>> I can't really understand what you are trying to say with the second > >>> sentence. There's > >>> nothing really special about BLOCK_SUPERCONTEXT and inlines so I believe this > >>> sentence only adds confusion. > >> > >> The sentence explains how SUPERCONTEXT chains inlined blocks. In > >> the manual diff I show an example: > >> > >> void f0 (char *p, int n) { memset (p, 1, n); } > >> > >> void f1 (char *p, int n) { f0 (p + 1, n + 1); } > >> > >> void f2 (char *p, int n) { f1 (p + 1, n + 1); } > >> > >> int a[6]; > >> void f3 (char *p, int n) { f2 (a, 3); } > >> > >> The blocks for all calls inlined into f3 are chained like so: > >> > >> CALL_EXPR: memset E > >> > >> BLOCK #13 <--+ TREE_BLOCK (E) > >> +-- SUPERCONTEXT: BLOCK #12 | > >> | ABSTRACT_ORIGIN: BLOCK #0 --+ > >> | | | > >> +-> BLOCK #12 (f1) <--|-+ | > >> +-- SUPERCONTEXT: BLOCK #10 | | | > >> | SUBBLOCKS: BLOCK #13 --|-| | > >> | ABSTRACT_ORIGIN: f0 ---+ | | > >> | | | > >> +-> BLOCK #10 (f2) <-+ | | > >> +--- SUPERCONTEXT: BLOCK #8 | | | > >> | SUBBLOCKS: BLOCK #12 ---|-| | > >> | ABSTRACT_ORIGIN: f1 ------+ | > >> | | | > >> +-> BLOCK #8 (f3) | | > >> +---- SUPERCONTEXT: BLOCK #0 | | > >> | SUBBLOCKS: BLOCK #10 --| | > >> | ABSTRACT_ORIGIN: f2 ---+ | > >> | | > >> +-> BLOCK #0 (f3) <---------------+ > >> SUPERCONTEXT: f3 > >> SUBBLOCKS: BLOCK #8 > >> > >> Does the following sound better? (Dropping the "in which E has been > >> made.") > >> > >> Inlined functions are chained by this so that given expression E > >> and its TREE_BLOCK(E) B, BLOCK_SUPERCONTEXT(B) is the scope into > >> which E has been inlined. > > > > Oh, I see what you mean. But this is misleading for the case where f0 > > has any blocks: > > > > f0 (..) { { int tem; { memset (..); } } if () { }... } > > > > because BLOCK_SUPERCONTEXT is simply the parent scope ( { int tem; ... } ) and > > not yet the artifical scope we generate to wrap f0. > > I haven't seen those scopes in my (admittedly simplistic) tests and > based on the internals manual have been assuming most lexical scopes > are removed during gimplification: > > Additionally, all the control structures used in GENERIC are lowered > into conditional jumps, lexical scopes are removed and exception > regions are converted into an on the side exception region tree. > > Is the above wrong or inaccurate? Do all nested scope not get removed? The scopes are not present in the GIMPLE IL but the lexical scope tree is still there in a function decls DECL_INITIAL. What happens is that scopes are gone semantically since all automatic vars get promoted to function scope by GIMPLE but we preserve (some) out-of-scope going by inserting CLOBBER instructions into the IL (there's nothing similar at the point the vars originally went live - liveness analysis has to be done for this and the compiler can freely move defs outside of the original scope). > > To figure the > > scope a block was > > inlined to you'd have to do sth like > > > > b = TREE_BLOCK (E); > > gcc_assert (BLOCK_ABSTRACT_ORIGIN (b)); // it was inlined > > while (!inlined_function_outer_scope_p (b)) > > b = BLOCK_SUPERCONTEXT (b); > > now BLOCK_SUPERCONTEXT (b) is the block the function containing E was > > inlined to. > > > > So again, I think tree.h is not the place to document this. There > > BLOCK_SUPERCONTEXT > > should simply say it's pointing to the parent BLOCK. > > I can simplify the text but want to make it clear it doesn't > necessarily point to a BLOCK but can also point to a FUNCTION_DECL. > So how about: > > BLOCK_SUPERCONTEXT > Either a BLOCK of the enclosing scope or FUNCTION_DECL for > the "outermost" function scope. In the middle end used to chain > scopes of functions inlined into their callers. */ Please remove the last sentence. There's nothing different in FE vs. middle-end about BLOCK_SUPERCONTEXT. It's BLOCK_ABSTRACT_ORIGIN where inlining comes into play. > > > > In the texi documentation I'd separate out the representation of > > inlines and clones, > > eventually put it on the BLOCK_ABSTRACT_ORIGIN documentation. > > > > [I did not review the texi part yet - I mainly want to avoid people > > being even more > > confused about the tree.h comments] > > I take your point that sometimes less is more. But from my personal > experience with these macros, I'd be surprised if the text that's > there now could be worse than having no comments at all. But wrong or misleading comments are worse than no comments. > > > >>> #define BLOCK_SUPERCONTEXT(NODE) (BLOCK_CHECK (NODE)->block.supercontext) > >>> +/* Points to the next scope at the same level of nesting as scope NODE. */ > >>> #define BLOCK_CHAIN(NODE) (BLOCK_CHECK (NODE)->block.chain) > >>> +/* A BLOCK, or FUNCTION_DECL of the function from which a block has been > >>> + inlined. > >>> > >>> ... from which a block has been ultimatively copied for example by inlining. > >>> > >>> [clones also will have abstract origins] > > Okay, how's this: > > /* A BLOCK, or FUNCTION_DECL of the function from which a block has been > copied by inlining or cloning. */ > #define BLOCK_ABSTRACT_ORIGIN(NODE) (BLOCK_CHECK > (NODE)->block.abstract_origin) Sounds good. > >>> > >>> In a scope immediately enclosing an inlined leaf expression, > >>> + points to the outermost scope into which it has been inlined (thus > >>> + bypassing all intermediate BLOCK_SUPERCONTEXTs). */ > >>> > >>> ? > >> > >> This describes the long arrow on the right, pointing Block #13's > >> ABSTRACT_ORIGIN down to Block #0. All the other AO's point down > >> to the next/enclosing block (arrows on the left). I didn't expect > >> this when I first worked with the blocks so it seemed like > >> an important detail to mention. > >> > >>> > >>> Maybe: An inlined function is represented by a scope with > >>> BLOCK_ABSTRACT_ORIGIN being the FUNCTION_DECL of the inlined function > >>> containing the inlined functions scope tree as children. All abstract origins > >>> are ultimate, that is BLOCK_ABSTRACT_ORIGIN(NODE) > >>> == BLOCK_ABSTRACT_ORIGIN(BLOCK_ABSTRACT_ORIGIN (NODE)). > >> > >> The first sentence sounds good to me as far as it goes but it > >> doesn't capture the long arrow above. (By children I assume you > >> mean SUBBLOCKS, correct?) > >> > >> I don't follow what you're trying to say in the second sentence. > >> The equality isn't true for Block #0 whose AO is null. It also > >> isn't true for Block #12 and the others whose AO is a DECL, not > >> a block. > >> > >> What do you mean by "ultimate" in plain English? > > > > Ultimate in the sense dwarf2out uses it. DWARF wants to refer to > > the abstract copy. Originally (I think till GCC 9) BLOCK_ABSTRACT_ORIGIN > > of a function inlined that had functions inlined into it pointed to the > > inlined block of the inner inline but now it points to the original > > outline copy BLOCK of the inner inline (uh, can you parse that?). > > That simplifies things and is all we need. > > I *think* I understand what you're saying about how it's set up now, > but I had to refer to the diagram above to help. You're referring > to the arrow below, f2's AO pointing to f1's block (and > the corresponding arrow from f3's AO to f2's block: > > BLOCK #12 (f1) <----+ > SUPERCONTEXT: BLOCK #10 | > SUBBLOCKS: BLOCK #13 | > ABSTRACT_ORIGIN: f0 | > | > BLOCK #10 (f2) | > SUPERCONTEXT: BLOCK #8 | > SUBBLOCKS: BLOCK #12 | > ABSTRACT_ORIGIN: f1 -----+ > > FWIW, a relationship between two interior parts of a larger whole > is not something I would understand by the word ultimate. A couple > of synonyms for "ultimate" are "last" and "final" and f1 is neither. > I would call f3 the ultimate origin the three inlined calls, and > that matches AO only in Block #13, but not in any others. But the diagram is only correct until GCC 9 (if I remembered correctly the point I changed it). Currently f2 abstract origin refers to the abstract origin of f1 (so f0). Otherwise the invariant I gave doesn't hold. Abstract origins are best understood in the context of generated DWARF for DW_TAG_lexical_block DW_AT_abstract_origins and the way you'd expect those for DW_TAG_inlined_subroutine DIEs (those we generate for the inlined_function_outer_scope_p blocks - the BLOCKs with FUNCTION_DECL abstract origin). > >> FWIW, if I were to try to explain it using the example I'd say > >> only Block #13's AO is "ultimate:" it points down in the diagram > >> to the block of the function into which the expression has > >> ultimately been inlined. The AO's of all the other intervening > >> inlined blocks are the DECLs of the inlined callees (up-pointing > >> arrows); they don't look ultimate to me in this sense. > >> > >> But however this is phrased I suspect it won't be perfectly clear > >> without an example or a picture. > > > > Which means giving partial info in tree.h isn't useful but confusing. > > I don't think that's true. Most comments in tree.h are only partial > and don't fully describe every fine detail of whatever they document. > They may not be enough for using the thing they describe properly > and safely in every instance but they're often good enough to read > the code and what what it does. BLOCK_ABSTRACT_ORIGIN or > BLOC_SUPERCONTEXT without any comment at all isn't good enough > for even that. But wrong info gives people the idea they can use it wrongly without investigating or asking. Richard. > Anyway, thanks for continuing to help with this. Attached are > the tweaks to tree.h. I'll post an update to the manual separately. > > Martin > > > > > Richard. > > > >> > >> Martin > >> > >>> > >>> #define BLOCK_ABSTRACT_ORIGIN(NODE) (BLOCK_CHECK (NODE)->block.abstract_origin) > >>> > >>> > >>>> On 1/27/21 5:54 PM, Martin Sebor wrote: > >>>>> Attached is an updated patch for both tree.h and the internals manual > >>>>> documenting the most important BLOCK_ macros and what they represent. > >>>>> > >>>>> On 1/21/21 2:52 PM, Martin Sebor wrote: > >>>>>> On 1/18/21 6:25 AM, Richard Biener wrote: > >>>>>>>> PS Here are my notes on the macros and the two related functions: > >>>>>>>> > >>>>>>>> BLOCK: Denotes a lexical scope. Contains BLOCK_VARS of variables > >>>>>>>> declared in it, BLOCK_SUBBLOCKS of scopes nested in it, and > >>>>>>>> BLOCK_CHAIN pointing to the next BLOCK. Its BLOCK_SUPERCONTEXT > >>>>>>>> point to the BLOCK of the enclosing scope. May have > >>>>>>>> a BLOCK_ABSTRACT_ORIGIN and a BLOCK_SOURCE_LOCATION. > >>>>>>>> > >>>>>>>> BLOCK_SUPERCONTEXT: The scope of the enclosing block, or FUNCTION_DECL > >>>>>>>> for the "outermost" function scope. Inlined functions are chained by > >>>>>>>> this so that given expression E and its TREE_BLOCK(E) B, > >>>>>>>> BLOCK_SUPERCONTEXT(B) is the scope (BLOCK) in which E has been made > >>>>>>>> or into which E has been inlined. In the latter case, > >>>>>>>> > >>>>>>>> BLOCK_ORIGIN(B) evaluates either to the enclosing BLOCK or to > >>>>>>>> the enclosing function DECL. It's never null. > >>>>>>>> > >>>>>>>> BLOCK_ABSTRACT_ORIGIN(B) is the FUNCTION_DECL of the function into > >>>>>>>> which it has been inlined, or null if B is not inlined. > >>>>>>> > >>>>>>> It's the BLOCK or FUNCTION it was inlined _from_, not were it was > >>>>>>> inlined to. > >>>>>>> It's the "ultimate" source, thus the abstract copy of the block or > >>>>>>> function decl > >>>>>>> (for the outermost scope, aka inlined_function_outer_scope_p). It > >>>>>>> corresponds > >>>>>>> to what you'd expect for the DWARF abstract origin. > >>>>>> > >>>>>> Thanks for the correction! It's just the "innermost" block that > >>>>>> points to the "ultimate" destination into which it's been inlined. > >>>>>> > >>>>>>> > >>>>>>> BLOCK_ABSTRACT_ORIGIN can be NULL (in case it isn't an inline instance). > >>>>>>> > >>>>>>>> BLOCK_ABSTRACT_ORIGIN: A BLOCK, or FUNCTION_DECL of the function > >>>>>>>> into which a block has been inlined. In a BLOCK immediately enclosing > >>>>>>>> an inlined leaf expression points to the outermost BLOCK into which it > >>>>>>>> has been inlined (thus bypassing all intermediate BLOCK_SUPERCONTEXTs). > >>>>>>>> > >>>>>>>> BLOCK_FRAGMENT_ORIGIN: ??? > >>>>>>>> BLOCK_FRAGMENT_CHAIN: ??? > >>>>>>> > >>>>>>> that's for scope blocks split by hot/cold partitioning and only > >>>>>>> temporarily > >>>>>>> populated. > >>>>>> > >>>>>> Thanks, I now see these documented in detail in tree.h. > >>>>>> > >>>>>>> > >>>>>>>> bool inlined_function_outer_scope_p(BLOCK) [tree.h] > >>>>>>>> Returns true if a BLOCK has a source location. > >>>>>>>> True for all but the innermost (no SUBBLOCKs?) and outermost blocks > >>>>>>>> into which an expression has been inlined. (Is this always true?) > >>>>>>>> > >>>>>>>> tree block_ultimate_origin(BLOCK) [tree.c] > >>>>>>>> Returns BLOCK_ABSTRACT_ORIGIN(BLOCK), AO, after asserting that > >>>>>>>> (DECL_P(AO) && DECL_ORIGIN(AO) == AO) || BLOCK_ORIGIN(AO) == AO). > >>>>>> > >>>>>> The attached diff adds the comments above to tree.h. > >>>>>> > >>>>>> I looked for a good place in the manual to add the same text but I'm > >>>>>> not sure. Would the Blocks @subsection in generic.texi be appropriate? > >>>>>> > >>>>>> Martin > >>>>> > >>>>> > >>>> > >> >