On 8/29/24 11:21 AM, Iain Sandoe wrote:
This will help with the patches to follow for revising the
frame layout when we have an entry with excess alignment.
Tested on x86_64 darwin/linux powerpc64le linux, OK for trunk?
thanks
Iain
--- 8< ---
Now that we have separated the codegen of the ramp, actor and
destroy functions, we no longer need to manage the scopes for
variables manually.
This introduces a helper function that allows us to build a
local var with a DECL_VALUE_EXPR that relates to the coroutine
state frame entry.
This fixes a latent issue where we would generate guard vars
when exceptions were disabled.
gcc/cp/ChangeLog:
* coroutines.cc (coro_build_artificial_var_with_dve): New.
(analyze_fn_parms): Ensure that frame entries cannot clash
with local variables.
(build_coroutine_frame_delete_expr): Amend comment.
(cp_coroutine_transform::build_ramp_function): Rework to
avoid manual management of variables and scopes.
Signed-off-by: Iain Sandoe <i...@sandoe.co.uk>
---
gcc/cp/coroutines.cc | 264 +++++++++++++++++++++----------------------
1 file changed, 129 insertions(+), 135 deletions(-)
diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index 2a1183d70e4..d4d74838eb4 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -1731,6 +1731,20 @@ coro_build_artificial_var (location_t loc, const char
*name, tree type,
type, ctx, init);
}
+/* Build a var NAME of TYPE in CTX and with INIT; add a DECL_VALUE_EXPR that
+ refers to BASE.FIELD. */
+
+static tree
+coro_build_artificial_var_with_dve (location_t loc, tree name, tree type,
+ tree ctx, tree init, tree base, tree field)
+{
+ tree v = coro_build_artificial_var (loc, name, type, ctx, init);
+ tree dve = coro_build_frame_access_expr (base, field, true,
+ tf_warning_or_error);
+ SET_DECL_VALUE_EXPR (v, dve);
+ DECL_HAS_VALUE_EXPR_P (v) = true;
+ return v;
+}
/* 2. Create a named label in the specified context. */
@@ -3841,8 +3855,10 @@ analyze_fn_parms (tree orig, hash_map<tree, param_info> *param_uses)
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))
+ /* Count the param copies from 1 as per the std. */
+ unsigned parm_num = 1;
+ for (tree arg = DECL_ARGUMENTS (orig); arg != NULL;
+ ++parm_num, arg = DECL_CHAIN (arg))
{
bool existed;
param_info &parm = param_uses->get_or_insert (arg, &existed);
@@ -3877,15 +3893,14 @@ analyze_fn_parms (tree orig, hash_map<tree, param_info>
*param_uses)
tree name = DECL_NAME (arg);
if (!name)
{
- char *buf = xasprintf ("_Coro_unnamed_parm_%d", no_name_parm++);
+ char *buf = xasprintf ("anon%d", parm_num);
Why the reduction in verbosity here?
name = get_identifier (buf);
free (buf);
}
parm.field_id = name;
-
if (TYPE_HAS_NONTRIVIAL_DESTRUCTOR (parm.frame_type))
{
- char *buf = xasprintf ("%s%s_live", DECL_NAME (arg) ? "_Coro_" : "",
+ char *buf = xasprintf ("_Coro_q%u_%s_live", parm_num,
IDENTIFIER_POINTER (name));
parm.guard_var
= coro_build_artificial_var (UNKNOWN_LOCATION, get_identifier (buf),
@@ -4569,7 +4584,7 @@ build_coroutine_frame_delete_expr (tree coro_fp, tree
frame_size,
removed, and use it as the declaration of the ramp which both replaces the
user's written function at call sites, and is responsible for starting
the coroutine it defined.
- returns NULL_TREE on error or an expression for the frame size.
+ returns false on error.
We should arrive here with the state of the compiler as if we had just
executed start_preparsed_function(). */
@@ -4628,9 +4643,7 @@ cp_coroutine_transform::build_ramp_function ()
frame_size = TYPE_SIZE_UNIT (frame_type);
- /* Make a var to represent the frame pointer early. Initialize to zero so
- that we can pass it to the IFN_CO_FRAME (to give that access to the frame
- type). */
+ /* Make a var to represent the frame pointer early. */
tree coro_fp = coro_build_artificial_var (loc, "_Coro_frameptr",
frame_ptr_type, orig_fn_decl,
NULL_TREE);
@@ -4660,66 +4673,54 @@ cp_coroutine_transform::build_ramp_function ()
/* So now construct the Ramp: */
- tree ramp_fnbody = begin_function_body ();
- /* Now build the ramp function pieces. */
- tree ramp_bind = build3 (BIND_EXPR, void_type_node, NULL, NULL, NULL);
- add_stmt (ramp_bind);
- tree ramp_outer_bind = push_stmt_list ();
- tree varlist = coro_fp;
-
- /* To signal that we need to cleanup copied function args. */
- if (flag_exceptions && DECL_ARGUMENTS (orig_fn_decl))
- for (tree arg = DECL_ARGUMENTS (orig_fn_decl); arg != NULL;
- arg = DECL_CHAIN (arg))
- {
- param_info *parm_i = param_uses.get (arg);
- gcc_checking_assert (parm_i);
- if (parm_i->trivial_dtor)
- continue;
- DECL_CHAIN (parm_i->guard_var) = varlist;
- varlist = parm_i->guard_var;
- }
+ tree ramp_fnbody = begin_compound_stmt (BCS_FN_BODY);
+ coro_fp = pushdecl (coro_fp);
+ add_decl_expr (coro_fp);
- /* Signal that we need to clean up the promise object on exception. */
- tree coro_promise_live
- = coro_build_artificial_var (loc, "_Coro_promise_live", boolean_type_node,
- orig_fn_decl, boolean_false_node);
- DECL_CHAIN (coro_promise_live) = varlist;
- varlist = coro_promise_live;
+ tree coro_promise_live = NULL_TREE;
+ tree coro_gro_live = NULL_TREE;
+ if (flag_exceptions)
+ {
+ /* Signal that we need to clean up the promise object on exception. */
+ coro_promise_live
+ = coro_build_artificial_var (loc, "_Coro_promise_live",
+ boolean_type_node, orig_fn_decl,
+ boolean_false_node);
+ coro_promise_live = pushdecl (coro_promise_live);
+ add_decl_expr (coro_promise_live);
/* When the get-return-object is in the RETURN slot, we need to arrange for
cleanup on exception. */
- tree coro_gro_live
+ coro_gro_live
= coro_build_artificial_var (loc, "_Coro_gro_live", boolean_type_node,
orig_fn_decl, boolean_false_node);
- DECL_CHAIN (coro_gro_live) = varlist;
- varlist = coro_gro_live;
-
- /* Collected the scope vars we need ... only one for now. */
- BIND_EXPR_VARS (ramp_bind) = nreverse (varlist);
-
- /* We're now going to create a new top level scope block for the ramp
- function. */
- tree top_block = make_node (BLOCK);
-
- BIND_EXPR_BLOCK (ramp_bind) = top_block;
- BLOCK_VARS (top_block) = BIND_EXPR_VARS (ramp_bind);
- BLOCK_SUBBLOCKS (top_block) = NULL_TREE;
- current_binding_level->blocks = top_block;
+ coro_gro_live = pushdecl (coro_gro_live);
+ add_decl_expr (coro_gro_live);
+ /* To signal that we need to cleanup copied function args. */
+ if (DECL_ARGUMENTS (orig_fn_decl))
+ for (tree arg = DECL_ARGUMENTS (orig_fn_decl); arg != NULL;
+ arg = DECL_CHAIN (arg))
+ {
+ param_info *parm_i = param_uses.get (arg);
+ if (parm_i->trivial_dtor)
+ continue;
+ parm_i->guard_var = pushdecl (parm_i->guard_var);
+ add_decl_expr (parm_i->guard_var);
+ }
+ }
- add_decl_expr (coro_fp);
- if (flag_exceptions && DECL_ARGUMENTS (orig_fn_decl))
- for (tree arg = DECL_ARGUMENTS (orig_fn_decl); arg != NULL;
- arg = DECL_CHAIN (arg))
- {
- param_info *parm_i = param_uses.get (arg);
- if (parm_i->trivial_dtor)
- continue;
- add_decl_expr (parm_i->guard_var);
- }
- add_decl_expr (coro_promise_live);
- add_decl_expr (coro_gro_live);
+ /* deref the frame pointer, to use in member access code. */
+ tree deref_fp
+ = cp_build_indirect_ref (loc, coro_fp, RO_UNARY_STAR,
+ tf_warning_or_error);
+ tree frame_needs_free
+ = coro_build_artificial_var_with_dve (loc, coro_frame_needs_free_id,
+ boolean_type_node, orig_fn_decl,
+ NULL_TREE, deref_fp,
+ coro_frame_needs_free_id);
+ frame_needs_free = pushdecl (frame_needs_free);
+ add_decl_expr (frame_needs_free);
/* Build the frame. */
@@ -4764,52 +4765,59 @@ cp_coroutine_transform::build_ramp_function ()
finish_if_stmt (if_stmt);
}
+ /* For now, once allocation has succeeded we always assume that this needs
+ destruction, there's no impl. for frame allocation elision. */
+ r = cp_build_init_expr (frame_needs_free, boolean_true_node);
+ finish_expr_stmt (r);
+
+ /* Set up the promise. */
+ tree p = coro_build_artificial_var_with_dve (loc, coro_promise_id,
+ promise_type, orig_fn_decl,
+ NULL_TREE, deref_fp,
+ coro_promise_id);
+ p = pushdecl_outermost_localscope (p);
+ add_decl_expr (p);
Why outermost_localscope but adding the DECL_EXPR in the same list as
the others that use plain pushdecl?
Incidentally, it seems like most uses of _build_art are followed by
pushdecl and add_decl_expr, maybe another function could combine them?
Say, ...push_artificial_var...? Or _declare_?
Jason
