On 9/3/21 9:56 AM, Iain Sandoe wrote:


On 3 Sep 2021, at 14:52, Jason Merrill <ja...@redhat.com> wrote:

On 9/1/21 6:55 AM, Iain Sandoe wrote:
The user might well wish to inspect some of the state that represents
the implementation of the coroutine machine.
In particular:
   The promise object.
   The function pointers for the resumer and destroyer.
   The current resume index (suspend point).
   The handle that represent this coroutine 'self handle'.
   Whether the coroutine frame is allocated and needs to be freed.
These variables are given names that can be 'well-known' and advertised
in debug documentation - they are placed in the implementation namespace
and all begin with _Coro_.

Signed-off-by: Iain Sandoe <i...@sandoe.co.uk>
gcc/cp/ChangeLog:
        * coroutines.cc (transform_await_expr): Use debug-friendly
        names for coroutine implementation.
        (build_actor_fn): Likewise.
        (build_destroy_fn): Likewise.
        (coro_rewrite_function_body): Likewise.
        (morph_fn_to_coro): Likewise.

Hmm, this patch doesn't seem to match the description and ChangeLog entry other 
than in the names of the functions changed.

with 20:20 hindsight I should have squashed the (several) patches related to 
the implementation symbols,

I’ll redo the description - essentially, this is just making use of the 
simplification available because we now have pre-defined values for the field 
names.

I can see how that describes a few lines in this patch, but not for instance the change to transform_await_expr, which seems to have nothing to do with names?

But yes, moving the changed lines that just use the variables from the previous patch into that commit sounds good. I use rebase -i for that sort of thing all the time.

---
  gcc/cp/coroutines.cc | 214 +++++++++++++++++++------------------------
  1 file changed, 94 insertions(+), 120 deletions(-)
diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index 3b46aac4dc5..aacf352f1c9 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -1906,7 +1906,6 @@ transform_await_expr (tree await_expr, await_xform_data 
*xform)
    /* So, on entry, we have:
       in : CO_AWAIT_EXPR (a, e_proxy, o, awr_call_vector, mode)
          We no longer need a [it had diagnostic value, maybe?]
-         We need to replace the promise proxy in all elements
          We need to replace the e_proxy in the awr_call.  */
      tree coro_frame_type = TREE_TYPE (xform->actor_frame);
@@ -1932,16 +1931,6 @@ transform_await_expr (tree await_expr, await_xform_data 
*xform)
        TREE_OPERAND (await_expr, 1) = as;
      }
  -  /* Now do the self_handle.  */
