Message ID | 20230326165447.43628-1-iain@sandoe.co.uk |
---|---|
State | New |
Headers |
Return-Path: <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 9C31C385840C for <patchwork@sourceware.org>; Sun, 26 Mar 2023 16:55:42 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 9C31C385840C DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1679849742; bh=w+cp6gBRtHQliSMxTc5b+2ClTzHXTTNoDsUaFmeCIRs=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:Cc:From; b=fC5mZJ7bagRjboitm0Eok4Qk7sPc16mnajg7wZbaS63+DEPjVgqYX/4sled++m7VK A/UWhJDZ9W+lKqOemIsFbBoRvmb33Uk85JsWxwr6OXnZs4iBern3a8gfDmhgC5zDQl 80EpgmVt/2K7Moz2xmWu6VigqwB8riA4F8jKNE54= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mail-pj1-x102b.google.com (mail-pj1-x102b.google.com [IPv6:2607:f8b0:4864:20::102b]) by sourceware.org (Postfix) with ESMTPS id 9C00E3858D39 for <gcc-patches@gcc.gnu.org>; Sun, 26 Mar 2023 16:54:57 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 9C00E3858D39 Received: by mail-pj1-x102b.google.com with SMTP id a16so5637968pjs.4 for <gcc-patches@gcc.gnu.org>; Sun, 26 Mar 2023 09:54:57 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1679849696; h=content-transfer-encoding:mime-version:reply-to:message-id:date :subject:cc:to:from:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=w+cp6gBRtHQliSMxTc5b+2ClTzHXTTNoDsUaFmeCIRs=; b=bcLhpPTX0rgNygYKG0CfLTT17P4ckaG5/z2KlyFPa/ViANp+eJIMkFX4P2T/R6DUng czpS2N3/hPUjqCXvSqmgYlAShN8ASRRwHoGZoPKzht4wUeQYzJepaRpOwssYv8J+4Kq6 IL/+BWKW/s1+uHDjIecpDERwJQufXtfSFxMvsC0LuaDJzvmkvRnprJvOKvyxjsH9ETM9 aGDdsw3Kk79VafjG1GR6NIGAxTXbfBP9b2aYQxD0pycXY045oD40n3vBet25e1fHenTJ x/4CY2S91/wOnP2b0bm7yyIXzaM2AxzwlM41/AuBxxx03PurxCgYVMx07/snQsXJ7HQU QYnw== X-Gm-Message-State: AAQBX9fksmbtselI3u/6wsO6zHMR2v7B1uJFfDNIKN6VU//W2i3CJzNU aD0/froB6tKs4Cbbnp6x3k5tIQWHNSs= X-Google-Smtp-Source: AKy350YekrjSU2TPlLz/cyeIoXtCmSVN+rR/I/GINbBaE8N3I2pxDirayCEOb5cC0U2Wbf8ZZ4Sj5Q== X-Received: by 2002:a17:90b:180e:b0:23e:f867:e0b with SMTP id lw14-20020a17090b180e00b0023ef8670e0bmr8766387pjb.49.1679849696546; Sun, 26 Mar 2023 09:54:56 -0700 (PDT) Received: from localhost.localdomain ([103.2.133.137]) by smtp.gmail.com with ESMTPSA id a12-20020a65640c000000b0050aeaf9f25csm16644626pgv.27.2023.03.26.09.54.53 (version=TLS1_3 cipher=TLS_CHACHA20_POLY1305_SHA256 bits=256/256); Sun, 26 Mar 2023 09:54:56 -0700 (PDT) X-Google-Original-From: Iain Sandoe <iain@sandoe.co.uk> To: gcc-patches@gcc.gnu.org Subject: [PATCH] c++, coroutines: Stabilize names of promoted slot vars [PR101118]. Date: Sun, 26 Mar 2023 22:24:47 +0530 Message-Id: <20230326165447.43628-1-iain@sandoe.co.uk> X-Mailer: git-send-email 2.37.1 (Apple Git-137.1) MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-7.3 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, RCVD_IN_BARRACUDACENTRAL, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) 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 <gcc-patches.gcc.gnu.org> List-Unsubscribe: <https://gcc.gnu.org/mailman/options/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=unsubscribe> List-Archive: <https://gcc.gnu.org/pipermail/gcc-patches/> List-Post: <mailto:gcc-patches@gcc.gnu.org> List-Help: <mailto:gcc-patches-request@gcc.gnu.org?subject=help> List-Subscribe: <https://gcc.gnu.org/mailman/listinfo/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=subscribe> From: Iain Sandoe via Gcc-patches <gcc-patches@gcc.gnu.org> Reply-To: iain@sandoe.co.uk Cc: Iain Sandoe <iains.gcc@gmail.com>, jason@redhat.com Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> |
Series |
c++, coroutines: Stabilize names of promoted slot vars [PR101118].
|
|
Commit Message
Iain Sandoe
March 26, 2023, 4:54 p.m. UTC
Tested on x86_64-darwin21, x86-64-linux-gnu
OK for trunk?
Iain
When we need to 'promote' a value (i.e. store it in the coroutine frame) it
is given a frame entry name. This was based on the DECL_UID for slot vars.
However, when LTO is used, the names from multiple TUs become visible at the
same time, and the DECL_UIDs usually differ between units. This leads to a
"ODR mismatch" warning for the frame type.
The fix here is to use a counter instead of the DECL_UID which makes a name
that is stable between TUs for each frame layout (one per coroutine func).
Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>
PR c++/101118
gcc/cp/ChangeLog:
* coroutines.cc: Add counter for promoted slot vars.
(flatten_await_stmt): Use slot vars counter instead of DECL_UID
to generate the frame entry name for promoted target expression
slot variables.
(morph_fn_to_coro): Reset the slot vars counter at the start of
each coroutine function.
---
gcc/cp/coroutines.cc | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
Comments
On Sun, Mar 26, 2023 at 6:55 PM Iain Sandoe via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > Tested on x86_64-darwin21, x86-64-linux-gnu > OK for trunk? > Iain > > When we need to 'promote' a value (i.e. store it in the coroutine frame) it > is given a frame entry name. This was based on the DECL_UID for slot vars. > However, when LTO is used, the names from multiple TUs become visible at the > same time, and the DECL_UIDs usually differ between units. This leads to a > "ODR mismatch" warning for the frame type. > > The fix here is to use a counter instead of the DECL_UID which makes a name > that is stable between TUs for each frame layout (one per coroutine func). I don't see how this avoids clashes across TUs? But are those VAR_DECLs not local anyway? I suppose -Wodr diagnostics for DECL_ARTIFICIAL vars are a bit on the edge as well ... Richard. > Signed-off-by: Iain Sandoe <iain@sandoe.co.uk> > > PR c++/101118 > > gcc/cp/ChangeLog: > > * coroutines.cc: Add counter for promoted slot vars. > (flatten_await_stmt): Use slot vars counter instead of DECL_UID > to generate the frame entry name for promoted target expression > slot variables. > (morph_fn_to_coro): Reset the slot vars counter at the start of > each coroutine function. > --- > gcc/cp/coroutines.cc | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc > index a2189e43db8..359a5bf46ff 100644 > --- a/gcc/cp/coroutines.cc > +++ b/gcc/cp/coroutines.cc > @@ -2726,6 +2726,11 @@ struct var_nest_node > var_nest_node *else_cl; > }; > > +/* This is used to make a stable, but unique-per-function, sequence number for > + each TARGET_EXPR slot variable that we 'promote' to a frame entry. It needs > + to be stable because the frame type is visible to LTO ODR checking. */ > +static unsigned tmpno = 0; > + > /* This is called for single statements from the co-await statement walker. > It checks to see if the statement contains any initializers for awaitables > and if any of these capture items by reference. */ > @@ -2889,7 +2894,7 @@ flatten_await_stmt (var_nest_node *n, hash_set<tree> *promoted, > tree init = t; > temps_used->add (init); > tree var_type = TREE_TYPE (init); > - char *buf = xasprintf ("D.%d", DECL_UID (TREE_OPERAND (init, 0))); > + char *buf = xasprintf ("T%03u", tmpno++); > tree var = build_lang_decl (VAR_DECL, get_identifier (buf), var_type); > DECL_ARTIFICIAL (var) = true; > free (buf); > @@ -4374,6 +4379,7 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer) > { > gcc_checking_assert (orig && TREE_CODE (orig) == FUNCTION_DECL); > > + tmpno = 0; > *resumer = error_mark_node; > *destroyer = error_mark_node; > if (!coro_function_valid_p (orig)) > -- > 2.37.1 (Apple Git-137.1) >
Hi Richard, (I’m away from my usual infrastructure, so responses could be slow and testing things could take a while). > On 27 Mar 2023, at 12:10, Richard Biener <richard.guenther@gmail.com> wrote: > > On Sun, Mar 26, 2023 at 6:55 PM Iain Sandoe via Gcc-patches > <gcc-patches@gcc.gnu.org> wrote: >> >> Tested on x86_64-darwin21, x86-64-linux-gnu >> OK for trunk? >> Iain >> >> When we need to 'promote' a value (i.e. store it in the coroutine frame) it >> is given a frame entry name. This was based on the DECL_UID for slot vars. >> However, when LTO is used, the names from multiple TUs become visible at the >> same time, and the DECL_UIDs usually differ between units. This leads to a >> "ODR mismatch" warning for the frame type. >> >> The fix here is to use a counter instead of the DECL_UID which makes a name >> that is stable between TUs for each frame layout (one per coroutine func). > > I don't see how this avoids clashes across TUs? But are those VAR_DECLs not > local anyway? The reported ODR issue is in the frame type (which is a structure) — it sees two frame layouts with the same types for each field but a different name for the entries that came from the promotion of the slot var (because I used the DECL_UID to generate the field name). > I suppose -Wodr diagnostics for DECL_ARTIFICIAL vars are a bit on the > edge as well ... These promoted vars get DECL_VALUE_EXPRs (and as noted above a name to assist in debugging) tying them to the frame entry, .. although I do agree that reporting warnings for compiler-internal stuff is definitely on the edge (ISTR seeing maybe unused reports against such too). Not sure if we have an easy way to tell that the frame type is an internal one tho. Perhaps that needs a DECL_ARTIFICAL - but would that not make it unavailable for debug? Iain > > Richard. > >> Signed-off-by: Iain Sandoe <iain@sandoe.co.uk> >> >> PR c++/101118 >> >> gcc/cp/ChangeLog: >> >> * coroutines.cc: Add counter for promoted slot vars. >> (flatten_await_stmt): Use slot vars counter instead of DECL_UID >> to generate the frame entry name for promoted target expression >> slot variables. >> (morph_fn_to_coro): Reset the slot vars counter at the start of >> each coroutine function. >> --- >> gcc/cp/coroutines.cc | 8 +++++++- >> 1 file changed, 7 insertions(+), 1 deletion(-) >> >> diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc >> index a2189e43db8..359a5bf46ff 100644 >> --- a/gcc/cp/coroutines.cc >> +++ b/gcc/cp/coroutines.cc >> @@ -2726,6 +2726,11 @@ struct var_nest_node >> var_nest_node *else_cl; >> }; >> >> +/* This is used to make a stable, but unique-per-function, sequence number for >> + each TARGET_EXPR slot variable that we 'promote' to a frame entry. It needs >> + to be stable because the frame type is visible to LTO ODR checking. */ >> +static unsigned tmpno = 0; >> + >> /* This is called for single statements from the co-await statement walker. >> It checks to see if the statement contains any initializers for awaitables >> and if any of these capture items by reference. */ >> @@ -2889,7 +2894,7 @@ flatten_await_stmt (var_nest_node *n, hash_set<tree> *promoted, >> tree init = t; >> temps_used->add (init); >> tree var_type = TREE_TYPE (init); >> - char *buf = xasprintf ("D.%d", DECL_UID (TREE_OPERAND (init, 0))); >> + char *buf = xasprintf ("T%03u", tmpno++); >> tree var = build_lang_decl (VAR_DECL, get_identifier (buf), var_type); >> DECL_ARTIFICIAL (var) = true; >> free (buf); >> @@ -4374,6 +4379,7 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer) >> { >> gcc_checking_assert (orig && TREE_CODE (orig) == FUNCTION_DECL); >> >> + tmpno = 0; >> *resumer = error_mark_node; >> *destroyer = error_mark_node; >> if (!coro_function_valid_p (orig)) >> -- >> 2.37.1 (Apple Git-137.1)
On Mon, Mar 27, 2023 at 8:58 AM Iain Sandoe <iain@sandoe.co.uk> wrote: > > Hi Richard, > (I’m away from my usual infrastructure, so responses could be slow and testing things > could take a while). > > > On 27 Mar 2023, at 12:10, Richard Biener <richard.guenther@gmail.com> wrote: > > > > On Sun, Mar 26, 2023 at 6:55 PM Iain Sandoe via Gcc-patches > > <gcc-patches@gcc.gnu.org> wrote: > >> > >> Tested on x86_64-darwin21, x86-64-linux-gnu > >> OK for trunk? > >> Iain > >> > >> When we need to 'promote' a value (i.e. store it in the coroutine frame) it > >> is given a frame entry name. This was based on the DECL_UID for slot vars. > >> However, when LTO is used, the names from multiple TUs become visible at the > >> same time, and the DECL_UIDs usually differ between units. This leads to a > >> "ODR mismatch" warning for the frame type. > >> > >> The fix here is to use a counter instead of the DECL_UID which makes a name > >> that is stable between TUs for each frame layout (one per coroutine func). > > > > I don't see how this avoids clashes across TUs? But are those VAR_DECLs not > > local anyway? > > The reported ODR issue is in the frame type (which is a structure) — it sees two > frame layouts with the same types for each field but a different name for the entries > that came from the promotion of the slot var (because I used the DECL_UID to generate > the field name). Ah, I see. If it's from the same TU then why do we generate two frame layouts with the same type in the first place? > > I suppose -Wodr diagnostics for DECL_ARTIFICIAL vars are a bit on the > > edge as well ... > > These promoted vars get DECL_VALUE_EXPRs (and as noted above a name to > assist in debugging) tying them to the frame entry, > > .. although I do agree that reporting warnings for compiler-internal stuff is definitely > on the edge (ISTR seeing maybe unused reports against such too). If the two layouts are used to access the same objects you might run into TBAA issues. But making them appear the same but still separate types won't help that issue (but -flto will "fix" it for you then) > Not sure if we have an easy way to tell that the frame type is an internal one tho. > Perhaps that needs a DECL_ARTIFICAL - but would that not make it unavailable > for debug? We have TYPE_ARTIFICIAL, artificial-ness and no-debug are generally separate (DECL_IGNORED for decls, but I don't think we have anything for types here). Richard. > > Iain > > > > > > Richard. > > > >> Signed-off-by: Iain Sandoe <iain@sandoe.co.uk> > >> > >> PR c++/101118 > >> > >> gcc/cp/ChangeLog: > >> > >> * coroutines.cc: Add counter for promoted slot vars. > >> (flatten_await_stmt): Use slot vars counter instead of DECL_UID > >> to generate the frame entry name for promoted target expression > >> slot variables. > >> (morph_fn_to_coro): Reset the slot vars counter at the start of > >> each coroutine function. > >> --- > >> gcc/cp/coroutines.cc | 8 +++++++- > >> 1 file changed, 7 insertions(+), 1 deletion(-) > >> > >> diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc > >> index a2189e43db8..359a5bf46ff 100644 > >> --- a/gcc/cp/coroutines.cc > >> +++ b/gcc/cp/coroutines.cc > >> @@ -2726,6 +2726,11 @@ struct var_nest_node > >> var_nest_node *else_cl; > >> }; > >> > >> +/* This is used to make a stable, but unique-per-function, sequence number for > >> + each TARGET_EXPR slot variable that we 'promote' to a frame entry. It needs > >> + to be stable because the frame type is visible to LTO ODR checking. */ > >> +static unsigned tmpno = 0; > >> + > >> /* This is called for single statements from the co-await statement walker. > >> It checks to see if the statement contains any initializers for awaitables > >> and if any of these capture items by reference. */ > >> @@ -2889,7 +2894,7 @@ flatten_await_stmt (var_nest_node *n, hash_set<tree> *promoted, > >> tree init = t; > >> temps_used->add (init); > >> tree var_type = TREE_TYPE (init); > >> - char *buf = xasprintf ("D.%d", DECL_UID (TREE_OPERAND (init, 0))); > >> + char *buf = xasprintf ("T%03u", tmpno++); > >> tree var = build_lang_decl (VAR_DECL, get_identifier (buf), var_type); > >> DECL_ARTIFICIAL (var) = true; > >> free (buf); > >> @@ -4374,6 +4379,7 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer) > >> { > >> gcc_checking_assert (orig && TREE_CODE (orig) == FUNCTION_DECL); > >> > >> + tmpno = 0; > >> *resumer = error_mark_node; > >> *destroyer = error_mark_node; > >> if (!coro_function_valid_p (orig)) > >> -- > >> 2.37.1 (Apple Git-137.1) >
Hi Richard > On 27 Mar 2023, at 12:48, Richard Biener <richard.guenther@gmail.com> wrote: > > On Mon, Mar 27, 2023 at 8:58 AM Iain Sandoe <iain@sandoe.co.uk> wrote: >> >> Hi Richard, >> (I’m away from my usual infrastructure, so responses could be slow and testing things >> could take a while). >> >>> On 27 Mar 2023, at 12:10, Richard Biener <richard.guenther@gmail.com> wrote: >>> >>> On Sun, Mar 26, 2023 at 6:55 PM Iain Sandoe via Gcc-patches >>> <gcc-patches@gcc.gnu.org> wrote: >>>> >>>> Tested on x86_64-darwin21, x86-64-linux-gnu >>>> OK for trunk? >>>> Iain >>>> >>>> When we need to 'promote' a value (i.e. store it in the coroutine frame) it >>>> is given a frame entry name. This was based on the DECL_UID for slot vars. >>>> However, when LTO is used, the names from multiple TUs become visible at the >>>> same time, and the DECL_UIDs usually differ between units. This leads to a >>>> "ODR mismatch" warning for the frame type. >>>> >>>> The fix here is to use a counter instead of the DECL_UID which makes a name >>>> that is stable between TUs for each frame layout (one per coroutine func). >>> >>> I don't see how this avoids clashes across TUs? But are those VAR_DECLs not >>> local anyway? >> >> The reported ODR issue is in the frame type (which is a structure) — it sees two >> frame layouts with the same types for each field but a different name for the entries >> that came from the promotion of the slot var (because I used the DECL_UID to generate >> the field name). > > Ah, I see. If it's from the same TU then why do we generate two frame > layouts with > the same type in the first place? They are different TUs. The frames are generated for coroutine types instantiated from templates declared in a (boost) header. (I do not see anything in the testcase header making stuff explicitily inline) AFAIR the rules this is OK ODR-use-wise …. >>> I suppose -Wodr diagnostics for DECL_ARTIFICIAL vars are a bit on the >>> edge as well ... >> >> These promoted vars get DECL_VALUE_EXPRs (and as noted above a name to >> assist in debugging) tying them to the frame entry, >> >> .. although I do agree that reporting warnings for compiler-internal stuff is definitely >> on the edge (ISTR seeing maybe unused reports against such too). > > If the two layouts are used to access the same objects you might run > into TBAA issues. > But making them appear the same but still separate types won't help that issue > (but -flto will "fix" it for you then) … but I wonder if I should be preventing LTO from doing this (perhaps my frame type needs a uniquing addition, and then we would not care about the differing). hmm… now I’m not sure that this patch is the right fix .. I’d welcome Jason’s take on this. >> Not sure if we have an easy way to tell that the frame type is an internal one tho. >> Perhaps that needs a DECL_ARTIFICAL - but would that not make it unavailable >> for debug? > > We have TYPE_ARTIFICIAL, artificial-ness and no-debug are generally separate > (DECL_IGNORED for decls, but I don't think we have anything for types here). OK .. I can see about adding that too - but probably not for 13.0 (unless that’s the right fix for the regression, I guess). Iain > > Richard. > >> >> Iain >> >> >>> >>> Richard. >>> >>>> Signed-off-by: Iain Sandoe <iain@sandoe.co.uk> >>>> >>>> PR c++/101118 >>>> >>>> gcc/cp/ChangeLog: >>>> >>>> * coroutines.cc: Add counter for promoted slot vars. >>>> (flatten_await_stmt): Use slot vars counter instead of DECL_UID >>>> to generate the frame entry name for promoted target expression >>>> slot variables. >>>> (morph_fn_to_coro): Reset the slot vars counter at the start of >>>> each coroutine function. >>>> --- >>>> gcc/cp/coroutines.cc | 8 +++++++- >>>> 1 file changed, 7 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc >>>> index a2189e43db8..359a5bf46ff 100644 >>>> --- a/gcc/cp/coroutines.cc >>>> +++ b/gcc/cp/coroutines.cc >>>> @@ -2726,6 +2726,11 @@ struct var_nest_node >>>> var_nest_node *else_cl; >>>> }; >>>> >>>> +/* This is used to make a stable, but unique-per-function, sequence number for >>>> + each TARGET_EXPR slot variable that we 'promote' to a frame entry. It needs >>>> + to be stable because the frame type is visible to LTO ODR checking. */ >>>> +static unsigned tmpno = 0; >>>> + >>>> /* This is called for single statements from the co-await statement walker. >>>> It checks to see if the statement contains any initializers for awaitables >>>> and if any of these capture items by reference. */ >>>> @@ -2889,7 +2894,7 @@ flatten_await_stmt (var_nest_node *n, hash_set<tree> *promoted, >>>> tree init = t; >>>> temps_used->add (init); >>>> tree var_type = TREE_TYPE (init); >>>> - char *buf = xasprintf ("D.%d", DECL_UID (TREE_OPERAND (init, 0))); >>>> + char *buf = xasprintf ("T%03u", tmpno++); >>>> tree var = build_lang_decl (VAR_DECL, get_identifier (buf), var_type); >>>> DECL_ARTIFICIAL (var) = true; >>>> free (buf); >>>> @@ -4374,6 +4379,7 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer) >>>> { >>>> gcc_checking_assert (orig && TREE_CODE (orig) == FUNCTION_DECL); >>>> >>>> + tmpno = 0; >>>> *resumer = error_mark_node; >>>> *destroyer = error_mark_node; >>>> if (!coro_function_valid_p (orig)) >>>> -- >>>> 2.37.1 (Apple Git-137.1) >>
On Mon, Mar 27, 2023 at 9:32 AM Iain Sandoe <iain@sandoe.co.uk> wrote: > > Hi Richard > > > On 27 Mar 2023, at 12:48, Richard Biener <richard.guenther@gmail.com> wrote: > > > > On Mon, Mar 27, 2023 at 8:58 AM Iain Sandoe <iain@sandoe.co.uk> wrote: > >> > >> Hi Richard, > >> (I’m away from my usual infrastructure, so responses could be slow and testing things > >> could take a while). > >> > >>> On 27 Mar 2023, at 12:10, Richard Biener <richard.guenther@gmail.com> wrote: > >>> > >>> On Sun, Mar 26, 2023 at 6:55 PM Iain Sandoe via Gcc-patches > >>> <gcc-patches@gcc.gnu.org> wrote: > >>>> > >>>> Tested on x86_64-darwin21, x86-64-linux-gnu > >>>> OK for trunk? > >>>> Iain > >>>> > >>>> When we need to 'promote' a value (i.e. store it in the coroutine frame) it > >>>> is given a frame entry name. This was based on the DECL_UID for slot vars. > >>>> However, when LTO is used, the names from multiple TUs become visible at the > >>>> same time, and the DECL_UIDs usually differ between units. This leads to a > >>>> "ODR mismatch" warning for the frame type. > >>>> > >>>> The fix here is to use a counter instead of the DECL_UID which makes a name > >>>> that is stable between TUs for each frame layout (one per coroutine func). > >>> > >>> I don't see how this avoids clashes across TUs? But are those VAR_DECLs not > >>> local anyway? > >> > >> The reported ODR issue is in the frame type (which is a structure) — it sees two > >> frame layouts with the same types for each field but a different name for the entries > >> that came from the promotion of the slot var (because I used the DECL_UID to generate > >> the field name). > > > > Ah, I see. If it's from the same TU then why do we generate two frame > > layouts with > > the same type in the first place? > > They are different TUs. > > The frames are generated for coroutine types instantiated from templates > declared in a (boost) header. > > (I do not see anything in the testcase header making stuff explicitily inline) > AFAIR the rules this is OK ODR-use-wise …. And I only now see the = 0 assignment to the static so I suppose the vars will indeed be consistent across compilations. Still you are building a VAR_DECL here, not a FIELD_DECL. I also wonder why you cannot simply use the original name of the TARGET_EXPR decl but have to invent a new one? If the TARGET_EXPR decl was nameless so can the new one be? > >>> I suppose -Wodr diagnostics for DECL_ARTIFICIAL vars are a bit on the > >>> edge as well ... > >> > >> These promoted vars get DECL_VALUE_EXPRs (and as noted above a name to > >> assist in debugging) tying them to the frame entry, > >> > >> .. although I do agree that reporting warnings for compiler-internal stuff is definitely > >> on the edge (ISTR seeing maybe unused reports against such too). > > > > If the two layouts are used to access the same objects you might run > > into TBAA issues. > > But making them appear the same but still separate types won't help that issue > > (but -flto will "fix" it for you then) > > … but I wonder if I should be preventing LTO from doing this (perhaps my frame > type needs a uniquing addition, and then we would not care about the differing). > > hmm… now I’m not sure that this patch is the right fix .. I’d welcome Jason’s take > on this. > > >> Not sure if we have an easy way to tell that the frame type is an internal one tho. > >> Perhaps that needs a DECL_ARTIFICAL - but would that not make it unavailable > >> for debug? > > > > We have TYPE_ARTIFICIAL, artificial-ness and no-debug are generally separate > > (DECL_IGNORED for decls, but I don't think we have anything for types here). > > OK .. I can see about adding that too - but probably not for 13.0 (unless that’s the > right fix for the regression, I guess). > > Iain > > > > > Richard. > > > >> > >> Iain > >> > >> > >>> > >>> Richard. > >>> > >>>> Signed-off-by: Iain Sandoe <iain@sandoe.co.uk> > >>>> > >>>> PR c++/101118 > >>>> > >>>> gcc/cp/ChangeLog: > >>>> > >>>> * coroutines.cc: Add counter for promoted slot vars. > >>>> (flatten_await_stmt): Use slot vars counter instead of DECL_UID > >>>> to generate the frame entry name for promoted target expression > >>>> slot variables. > >>>> (morph_fn_to_coro): Reset the slot vars counter at the start of > >>>> each coroutine function. > >>>> --- > >>>> gcc/cp/coroutines.cc | 8 +++++++- > >>>> 1 file changed, 7 insertions(+), 1 deletion(-) > >>>> > >>>> diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc > >>>> index a2189e43db8..359a5bf46ff 100644 > >>>> --- a/gcc/cp/coroutines.cc > >>>> +++ b/gcc/cp/coroutines.cc > >>>> @@ -2726,6 +2726,11 @@ struct var_nest_node > >>>> var_nest_node *else_cl; > >>>> }; > >>>> > >>>> +/* This is used to make a stable, but unique-per-function, sequence number for > >>>> + each TARGET_EXPR slot variable that we 'promote' to a frame entry. It needs > >>>> + to be stable because the frame type is visible to LTO ODR checking. */ > >>>> +static unsigned tmpno = 0; > >>>> + > >>>> /* This is called for single statements from the co-await statement walker. > >>>> It checks to see if the statement contains any initializers for awaitables > >>>> and if any of these capture items by reference. */ > >>>> @@ -2889,7 +2894,7 @@ flatten_await_stmt (var_nest_node *n, hash_set<tree> *promoted, > >>>> tree init = t; > >>>> temps_used->add (init); > >>>> tree var_type = TREE_TYPE (init); > >>>> - char *buf = xasprintf ("D.%d", DECL_UID (TREE_OPERAND (init, 0))); > >>>> + char *buf = xasprintf ("T%03u", tmpno++); > >>>> tree var = build_lang_decl (VAR_DECL, get_identifier (buf), var_type); > >>>> DECL_ARTIFICIAL (var) = true; > >>>> free (buf); > >>>> @@ -4374,6 +4379,7 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer) > >>>> { > >>>> gcc_checking_assert (orig && TREE_CODE (orig) == FUNCTION_DECL); > >>>> > >>>> + tmpno = 0; > >>>> *resumer = error_mark_node; > >>>> *destroyer = error_mark_node; > >>>> if (!coro_function_valid_p (orig)) > >>>> -- > >>>> 2.37.1 (Apple Git-137.1) > >> >
Hi Richard, > On 28 Mar 2023, at 11:58, Richard Biener <richard.guenther@gmail.com> wrote: > > On Mon, Mar 27, 2023 at 9:32 AM Iain Sandoe <iain@sandoe.co.uk> wrote: >> >> Hi Richard >> >>> On 27 Mar 2023, at 12:48, Richard Biener <richard.guenther@gmail.com> wrote: >>> >>> On Mon, Mar 27, 2023 at 8:58 AM Iain Sandoe <iain@sandoe.co.uk> wrote: >>>> >>>> Hi Richard, >>>> (I’m away from my usual infrastructure, so responses could be slow and testing things >>>> could take a while). >>>> >>>>> On 27 Mar 2023, at 12:10, Richard Biener <richard.guenther@gmail.com> wrote: >>>>> >>>>> On Sun, Mar 26, 2023 at 6:55 PM Iain Sandoe via Gcc-patches >>>>> <gcc-patches@gcc.gnu.org> wrote: >>>>>> >>>>>> Tested on x86_64-darwin21, x86-64-linux-gnu >>>>>> OK for trunk? >>>>>> Iain >>>>>> >>>>>> When we need to 'promote' a value (i.e. store it in the coroutine frame) it >>>>>> is given a frame entry name. This was based on the DECL_UID for slot vars. >>>>>> However, when LTO is used, the names from multiple TUs become visible at the >>>>>> same time, and the DECL_UIDs usually differ between units. This leads to a >>>>>> "ODR mismatch" warning for the frame type. >>>>>> >>>>>> The fix here is to use a counter instead of the DECL_UID which makes a name >>>>>> that is stable between TUs for each frame layout (one per coroutine func). >>>>> >>>>> I don't see how this avoids clashes across TUs? But are those VAR_DECLs not >>>>> local anyway? >>>> >>>> The reported ODR issue is in the frame type (which is a structure) — it sees two >>>> frame layouts with the same types for each field but a different name for the entries >>>> that came from the promotion of the slot var (because I used the DECL_UID to generate >>>> the field name). >>> >>> Ah, I see. If it's from the same TU then why do we generate two frame >>> layouts with >>> the same type in the first place? >> >> They are different TUs. >> >> The frames are generated for coroutine types instantiated from templates >> declared in a (boost) header. >> >> (I do not see anything in the testcase header making stuff explicitily inline) >> AFAIR the rules this is OK ODR-use-wise …. > > And I only now see the = 0 assignment to the static so I suppose the vars will > indeed be consistent across compilations. It’s incremented at the point of use. > Still you are building a VAR_DECL here, not a FIELD_DECL. I also wonder > why you cannot simply use the original name of the TARGET_EXPR decl > but have to invent a new one? If the TARGET_EXPR decl was nameless so > can the new one be? VAR_DECLs with names are added to the coroutine frame, (inlcuding those that correspond to ones the user wrote in their source) so the addition of the name was an easy mechanism to flag that the slot var needed to be added. (The use of a name based on the DECL_UID was also useful in debugging coroutine code-gen issues). Iain P.S. Yes, there are improvements that can/should be made to this area - and there are two optimisations in the pipeline for frame layout .. hopefully, I will get some cycles in GCC-14. >>>>> I suppose -Wodr diagnostics for DECL_ARTIFICIAL vars are a bit on the >>>>> edge as well ... >>>> >>>> These promoted vars get DECL_VALUE_EXPRs (and as noted above a name to >>>> assist in debugging) tying them to the frame entry, >>>> >>>> .. although I do agree that reporting warnings for compiler-internal stuff is definitely >>>> on the edge (ISTR seeing maybe unused reports against such too). >>> >>> If the two layouts are used to access the same objects you might run >>> into TBAA issues. >>> But making them appear the same but still separate types won't help that issue >>> (but -flto will "fix" it for you then) >> >> … but I wonder if I should be preventing LTO from doing this (perhaps my frame >> type needs a uniquing addition, and then we would not care about the differing). >> >> hmm… now I’m not sure that this patch is the right fix .. I’d welcome Jason’s take >> on this. >> >>>> Not sure if we have an easy way to tell that the frame type is an internal one tho. >>>> Perhaps that needs a DECL_ARTIFICAL - but would that not make it unavailable >>>> for debug? >>> >>> We have TYPE_ARTIFICIAL, artificial-ness and no-debug are generally separate >>> (DECL_IGNORED for decls, but I don't think we have anything for types here). >> >> OK .. I can see about adding that too - but probably not for 13.0 (unless that’s the >> right fix for the regression, I guess). >> >> Iain >> >>> >>> Richard. >>> >>>> >>>> Iain >>>> >>>> >>>>> >>>>> Richard. >>>>> >>>>>> Signed-off-by: Iain Sandoe <iain@sandoe.co.uk> >>>>>> >>>>>> PR c++/101118 >>>>>> >>>>>> gcc/cp/ChangeLog: >>>>>> >>>>>> * coroutines.cc: Add counter for promoted slot vars. >>>>>> (flatten_await_stmt): Use slot vars counter instead of DECL_UID >>>>>> to generate the frame entry name for promoted target expression >>>>>> slot variables. >>>>>> (morph_fn_to_coro): Reset the slot vars counter at the start of >>>>>> each coroutine function. >>>>>> --- >>>>>> gcc/cp/coroutines.cc | 8 +++++++- >>>>>> 1 file changed, 7 insertions(+), 1 deletion(-) >>>>>> >>>>>> diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc >>>>>> index a2189e43db8..359a5bf46ff 100644 >>>>>> --- a/gcc/cp/coroutines.cc >>>>>> +++ b/gcc/cp/coroutines.cc >>>>>> @@ -2726,6 +2726,11 @@ struct var_nest_node >>>>>> var_nest_node *else_cl; >>>>>> }; >>>>>> >>>>>> +/* This is used to make a stable, but unique-per-function, sequence number for >>>>>> + each TARGET_EXPR slot variable that we 'promote' to a frame entry. It needs >>>>>> + to be stable because the frame type is visible to LTO ODR checking. */ >>>>>> +static unsigned tmpno = 0; >>>>>> + >>>>>> /* This is called for single statements from the co-await statement walker. >>>>>> It checks to see if the statement contains any initializers for awaitables >>>>>> and if any of these capture items by reference. */ >>>>>> @@ -2889,7 +2894,7 @@ flatten_await_stmt (var_nest_node *n, hash_set<tree> *promoted, >>>>>> tree init = t; >>>>>> temps_used->add (init); >>>>>> tree var_type = TREE_TYPE (init); >>>>>> - char *buf = xasprintf ("D.%d", DECL_UID (TREE_OPERAND (init, 0))); >>>>>> + char *buf = xasprintf ("T%03u", tmpno++); >>>>>> tree var = build_lang_decl (VAR_DECL, get_identifier (buf), var_type); >>>>>> DECL_ARTIFICIAL (var) = true; >>>>>> free (buf); >>>>>> @@ -4374,6 +4379,7 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer) >>>>>> { >>>>>> gcc_checking_assert (orig && TREE_CODE (orig) == FUNCTION_DECL); >>>>>> >>>>>> + tmpno = 0; >>>>>> *resumer = error_mark_node; >>>>>> *destroyer = error_mark_node; >>>>>> if (!coro_function_valid_p (orig)) >>>>>> -- >>>>>> 2.37.1 (Apple Git-137.1) >>>> >>
Hi Richard > On 28 Mar 2023, at 12:27, Iain Sandoe <iain@sandoe.co.uk> wrote: >> On 28 Mar 2023, at 11:58, Richard Biener <richard.guenther@gmail.com> wrote: >> >> On Mon, Mar 27, 2023 at 9:32 AM Iain Sandoe <iain@sandoe.co.uk> wrote: >>> >>>> On 27 Mar 2023, at 12:48, Richard Biener <richard.guenther@gmail.com> wrote: >>>> >>>> On Mon, Mar 27, 2023 at 8:58 AM Iain Sandoe <iain@sandoe.co.uk> wrote: >>>> >>>> If the two layouts are used to access the same objects you might run >>>> into TBAA issues. >>>> But making them appear the same but still separate types won't help that issue >>>> (but -flto will "fix" it for you then) >>> >>> … but I wonder if I should be preventing LTO from doing this (perhaps my frame >>> type needs a uniquing addition, and then we would not care about the differing). >>> >>> hmm… now I’m not sure that this patch is the right fix .. I’d welcome Jason’s take >>> on this. taking another look to refresh my memory - the frame type is named for the function that it corresponds to. from the testcase example: e.g. if we have a function _Z3fn1v the frame type will be _Z3fn1v.Frame so - if that is incorrect, then we have deeper problems - since that would imply that the orignal function was also incorrectly named. So, ISTM that LTO is DTRT to merge the types (and presumably the functions) and the fix proposed is minimally invasive for this late in the cycle .. with an intent to revisit this in the future. Iain
On Tue, Mar 28, 2023 at 1:16 PM Iain Sandoe <iain@sandoe.co.uk> wrote: > > Hi Richard > > > On 28 Mar 2023, at 12:27, Iain Sandoe <iain@sandoe.co.uk> wrote: > >> On 28 Mar 2023, at 11:58, Richard Biener <richard.guenther@gmail.com> wrote: > >> > >> On Mon, Mar 27, 2023 at 9:32 AM Iain Sandoe <iain@sandoe.co.uk> wrote: > >>> > >>>> On 27 Mar 2023, at 12:48, Richard Biener <richard.guenther@gmail.com> wrote: > >>>> > >>>> On Mon, Mar 27, 2023 at 8:58 AM Iain Sandoe <iain@sandoe.co.uk> wrote: > > >>>> > >>>> If the two layouts are used to access the same objects you might run > >>>> into TBAA issues. > >>>> But making them appear the same but still separate types won't help that issue > >>>> (but -flto will "fix" it for you then) > >>> > >>> … but I wonder if I should be preventing LTO from doing this (perhaps my frame > >>> type needs a uniquing addition, and then we would not care about the differing). > >>> > >>> hmm… now I’m not sure that this patch is the right fix .. I’d welcome Jason’s take > >>> on this. > > taking another look to refresh my memory > - the frame type is named for the function that it corresponds to. > > from the testcase example: > e.g. if we have a function _Z3fn1v the frame type will be _Z3fn1v.Frame > so - if that is incorrect, then we have deeper problems - since that would imply that the > orignal function was also incorrectly named. > > So, ISTM that LTO is DTRT to merge the types (and presumably the functions) and the > fix proposed is minimally invasive for this late in the cycle .. with an intent to revisit this > in the future. Btw, I agree the fix is a strict improvement even without fully understanding the context. So it's OK from my side. Richard. > Iain >
On 3/26/23 12:54, Iain Sandoe wrote: > Tested on x86_64-darwin21, x86-64-linux-gnu > OK for trunk? > Iain > > When we need to 'promote' a value (i.e. store it in the coroutine frame) it > is given a frame entry name. This was based on the DECL_UID for slot vars. > However, when LTO is used, the names from multiple TUs become visible at the > same time, and the DECL_UIDs usually differ between units. This leads to a > "ODR mismatch" warning for the frame type. > > The fix here is to use a counter instead of the DECL_UID which makes a name > that is stable between TUs for each frame layout (one per coroutine func). > > Signed-off-by: Iain Sandoe <iain@sandoe.co.uk> > > PR c++/101118 > > gcc/cp/ChangeLog: > > * coroutines.cc: Add counter for promoted slot vars. > (flatten_await_stmt): Use slot vars counter instead of DECL_UID > to generate the frame entry name for promoted target expression > slot variables. > (morph_fn_to_coro): Reset the slot vars counter at the start of > each coroutine function. > --- > gcc/cp/coroutines.cc | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc > index a2189e43db8..359a5bf46ff 100644 > --- a/gcc/cp/coroutines.cc > +++ b/gcc/cp/coroutines.cc > @@ -2726,6 +2726,11 @@ struct var_nest_node > var_nest_node *else_cl; > }; > > +/* This is used to make a stable, but unique-per-function, sequence number for > + each TARGET_EXPR slot variable that we 'promote' to a frame entry. It needs > + to be stable because the frame type is visible to LTO ODR checking. */ > +static unsigned tmpno = 0; How about using temps_used->elements() for the index instead of a separate static counter? > /* This is called for single statements from the co-await statement walker. > It checks to see if the statement contains any initializers for awaitables > and if any of these capture items by reference. */ > @@ -2889,7 +2894,7 @@ flatten_await_stmt (var_nest_node *n, hash_set<tree> *promoted, > tree init = t; > temps_used->add (init); > tree var_type = TREE_TYPE (init); > - char *buf = xasprintf ("D.%d", DECL_UID (TREE_OPERAND (init, 0))); > + char *buf = xasprintf ("T%03u", tmpno++); > tree var = build_lang_decl (VAR_DECL, get_identifier (buf), var_type); > DECL_ARTIFICIAL (var) = true; > free (buf); > @@ -4374,6 +4379,7 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer) > { > gcc_checking_assert (orig && TREE_CODE (orig) == FUNCTION_DECL); > > + tmpno = 0; > *resumer = error_mark_node; > *destroyer = error_mark_node; > if (!coro_function_valid_p (orig))
Hi Jason, > On 30 Mar 2023, at 00:53, Jason Merrill <jason@redhat.com> wrote: > > On 3/26/23 12:54, Iain Sandoe wrote: >> Tested on x86_64-darwin21, x86-64-linux-gnu >> +/* This is used to make a stable, but unique-per-function, sequence number for >> + each TARGET_EXPR slot variable that we 'promote' to a frame entry. It needs >> + to be stable because the frame type is visible to LTO ODR checking. */ >> +static unsigned tmpno = 0; > > How about using temps_used->elements() for the index instead of a separate static counter? That’s a good idea (the only slightly weird effect is that the count does not start at 0, because we’ve added one or more entries by the time we get to produce a name, but that does not affect functionality). re-tested on x86_64-darwin21, as below, OK for trunk? thanks Iain === [PATCH] c++,coroutines: Stabilize names of promoted slot vars [PR101118]. When we need to 'promote' a value (i.e. store it in the coroutine frame) it is given a frame entry name. This was based on the DECL_UID for slot vars. However, when LTO is used, the names from multiple TUs become visible at the same time, and the DECL_UIDs usually differ between units. This leads to a "ODR mismatch" warning for the frame type. The fix here is to use the current promoted temporaries count to produce the name, this is stable between TUs and computed per coroutine. Signed-off-by: Iain Sandoe <iain@sandoe.co.uk> PR c++/101118 gcc/cp/ChangeLog: * coroutines.cc (flatten_await_stmt): Use the current count of promoted temporaries to build a unique name for the frame entries. --- gcc/cp/coroutines.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc index a2189e43db8..9f546db7437 100644 --- a/gcc/cp/coroutines.cc +++ b/gcc/cp/coroutines.cc @@ -2889,7 +2889,7 @@ flatten_await_stmt (var_nest_node *n, hash_set<tree> *promoted, tree init = t; temps_used->add (init); tree var_type = TREE_TYPE (init); - char *buf = xasprintf ("D.%d", DECL_UID (TREE_OPERAND (init, 0))); + char *buf = xasprintf ("T%03u", temps_used->elements()); tree var = build_lang_decl (VAR_DECL, get_identifier (buf), var_type); DECL_ARTIFICIAL (var) = true; free (buf); —
On 3/30/23 03:53, Iain Sandoe wrote: > Hi Jason, > >> On 30 Mar 2023, at 00:53, Jason Merrill <jason@redhat.com> wrote: >> >> On 3/26/23 12:54, Iain Sandoe wrote: >>> Tested on x86_64-darwin21, x86-64-linux-gnu > >>> +/* This is used to make a stable, but unique-per-function, sequence number for >>> + each TARGET_EXPR slot variable that we 'promote' to a frame entry. It needs >>> + to be stable because the frame type is visible to LTO ODR checking. */ >>> +static unsigned tmpno = 0; >> >> How about using temps_used->elements() for the index instead of a separate static counter? > > That’s a good idea (the only slightly weird effect is that the count does not start at 0, > because we’ve added one or more entries by the time we get to produce a name, but > that does not affect functionality). Perhaps promoted->elements() would be a better choice? OK either way. > re-tested on x86_64-darwin21, as below, > OK for trunk? > thanks > Iain > > === > > [PATCH] c++,coroutines: Stabilize names of promoted slot vars [PR101118]. > > When we need to 'promote' a value (i.e. store it in the coroutine frame) it > is given a frame entry name. This was based on the DECL_UID for slot vars. > However, when LTO is used, the names from multiple TUs become visible at the > same time, and the DECL_UIDs usually differ between units. This leads to a > "ODR mismatch" warning for the frame type. > > The fix here is to use the current promoted temporaries count to produce > the name, this is stable between TUs and computed per coroutine. > > Signed-off-by: Iain Sandoe <iain@sandoe.co.uk> > > PR c++/101118 > > gcc/cp/ChangeLog: > > * coroutines.cc (flatten_await_stmt): Use the current count of > promoted temporaries to build a unique name for the frame entries. > --- > gcc/cp/coroutines.cc | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc > index a2189e43db8..9f546db7437 100644 > --- a/gcc/cp/coroutines.cc > +++ b/gcc/cp/coroutines.cc > @@ -2889,7 +2889,7 @@ flatten_await_stmt (var_nest_node *n, hash_set<tree> *promoted, > tree init = t; > temps_used->add (init); > tree var_type = TREE_TYPE (init); > - char *buf = xasprintf ("D.%d", DECL_UID (TREE_OPERAND (init, 0))); > + char *buf = xasprintf ("T%03u", temps_used->elements()); > tree var = build_lang_decl (VAR_DECL, get_identifier (buf), var_type); > DECL_ARTIFICIAL (var) = true; > free (buf); > — > >
diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc index a2189e43db8..359a5bf46ff 100644 --- a/gcc/cp/coroutines.cc +++ b/gcc/cp/coroutines.cc @@ -2726,6 +2726,11 @@ struct var_nest_node var_nest_node *else_cl; }; +/* This is used to make a stable, but unique-per-function, sequence number for + each TARGET_EXPR slot variable that we 'promote' to a frame entry. It needs + to be stable because the frame type is visible to LTO ODR checking. */ +static unsigned tmpno = 0; + /* This is called for single statements from the co-await statement walker. It checks to see if the statement contains any initializers for awaitables and if any of these capture items by reference. */ @@ -2889,7 +2894,7 @@ flatten_await_stmt (var_nest_node *n, hash_set<tree> *promoted, tree init = t; temps_used->add (init); tree var_type = TREE_TYPE (init); - char *buf = xasprintf ("D.%d", DECL_UID (TREE_OPERAND (init, 0))); + char *buf = xasprintf ("T%03u", tmpno++); tree var = build_lang_decl (VAR_DECL, get_identifier (buf), var_type); DECL_ARTIFICIAL (var) = true; free (buf); @@ -4374,6 +4379,7 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer) { gcc_checking_assert (orig && TREE_CODE (orig) == FUNCTION_DECL); + tmpno = 0; *resumer = error_mark_node; *destroyer = error_mark_node; if (!coro_function_valid_p (orig))