> On 3 Sep 2021, at 15:07, Jason Merrill via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
>
> On 9/1/21 6:56 AM, Iain Sandoe wrote:
>> This adds top level proxy variables for the coroutine frame
>> copies of the original function args. These are then available
>> in the debugger to refer to the frame copies. We rewrite the
>> function body to use the copies, since the original parms will
>> no longer be in scope when the coroutine is running.
>> Signed-off-by: Iain Sandoe <i...@sandoe.co.uk>
>> gcc/cp/ChangeLog:
>> * coroutines.cc (struct param_info): Add copy_var.
>> (build_actor_fn): Use simplified param references.
>> (register_param_uses): Likewise.
>> (rewrite_param_uses): Likewise.
>> (analyze_fn_parms): New function.
>> (coro_rewrite_function_body): Add proxies for the fn
>> parameters to the outer bind scope of the rewritten code.
>> (morph_fn_to_coro): Use simplified version of param ref.
>> ---
>> gcc/cp/coroutines.cc | 247 ++++++++++++++++++++-----------------------
>> 1 file changed, 117 insertions(+), 130 deletions(-)
>> diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
>> index aacf352f1c9..395e5c488e5 100644
>> --- a/gcc/cp/coroutines.cc
>> +++ b/gcc/cp/coroutines.cc
>> @@ -1964,6 +1964,7 @@ transform_await_wrapper (tree *stmt, int *do_subtree,
>> void *d)
>> struct param_info
>> {
>> tree field_id; /* The name of the copy in the coroutine frame. */
>> + tree copy_var; /* The local var proxy for the frame copy. */
>> vec<tree *> *body_uses; /* Worklist of uses, void if there are none. */
>> tree frame_type; /* The type used to represent this parm in the frame.
>> */
>> tree orig_type; /* The original type of the parm (not as passed). */
>> @@ -2169,36 +2170,6 @@ build_actor_fn (location_t loc, tree coro_frame_type,
>> tree actor, tree fnbody,
>> /* Declare the continuation handle. */
>> add_decl_expr (continuation);
>> - /* Re-write param references in the body, no code should be generated
>> - here. */
>> - if (DECL_ARGUMENTS (orig))
>> - {
>> - tree arg;
>> - for (arg = DECL_ARGUMENTS (orig); arg != NULL; arg = DECL_CHAIN (arg))
>> - {
>> - bool existed;
>> - param_info &parm = param_uses->get_or_insert (arg, &existed);
>> - if (!parm.body_uses)
>> - continue; /* Wasn't used in the original function body. */
>> -
>> - tree fld_ref = lookup_member (coro_frame_type, parm.field_id,
>> - /*protect=*/1, /*want_type=*/0,
>> - tf_warning_or_error);
>> - tree fld_idx = build3_loc (loc, COMPONENT_REF, parm.frame_type,
>> - actor_frame, fld_ref, NULL_TREE);
>> -
>> - /* We keep these in the frame as a regular pointer, so convert that
>> - back to the type expected. */
>> - if (parm.pt_ref)
>> - fld_idx = build1_loc (loc, CONVERT_EXPR, TREE_TYPE (arg), fld_idx);
>> -
>> - int i;
>> - tree *puse;
>> - FOR_EACH_VEC_ELT (*parm.body_uses, i, puse)
>> - *puse = fld_idx;
>> - }
>> - }
>> -
>> /* Re-write local vars, similarly. */
>> local_vars_transform xform_vars_data
>> = {actor, actor_frame, coro_frame_type, loc, local_var_uses};
>> @@ -3772,11 +3743,11 @@ struct param_frame_data
>> bool param_seen;
>> };
>> -/* A tree-walk callback that records the use of parameters (to allow for
>> - optimizations where handling unused parameters may be omitted). */
>> +/* A tree walk callback that rewrites each parm use to the local variable
>> + that represents its copy in the frame. */
>> static tree
>> -register_param_uses (tree *stmt, int *do_subtree ATTRIBUTE_UNUSED, void *d)
>> +rewrite_param_uses (tree *stmt, int *do_subtree ATTRIBUTE_UNUSED, void *d)
>> {
>> param_frame_data *data = (param_frame_data *) d;
>> @@ -3784,7 +3755,7 @@ register_param_uses (tree *stmt, int *do_subtree
>> ATTRIBUTE_UNUSED, void *d)
>> if (TREE_CODE (*stmt) == VAR_DECL && DECL_HAS_VALUE_EXPR_P (*stmt))
>> {
>> tree t = DECL_VALUE_EXPR (*stmt);
>> - return cp_walk_tree (&t, register_param_uses, d, NULL);
>> + return cp_walk_tree (&t, rewrite_param_uses, d, NULL);
>> }
>> if (TREE_CODE (*stmt) != PARM_DECL)
>> @@ -3798,16 +3769,87 @@ register_param_uses (tree *stmt, int *do_subtree
>> ATTRIBUTE_UNUSED, void *d)
>> param_info &parm = data->param_uses->get_or_insert (*stmt, &existed);
>> gcc_checking_assert (existed);
>> - if (!parm.body_uses)
>> + *stmt = parm.copy_var;
>> + return NULL_TREE;
>> +}
>> +
>> +/* Build up a set of info that determines how each param copy will be
>> + handled. */
>> +
>> +static hash_map<tree, param_info> *analyze_fn_parms (tree orig)
>
> Function name should be on a new line.
oops.
>
>> +{
>> + if (!DECL_ARGUMENTS (orig))
>> + return NULL;
>> +
>> + hash_map<tree, param_info> *param_uses = new hash_map<tree, param_info>;
>> +
>> + /* Build a hash map with an entry for each param.
>> + The key is the param tree.
>> + Then we have an entry for the frame field name.
>> + Then a cache for the field ref when we come to use it.
>> + Then a tree list of the uses.
>> + The second two entries start out empty - and only get populated
>> + when we see uses. */
>> + bool lambda_p = LAMBDA_FUNCTION_P (orig);
>> +
>> + unsigned no_name_parm = 0;
>> + for (tree arg = DECL_ARGUMENTS (orig); arg != NULL; arg = DECL_CHAIN
>> (arg))
>> {
>> - vec_alloc (parm.body_uses, 4);
>> - parm.body_uses->quick_push (stmt);
>> - data->param_seen = true;
>> + bool existed;
>> + param_info &parm = param_uses->get_or_insert (arg, &existed);
>> + gcc_checking_assert (!existed);
>> + parm.body_uses = NULL;
>> + tree actual_type = TREE_TYPE (arg);
>> + actual_type = complete_type_or_else (actual_type, orig);
>> + if (actual_type == NULL_TREE)
>> + actual_type = error_mark_node;
>> + parm.orig_type = actual_type;
>> + parm.by_ref = parm.pt_ref = parm.rv_ref = false;
>> + if (TREE_CODE (actual_type) == REFERENCE_TYPE)
>> + {
>> + /* If the user passes by reference, then we will save the
>> + pointer to the original. As noted in
>> + [dcl.fct.def.coroutine] / 13, if the lifetime of the
>> + referenced item ends and then the coroutine is resumed,
>> + we have UB; well, the user asked for it. */
>> + if (TYPE_REF_IS_RVALUE (actual_type))
>> + parm.rv_ref = true;
>> + else
>> + parm.pt_ref = true;
>> + }
>> + else if (TYPE_REF_P (DECL_ARG_TYPE (arg)))
>> + parm.by_ref = true;
>> +
>> + parm.frame_type = actual_type;
>> +
>> + parm.this_ptr = is_this_parameter (arg);
>> + parm.lambda_cobj = lambda_p && DECL_NAME (arg) == closure_identifier;
>> +
>> + tree name = DECL_NAME (arg);
>> + if (!name)
>> + {
>> + char *buf = xasprintf ("_Coro_unnamed_parm_%d", no_name_parm++);
>> + name = get_identifier (buf);
>> + free (buf);
>> + }
>> + parm.field_id = name;
>> +
>> + if (TYPE_HAS_NONTRIVIAL_DESTRUCTOR (parm.frame_type))
>> + {
>> + char *buf = xasprintf ("_Coro_%s_live", IDENTIFIER_POINTER (name));
>> + parm.guard_var = build_lang_decl (VAR_DECL, get_identifier (buf),
>> + boolean_type_node);
>> + free (buf);
>> + DECL_ARTIFICIAL (parm.guard_var) = true;
>> + DECL_CONTEXT (parm.guard_var) = orig;
>> + DECL_INITIAL (parm.guard_var) = boolean_false_node;
>> + parm.trivial_dtor = false;
>> + }
>> + else
>> + parm.trivial_dtor = true;
>> }
>> - else
>> - parm.body_uses->safe_push (stmt);
>> - return NULL_TREE;
>> + return param_uses;
>> }
>> /* Small helper for the repetitive task of adding a new field to the coro
>> @@ -3991,6 +4033,7 @@ 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,
>> + hash_map<tree, param_info> *param_uses,
>> tree resume_fn_ptr_type,
>> tree& resume_idx_var, tree& fs_label)
>> {
>> @@ -4074,6 +4117,36 @@ coro_rewrite_function_body (location_t fn_start, tree
>> fnbody, tree orig,
>> var_list = var;
>> add_decl_expr (var);
>> + /* If we have function parms, then these will be copied to the coroutine
>> + frame. Create a local variable to point to each of them so that we can
>> + see them in the debugger. */
>> +
>> + if (param_uses)
>> + {
>> + gcc_checking_assert (DECL_ARGUMENTS (orig));
>> + /* Add a local var for each parm. */
>> + for (tree arg = DECL_ARGUMENTS (orig); arg != NULL;
>> + arg = DECL_CHAIN (arg))
>> + {
>> + param_info *parm_i = param_uses->get (arg);
>> + gcc_checking_assert (parm_i);
>> + parm_i->copy_var
>> + = build_lang_decl (VAR_DECL, parm_i->field_id, TREE_TYPE (arg));
>> + DECL_SOURCE_LOCATION (parm_i->copy_var) = DECL_SOURCE_LOCATION (arg);
>> + DECL_CONTEXT (parm_i->copy_var) = orig;
>> + DECL_ARTIFICIAL (parm_i->copy_var) = true;
>> + DECL_CHAIN (parm_i->copy_var) = var_list;
>> + var_list = parm_i->copy_var;
>> + add_decl_expr (parm_i->copy_var);
>
> Are these getting DECL_VALUE_EXPR somewhere?
Yes - all variables in the outermost bind expression will be allocated a frame
slot, and gain a DECL_VALUE_EXPR to point to it.
>> + }
>> +
>> + /* Now replace all uses of the parms in the function body with the
>> local
>> + vars. */
>
> I think the following old comment still applies to how 'visited' is used, and
> should be adapted here as well:
Yes, will do.
>
>>> - /* We want to record every instance of param's use, so don't include
>>> - a 'visited' hash_set on the tree walk, but only record a containing
>>> - expression once. */
>
>> + hash_set<tree *> visited;
>> + param_frame_data param_data = {NULL, param_uses,
>> + &visited, fn_start, false};
>> + cp_walk_tree (&fnbody, rewrite_param_uses, ¶m_data, NULL);
>> + }
>> /* We create a resume index, this is initialized in the ramp. */
>> resume_idx_var
>> @@ -4343,7 +4416,9 @@ morph_fn_to_coro (tree orig, tree *resumer, tree
>> *destroyer)
>> tree resume_idx_var = NULL_TREE;
>> tree fs_label = NULL_TREE;
>> - fnbody = coro_rewrite_function_body (fn_start, fnbody, orig,
>> + hash_map<tree, param_info> *param_uses = analyze_fn_parms (orig);
>> +
>> + fnbody = coro_rewrite_function_body (fn_start, fnbody, orig, param_uses,
>> act_des_fn_ptr,
>> resume_idx_var, fs_label);
>> /* Build our dummy coro frame layout. */
>> @@ -4352,94 +4427,6 @@ morph_fn_to_coro (tree orig, tree *resumer, tree
>> *destroyer)
>> /* The fields for the coro frame. */
>> tree field_list = NULL_TREE;
>> - /* 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
>> - uses and build worklists to replace those when the state machine is
>> - lowered. */
>> -
>> - hash_map<tree, param_info> *param_uses = NULL;
>> - if (DECL_ARGUMENTS (orig))
>> - {
>> - /* Build a hash map with an entry for each param.
>> - The key is the param tree.
>> - Then we have an entry for the frame field name.
>> - Then a cache for the field ref when we come to use it.
>> - Then a tree list of the uses.
>> - The second two entries start out empty - and only get populated
>> - when we see uses. */
>> - param_uses = new hash_map<tree, param_info>;
>> - bool lambda_p = LAMBDA_FUNCTION_P (orig);
>> -
>> - unsigned no_name_parm = 0;
>> - for (tree arg = DECL_ARGUMENTS (orig); arg != NULL;
>> - arg = DECL_CHAIN (arg))
>> - {
>> - bool existed;
>> - param_info &parm = param_uses->get_or_insert (arg, &existed);
>> - gcc_checking_assert (!existed);
>> - parm.body_uses = NULL;
>> - tree actual_type = TREE_TYPE (arg);
>> - actual_type = complete_type_or_else (actual_type, orig);
>> - if (actual_type == NULL_TREE)
>> - actual_type = error_mark_node;
>> - parm.orig_type = actual_type;
>> - parm.by_ref = parm.pt_ref = parm.rv_ref = false;
>> - if (TREE_CODE (actual_type) == REFERENCE_TYPE)
>> - {
>> - /* If the user passes by reference, then we will save the
>> - pointer to the original. As noted in
>> - [dcl.fct.def.coroutine] / 13, if the lifetime of the
>> - referenced item ends and then the coroutine is resumed,
>> - we have UB; well, the user asked for it. */
>> - if (TYPE_REF_IS_RVALUE (actual_type))
>> - parm.rv_ref = true;
>> - else
>> - parm.pt_ref = true;
>> - }
>> - else if (TYPE_REF_P (DECL_ARG_TYPE (arg)))
>> - parm.by_ref = true;
>> -
>> - parm.frame_type = actual_type;
>> -
>> - parm.this_ptr = is_this_parameter (arg);
>> - parm.lambda_cobj = lambda_p && DECL_NAME (arg) == closure_identifier;
>> -
>> - char *buf;
>> - if (DECL_NAME (arg))
>> - {
>> - tree pname = DECL_NAME (arg);
>> - buf = xasprintf ("_P_%s", IDENTIFIER_POINTER (pname));
>> - }
>> - else
>> - buf = xasprintf ("_P_unnamed_%d", no_name_parm++);
>> -
>> - if (TYPE_HAS_NONTRIVIAL_DESTRUCTOR (parm.frame_type))
>> - {
>> - char *gbuf = xasprintf ("%s_live", buf);
>> - parm.guard_var
>> - = build_lang_decl (VAR_DECL, get_identifier (gbuf),
>> - boolean_type_node);
>> - free (gbuf);
>> - DECL_ARTIFICIAL (parm.guard_var) = true;
>> - DECL_INITIAL (parm.guard_var) = boolean_false_node;
>> - parm.trivial_dtor = false;
>> - }
>> - else
>> - parm.trivial_dtor = true;
>> - parm.field_id = coro_make_frame_entry
>> - (&field_list, buf, actual_type, DECL_SOURCE_LOCATION (arg));
>> - free (buf);
>> - }
>> -
>> - /* We want to record every instance of param's use, so don't include
>> - a 'visited' hash_set on the tree walk, but only record a containing
>> - expression once. */
>> - hash_set<tree *> visited;
>> - param_frame_data param_data
>> - = {&field_list, param_uses, &visited, fn_start, false};
>> - cp_walk_tree (&fnbody, register_param_uses, ¶m_data, NULL);
>> - }
>> -
>> /* We need to know, and inspect, each suspend point in the function
>> in several places. It's convenient to place this map out of line
>> since it's used from tree walk callbacks. */