-  data.from = xform->self_h_proxy;
-  data.to = xform->real_self_h;
-  cp_walk_tree (&await_expr, replace_proxy, &data, NULL);
-
-  /* Now do the promise.  */
-  data.from = xform->promise_proxy;
-  data.to = xform->real_promise;
-  cp_walk_tree (&await_expr, replace_proxy, &data, NULL);
-
    return await_expr;
  }
  @@ -2128,10 +2117,9 @@ coro_get_frame_dtor (tree coro_fp, tree orig, tree 
frame_size,
    static void
  build_actor_fn (location_t loc, tree coro_frame_type, tree actor, tree fnbody,
-               tree orig, hash_map<tree, param_info> *param_uses,
-               hash_map<tree, local_var_info> *local_var_uses,
-               vec<tree, va_gc> *param_dtor_list, tree resume_fn_field,
-               tree resume_idx_field, unsigned body_count, tree frame_size)
+               tree orig, hash_map<tree, local_var_info> *local_var_uses,
+               vec<tree, va_gc> *param_dtor_list,
+               tree resume_idx_var, unsigned body_count, tree frame_size)
  {
    verify_stmt_tree (fnbody);
    /* Some things we inherit from the original function.  */
@@ -2216,8 +2204,8 @@ build_actor_fn (location_t loc, tree coro_frame_type, 
tree actor, tree fnbody,
      = {actor, actor_frame, coro_frame_type, loc, local_var_uses};
    cp_walk_tree (&fnbody, transform_local_var_uses, &xform_vars_data, NULL);
  -  tree rat_field = lookup_member (coro_frame_type, coro_resume_index_field, 
1, 0,
-                                 tf_warning_or_error);
+  tree rat_field = lookup_member (coro_frame_type, coro_resume_index_field,
+                                 1, 0, tf_warning_or_error);
    tree rat = build3 (COMPONENT_REF, short_unsigned_type_node, actor_frame,
                     rat_field, NULL_TREE);
  @@ -2319,14 +2307,8 @@ build_actor_fn (location_t loc, tree coro_frame_type, 
tree actor, tree fnbody,
    tree r = build_stmt (loc, LABEL_EXPR, actor_begin_label);
    add_stmt (r);
  -  /* actor's version of the promise.  */
-  tree ap_m = lookup_member (coro_frame_type, get_identifier 
("_Coro_promise"), 1, 0,
-                            tf_warning_or_error);
-  tree ap = build_class_member_access_expr (actor_frame, ap_m, NULL_TREE, 
false,
-                                           tf_warning_or_error);
-
    /* actor's coroutine 'self handle'.  */
-  tree ash_m = lookup_member (coro_frame_type, get_identifier 
("_Coro_self_handle"), 1,
+  tree ash_m = lookup_member (coro_frame_type, coro_self_handle_field, 1,
                              0, tf_warning_or_error);
    tree ash = build_class_member_access_expr (actor_frame, ash_m, NULL_TREE,
                                             false, tf_warning_or_error);
@@ -2347,36 +2329,13 @@ build_actor_fn (location_t loc, tree coro_frame_type, 
tree actor, tree fnbody,
       decide where to put things.  */
      await_xform_data xform
-    = {actor, actor_frame, promise_proxy, ap, self_h_proxy, ash};
+    = {actor, actor_frame, NULL_TREE, NULL_TREE, self_h_proxy, ash};
      /* Transform the await expressions in the function body.  Only do each
       await tree once!  */
    hash_set<tree> pset;
    cp_walk_tree (&fnbody, transform_await_wrapper, &xform, &pset);
  -  /* Now replace the promise proxy with its real value.  */
-  proxy_replace p_data;
-  p_data.from = promise_proxy;
-  p_data.to = ap;
-  cp_walk_tree (&fnbody, replace_proxy, &p_data, NULL);
-
-  /* The rewrite of the function adds code to set the resume_fn field to
-     nullptr when the coroutine is done and also the index to zero when
-     calling an unhandled exception.  These are represented by two proxies
-     in the function, so rewrite them to the proper frame access.  */
-  tree resume_m
-    = lookup_member (coro_frame_type, get_identifier ("_Coro_resume_fn"),
-                    /*protect=*/1, /*want_type=*/0, tf_warning_or_error);
-  tree res_x = build_class_member_access_expr (actor_frame, resume_m, 
NULL_TREE,
-                                              false, tf_warning_or_error);
-  p_data.from = resume_fn_field;
-  p_data.to = res_x;
-  cp_walk_tree (&fnbody, replace_proxy, &p_data, NULL);
-
-  p_data.from = resume_idx_field;
-  p_data.to = rat;
-  cp_walk_tree (&fnbody, replace_proxy, &p_data, NULL);
-
    /* Add in our function body with the co_returns rewritten to final form.  */
    add_stmt (fnbody);
  @@ -2385,7 +2344,7 @@ build_actor_fn (location_t loc, tree coro_frame_type, 
tree actor, tree fnbody,
    add_stmt (r);
      /* Destructors for the things we built explicitly.  */
-  r = build_special_member_call (ap, complete_dtor_identifier, NULL,
+  r = build_special_member_call (promise_proxy, complete_dtor_identifier, NULL,
                                 promise_type, LOOKUP_NORMAL,
                                 tf_warning_or_error);
    add_stmt (r);
@@ -2398,7 +2357,7 @@ build_actor_fn (location_t loc, tree coro_frame_type, 
tree actor, tree fnbody,
    /* Here deallocate the frame (if we allocated it), which we will have at
       present.  */
    tree fnf_m
-    = lookup_member (coro_frame_type, get_identifier 
("_Coro_frame_needs_free"), 1,
+    = lookup_member (coro_frame_type, coro_frame_needs_free_field, 1,
                     0, tf_warning_or_error);
    tree fnf2_x = build_class_member_access_expr (actor_frame, fnf_m, NULL_TREE,
                                                false, tf_warning_or_error);
@@ -2477,18 +2436,10 @@ build_actor_fn (location_t loc, tree coro_frame_type, 
tree actor, tree fnbody,
    gcc_checking_assert (maybe_cleanup_point_expr_void (r) == r);
    add_stmt (r);
  -  /* We will need to know which resume point number should be encoded.  */
-  tree res_idx_m
-    = lookup_member (coro_frame_type, coro_resume_index_field,
-                    /*protect=*/1, /*want_type=*/0, tf_warning_or_error);
-  tree resume_pt_number
-    = build_class_member_access_expr (actor_frame, res_idx_m, NULL_TREE, false,
-                                     tf_warning_or_error);
-
    /* We've now rewritten the tree and added the initial and final
       co_awaits.  Now pass over the tree and expand the co_awaits.  */
  -  coro_aw_data data = {actor, actor_fp, resume_pt_number, NULL_TREE,
+  coro_aw_data data = {actor, actor_fp, resume_idx_var, NULL_TREE,
                       ash, del_promise_label, ret_label,
                       continue_label, continuation, 2};
    cp_walk_tree (&actor_body, await_statement_expander, &data, NULL);
@@ -2502,7 +2453,7 @@ build_actor_fn (location_t loc, tree coro_frame_type, 
tree actor, tree fnbody,
  }
    /* The prototype 'destroy' function :
-   frame->__resume_at |= 1;
+   frame->__Coro_resume_index |= 1;
     actor (frame);  */
    static void
@@ -2521,10 +2472,10 @@ build_destroy_fn (location_t loc, tree coro_frame_type, 
tree destroy,
      tree destr_frame = build1 (INDIRECT_REF, coro_frame_type, destr_fp);
  -  tree rat_field = lookup_member (coro_frame_type, coro_resume_index_field, 
1, 0,
-                                 tf_warning_or_error);
-  tree rat = build3 (COMPONENT_REF, short_unsigned_type_node, destr_frame,
-                    rat_field, NULL_TREE);
+  tree rat_field = lookup_member (coro_frame_type, coro_resume_index_field,
+                                 1, 0, tf_warning_or_error);
+  tree rat = build3 (COMPONENT_REF, short_unsigned_type_node,
+                        destr_frame, rat_field, NULL_TREE);
      /* _resume_at |= 1 */
    tree dstr_idx = build2 (BIT_IOR_EXPR, short_unsigned_type_node, rat,
@@ -4040,8 +3991,8 @@ coro_build_actor_or_destroy_function (tree orig, tree 
fn_type,
    static tree
  coro_rewrite_function_body (location_t fn_start, tree fnbody, tree orig,
-                           tree resume_fn_ptr_type, tree& resume_fn_field,
-                           tree& resume_idx_field, tree& fs_label)
+                           tree resume_fn_ptr_type,
+                           tree& resume_idx_var, tree& fs_label)
  {
    /* This will be our new outer scope.  */
    tree update_body = build3 (BIND_EXPR, void_type_node, NULL, NULL, NULL);
@@ -4074,7 +4025,6 @@ coro_rewrite_function_body (location_t fn_start, tree 
fnbody, tree orig,
      /* Wrap the function body in a try {} catch (...) {} block, if exceptions
       are enabled.  */
-  tree promise = get_coroutine_promise_proxy (orig);
    tree var_list = NULL_TREE;
    tree initial_await = build_init_or_final_await (fn_start, false);
  @@ -4085,24 +4035,61 @@ coro_rewrite_function_body (location_t fn_start, tree 
fnbody, tree orig,
    tree return_void
      = get_coroutine_return_void_expr (current_function_decl, fn_start, false);
  +  /* The pointer to the resume function.  */
+  tree resume_fn_ptr
+    = coro_build_artificial_var (fn_start, coro_resume_fn_field,
+                                resume_fn_ptr_type, orig, NULL_TREE);
+  DECL_CHAIN (resume_fn_ptr) = var_list;
+  var_list = resume_fn_ptr;
+  add_decl_expr (resume_fn_ptr);
+
    /* We will need to be able to set the resume function pointer to nullptr
       to signal that the coroutine is 'done'.  */
-  resume_fn_field
-    = build_lang_decl (VAR_DECL, get_identifier ("resume.fn.ptr.proxy"),
-                      resume_fn_ptr_type);
-  DECL_ARTIFICIAL (resume_fn_field) = true;
    tree zero_resume
      = build1 (CONVERT_EXPR, resume_fn_ptr_type, integer_zero_node);
-  zero_resume
-    = build2 (INIT_EXPR, resume_fn_ptr_type, resume_fn_field, zero_resume);
-  /* Likewise, the resume index needs to be reset.  */
-  resume_idx_field
-    = build_lang_decl (VAR_DECL, get_identifier ("resume.index.proxy"),
-                      short_unsigned_type_node);
-  DECL_ARTIFICIAL (resume_idx_field) = true;
-  tree zero_resume_idx = build_int_cst (short_unsigned_type_node, 0);
-  zero_resume_idx = build2 (INIT_EXPR, short_unsigned_type_node,
-                           resume_idx_field, zero_resume_idx);
+
+  /* The pointer to the destroy function.  */
+  tree var = coro_build_artificial_var (fn_start, coro_destroy_fn_field,
+                                       resume_fn_ptr_type, orig, NULL_TREE);
+  DECL_CHAIN (var) = var_list;
+  var_list = var;
+  add_decl_expr (var);
+
+  /* The promise was created on demand when parsing we now link it into
+      our scope.  */
+  tree promise = get_coroutine_promise_proxy (orig);
+  DECL_CONTEXT (promise) = orig;
+  DECL_SOURCE_LOCATION (promise) = fn_start;
+  DECL_CHAIN (promise) = var_list;
+  var_list = promise;
+  add_decl_expr (promise);
+
+  /* We need a handle to this coroutine, which is passed to every
+     await_suspend().  This was created on demand when parsing we now link it
+     into our scope.  */
+  var = get_coroutine_self_handle_proxy (orig);
+  DECL_CONTEXT (var) = orig;
+  DECL_SOURCE_LOCATION (var) = fn_start;
+  DECL_CHAIN (var) = var_list;
+  var_list = var;
+  add_decl_expr (var);
+
+
+  /* We create a resume index, this is initialized in the ramp.  */
+  resume_idx_var
+    = coro_build_artificial_var (fn_start, coro_resume_index_field,
+                                short_unsigned_type_node, orig, NULL_TREE);
+  DECL_CHAIN (resume_idx_var) = var_list;
+  var_list = resume_idx_var;
+  add_decl_expr (resume_idx_var);
+
+  /* If the coroutine has a frame that needs to be freed, this will be set by
+     the ramp.  */
+  var = coro_build_artificial_var (fn_start, coro_frame_needs_free_field,
+                                  boolean_type_node, orig, NULL_TREE);
+  DECL_CHAIN (var) = var_list;
+  var_list = var;
+  add_decl_expr (var);
      if (flag_exceptions)
      {
@@ -4166,10 +4153,14 @@ coro_rewrite_function_body (location_t fn_start, tree 
fnbody, tree orig,
         If the unhandled exception method returns, then we continue
         to the final await expression (which duplicates the clearing of
         the field). */
-      finish_expr_stmt (zero_resume);
-      finish_expr_stmt (zero_resume_idx);
-      ueh = maybe_cleanup_point_expr_void (ueh);
-      add_stmt (ueh);
+      tree r = build2 (MODIFY_EXPR, resume_fn_ptr_type, resume_fn_ptr,
+                      zero_resume);
+      finish_expr_stmt (r);
+      tree short_zero = build_int_cst (short_unsigned_type_node, 0);
+      r = build2 (MODIFY_EXPR, short_unsigned_type_node, resume_idx_var,
+                 short_zero);
+      finish_expr_stmt (r);
+      finish_expr_stmt (ueh);
        finish_handler (handler);
        TRY_HANDLERS (tcb) = pop_stmt_list (TRY_HANDLERS (tcb));
      }
@@ -4204,6 +4195,8 @@ coro_rewrite_function_body (location_t fn_start, tree 
fnbody, tree orig,
    /* Before entering the final suspend point, we signal that this point has
       been reached by setting the resume function pointer to zero (this is
       what the 'done()' builtin tests) as per the current ABI.  */
+  zero_resume = build2 (MODIFY_EXPR, resume_fn_ptr_type, resume_fn_ptr,
+                       zero_resume);
    finish_expr_stmt (zero_resume);
    finish_expr_stmt (build_init_or_final_await (fn_start, true));
    BIND_EXPR_BODY (update_body) = pop_stmt_list (BIND_EXPR_BODY (update_body));
@@ -4348,33 +4341,16 @@ morph_fn_to_coro (tree orig, tree *resumer, tree 
*destroyer)
    /* Construct the wrapped function body; we will analyze this to determine
       the requirements for the coroutine frame.  */
  -  tree resume_fn_field = NULL_TREE;
-  tree resume_idx_field = NULL_TREE;
+  tree resume_idx_var = NULL_TREE;
    tree fs_label = NULL_TREE;
    fnbody = coro_rewrite_function_body (fn_start, fnbody, orig,
-                                      act_des_fn_ptr, resume_fn_field,
-                                      resume_idx_field, fs_label);
+                                      act_des_fn_ptr,
+                                      resume_idx_var, fs_label);
    /* Build our dummy coro frame layout.  */
    coro_frame_type = begin_class_definition (coro_frame_type);
  +  /* The fields for the coro frame.  */
    tree field_list = NULL_TREE;
-  tree resume_name
-    = coro_make_frame_entry (&field_list, "_Coro_resume_fn",
-                            act_des_fn_ptr, fn_start);
-  tree destroy_name
-    = coro_make_frame_entry (&field_list, "_Coro_destroy_fn",
-                            act_des_fn_ptr, fn_start);
-  tree promise_name
-    = coro_make_frame_entry (&field_list, "_Coro_promise", promise_type, 
fn_start);
-  tree fnf_name = coro_make_frame_entry (&field_list, "_Coro_frame_needs_free",
-                                        boolean_type_node, fn_start);
-  tree resume_idx_name
-    = coro_make_frame_entry (&field_list, "_Coro_resume_index",
-                            short_unsigned_type_node, fn_start);
-
-  /* We need a handle to this coroutine, which is passed to every
-     await_suspend().  There's no point in creating it over and over.  */
-  (void) coro_make_frame_entry (&field_list, "_Coro_self_handle", handle_type, 
fn_start);
      /* Now add in fields for function params (if there are any).
       We do not attempt elision of copies at this stage, we do analyze the
@@ -4776,8 +4752,8 @@ morph_fn_to_coro (tree orig, tree *resumer, tree 
*destroyer)
      /* For now, once allocation has succeeded we always assume that this needs
       destruction, there's no impl. for frame allocation elision.  */
-  tree fnf_m
-    = lookup_member (coro_frame_type, fnf_name, 1, 0, tf_warning_or_error);
+  tree fnf_m = lookup_member (coro_frame_type, coro_frame_needs_free_field,
+                             1, 0,tf_warning_or_error);
    tree fnf_x = build_class_member_access_expr (deref_fp, fnf_m, NULL_TREE,
                                               false, tf_warning_or_error);
    r = build2 (INIT_EXPR, boolean_type_node, fnf_x, boolean_true_node);
@@ -4788,24 +4764,22 @@ morph_fn_to_coro (tree orig, tree *resumer, tree 
*destroyer)
      tree actor_addr = build1 (ADDR_EXPR, act_des_fn_ptr, actor);
    tree resume_m
-    = lookup_member (coro_frame_type, resume_name,
+    = lookup_member (coro_frame_type, coro_resume_fn_field,
                     /*protect=*/1, /*want_type=*/0, tf_warning_or_error);
    tree resume_x = build_class_member_access_expr (deref_fp, resume_m, 
NULL_TREE,
                                                  false, tf_warning_or_error);
    r = build2_loc (fn_start, INIT_EXPR, act_des_fn_ptr, resume_x, actor_addr);
-  r = coro_build_cvt_void_expr_stmt (r, fn_start);
-  add_stmt (r);
+  finish_expr_stmt (r);
      tree destroy_addr = build1 (ADDR_EXPR, act_des_fn_ptr, destroy);
    tree destroy_m
-    = lookup_member (coro_frame_type, destroy_name,
+    = lookup_member (coro_frame_type, coro_destroy_fn_field,
                     /*protect=*/1, /*want_type=*/0, tf_warning_or_error);
    tree destroy_x
      = build_class_member_access_expr (deref_fp, destroy_m, NULL_TREE, false,
                                      tf_warning_or_error);
    r = build2_loc (fn_start, INIT_EXPR, act_des_fn_ptr, destroy_x, 
destroy_addr);
-  r = coro_build_cvt_void_expr_stmt (r, fn_start);
-  add_stmt (r);
+  finish_expr_stmt (r);
      /* [dcl.fct.def.coroutine] /13
       When a coroutine is invoked, a copy is created for each coroutine
@@ -4896,7 +4870,7 @@ morph_fn_to_coro (tree orig, tree *resumer, tree 
*destroyer)
      /* Set up the promise.  */
    tree promise_m
-    = lookup_member (coro_frame_type, promise_name,
+    = lookup_member (coro_frame_type, coro_promise_field,
                     /*protect=*/1, /*want_type=*/0, tf_warning_or_error);
      tree p = build_class_member_access_expr (deref_fp, promise_m, NULL_TREE,
@@ -5042,9 +5016,9 @@ morph_fn_to_coro (tree orig, tree *resumer, tree 
*destroyer)
                              boolean_type_node);
        finish_expr_stmt (r);
      }
-  /* Initialize the resume_idx_name to 0, meaning "not started".  */
+  /* Initialize the resume_idx_var to 0, meaning "not started".  */
    tree resume_idx_m
-    = lookup_member (coro_frame_type, resume_idx_name,
+    = lookup_member (coro_frame_type, coro_resume_index_field,
                     /*protect=*/1, /*want_type=*/0, tf_warning_or_error);
    tree resume_idx
      = build_class_member_access_expr (deref_fp, resume_idx_m, NULL_TREE, 
false,
@@ -5187,9 +5161,9 @@ morph_fn_to_coro (tree orig, tree *resumer, tree 
*destroyer)
    push_deferring_access_checks (dk_no_check);
      /* Build the actor...  */
-  build_actor_fn (fn_start, coro_frame_type, actor, fnbody, orig, param_uses,
-                 &local_var_uses, param_dtor_list, resume_fn_field,
-                 resume_idx_field, body_aw_points.await_number, frame_size);
+  build_actor_fn (fn_start, coro_frame_type, actor, fnbody, orig,
+                 &local_var_uses, param_dtor_list,
+                 resume_idx_var, body_aw_points.await_number, frame_size);
      /* Destroyer ... */
    build_destroy_fn (fn_start, coro_frame_type, destroy, actor);



Reply via email to