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