public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jason Merrill <jason@redhat.com>
To: Iain Sandoe <iain@sandoe.co.uk>
Cc: GCC Patches <gcc-patches@gcc.gnu.org>
Subject: Re: [PATCH] c++,coroutines: Stabilize names of promoted slot vars [PR101118].
Date: Thu, 30 Mar 2023 16:50:47 -0400	[thread overview]
Message-ID: <a3681f1a-f639-b3fc-5206-066ed1515997@redhat.com> (raw)
In-Reply-To: <86FF61EC-F584-4249-A5A6-2EAE883FC4D2@sandoe.co.uk>

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);
> —
> 
> 


      reply	other threads:[~2023-03-30 20:50 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-26 16:54 Iain Sandoe
2023-03-27  6:40 ` [PATCH] c++, coroutines: " Richard Biener
2023-03-27  6:58   ` Iain Sandoe
2023-03-27  7:18     ` Richard Biener
2023-03-27  7:32       ` Iain Sandoe
2023-03-28  6:28         ` Richard Biener
2023-03-28  6:57           ` Iain Sandoe
2023-03-28 11:16             ` Iain Sandoe
2023-03-28 11:27               ` Richard Biener
2023-03-29 19:23 ` [PATCH] c++,coroutines: " Jason Merrill
2023-03-30  7:53   ` Iain Sandoe
2023-03-30 20:50     ` Jason Merrill [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a3681f1a-f639-b3fc-5206-066ed1515997@redhat.com \
    --to=jason@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=iain@sandoe.co.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).