> On 28 Apr 2022, at 20:26, Jakub Jelinek <ja...@redhat.com> wrote:
> 
> On Thu, Apr 28, 2022 at 08:17:04PM +0100, Iain Sandoe wrote:
>> Signed-off-by: Iain Sandoe <i...@sandoe.co.uk>
>> 
>>      PR c++/105426
>> 
>> gcc/cp/ChangeLog:
>> 
>>      * coroutines.cc (register_local_var_uses): Allow promotion of unnamed
>>      temporaries to coroutine frame copies.
>> ---
>> gcc/cp/coroutines.cc | 3 +++
>> 1 file changed, 3 insertions(+)
>> 
>> diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
>> index 551ddc9cc41..2e393b2cddc 100644
>> --- a/gcc/cp/coroutines.cc
>> +++ b/gcc/cp/coroutines.cc
>> @@ -3973,6 +3973,9 @@ register_local_var_uses (tree *stmt, int *do_subtree, 
>> void *d)
>>        else if (lvname != NULL_TREE)
>>          buf = xasprintf ("%s_%u_%u", IDENTIFIER_POINTER (lvname),
>>                           lvd->nest_depth, lvd->bind_indx);
>> +      else
>> +        buf = xasprintf ("_D%u_%u_%u", DECL_UID (lvar), lvd->nest_depth,
>> +                         lvd->bind_indx);
>>        /* TODO: Figure out if we should build a local type that has any
>>           excess alignment or size from the original decl.  */
>>        if (buf)
> 
> This isn't going to play well with -fcompare-debug.
> We don't guarantee same DECL_UID values between -g and -g0, just that
> when two decls are created with both -g and -g0 they compare the same,
> but with -g the gap in between them could be larger.
> So names that include DECL_UID will be often different between -g and -g0.

that’s somewhat unfortunate (having the UIDs in the frame and the gimple is 
quite
helpful to debugging).  I guess I was not expecting a difference this early in 
the FE
(we are before initial folding / constexpr stuff).

FWIW, I ran the coroutines and coroutines torture testsuites with 
-fcompare-debug.
There are 5 fails, and those are unaffected by whether the field is made 
nameless as
suggested below (so something else to investigate).

> Can't the FIELD_DECL be instead nameless?  Say change coro_make_frame_entry
> to do
>  tree id = name ? get_identifier (name) : NULL_TREE;
> instead of
>  tree id = get_identifier (name);

Unfortunately, that does not work - although I cannot see anything in the 
coroutines code
that would cause it to fail - perhaps something is not playing nicely with
DECL_VALUE_EXPRs on unnamed temps?

how about the following, which uniques the names by bind scope, scope nest and 
then
sequence within that?

Iain

diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index 2e393b2cddc..1d886b31c77 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -3913,6 +3913,7 @@ register_local_var_uses (tree *stmt, int *do_subtree, 
void *d)
   if (TREE_CODE (*stmt) == BIND_EXPR)
     {
       tree lvar;
+      unsigned serial = 0;
       for (lvar = BIND_EXPR_VARS (*stmt); lvar != NULL;
           lvar = DECL_CHAIN (lvar))
        {
@@ -3974,16 +3975,14 @@ register_local_var_uses (tree *stmt, int *do_subtree, 
void *d)
            buf = xasprintf ("%s_%u_%u", IDENTIFIER_POINTER (lvname),
                             lvd->nest_depth, lvd->bind_indx);
          else
-           buf = xasprintf ("_D%u_%u_%u", DECL_UID (lvar), lvd->nest_depth,
-                            lvd->bind_indx);
+           buf = xasprintf ("_D%u_%u_%u", lvd->nest_depth, lvd->bind_indx,
+                            serial++);
+
          /* TODO: Figure out if we should build a local type that has any
             excess alignment or size from the original decl.  */
-         if (buf)
-           {
-             local_var.field_id = coro_make_frame_entry (lvd->field_list, buf,
-                                                         lvtype, lvd->loc);
-             free (buf);
-           }
+         local_var.field_id = coro_make_frame_entry (lvd->field_list, buf,
+                                                     lvtype, lvd->loc);
+         free (buf);
          /* We don't walk any of the local var sub-trees, they won't contain
             any bind exprs.  */
        }




Reply via email to