https://gcc.gnu.org/g:400e3b66b782287aa895a8b473ec5f2c24adf698
commit r16-6755-g400e3b66b782287aa895a8b473ec5f2c24adf698 Author: Nathaniel Shead <[email protected]> Date: Fri Jan 9 21:36:32 2026 +1100 c++: modules and coroutines While working on another issue I found that currently modules do not work with coroutines at all. This patch fixes a number of issues in both the coroutines logic and modules logic to ensure that they play well together. To summarize: - The coroutine proxy objects did not have a DECL_CONTEXT set (required for modules to merge declarations). - The coroutine transformation functions are always non-inline, even for an inline ramp function, which means that modules need an override to ensure the definitions are available where needed. - Coroutine transformation functions were not marked DECL_COROUTINE_P, despite accessors implying that they were. - In an importing TU we had lost the connection between the ramp functions and the transform functions, as they were kept in a pair of global maps. - Modules streaming couldn't discriminate between the actor or destroy functions when merging. - Modules streaming wasn't setting the cfun->coroutine_component flag, needed to activate the middle-end coroutine lowering pass. This patch also separates the coroutine_info_table initialization from the ensure_coro_initialized function. If the first time we see a coroutine is from a module import, we need to register the transformation functions now but calling ensure_coro_initialized would lookup e.g. std::coroutine_traits, which may only be visible from this module that we're currently reading, causing a recursive load. Separating the concerns allows this to work correctly. gcc/cp/ChangeLog: * coroutines.cc (create_coroutine_info_table): New function. (get_or_insert_coroutine_info): Mark static. (ensure_coro_initialized): Likewise; use create_coroutine_info_table. (coro_promise_type_found_p): Set DECL_CONTEXT for proxies. (coro_set_ramp_function): New function. (coro_set_transform_functions): New function. (coro_build_actor_or_destroy_function): Use coro_set_ramp_function, mark as DECL_COROUTINE_P. * cp-tree.h (coro_set_transform_functions): Declare. (coro_set_ramp_function): Declare. * module.cc (struct merge_key): New field coro_disc. (dumper::impl::nested_name): Distinguish coroutine transform functions. (get_coroutine_discriminator): New function. (trees_out::key_mergeable): Stream coroutine discriminator. (check_mergeable_decl): Adjust comment, check for matching coroutine discriminator. (trees_in::key_mergeable): Read coroutine discriminator. (has_definition): Override for coroutine transform functions. (trees_out::write_function_def): Stream linked ramp, actor, and destroy functions for coroutines. (trees_in::read_function_def): Read them. (module_state::read_cluster): Set cfun->coroutine_component. gcc/testsuite/ChangeLog: * g++.dg/modules/coro-1_a.C: New test. * g++.dg/modules/coro-1_b.C: New test. Reviewed-by: Iain Sandoe <[email protected]> Reviewed-by: Jason Merrill <[email protected]> Signed-off-by: Nathaniel Shead <[email protected]> Diff: --- gcc/cp/coroutines.cc | 59 +++++++++++++++++---- gcc/cp/cp-tree.h | 4 ++ gcc/cp/module.cc | 90 ++++++++++++++++++++++++++++++--- gcc/testsuite/g++.dg/modules/coro-1_a.C | 34 +++++++++++++ gcc/testsuite/g++.dg/modules/coro-1_b.C | 19 +++++++ 5 files changed, 189 insertions(+), 17 deletions(-) diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc index f0485a95073b..4a2f410f225c 100644 --- a/gcc/cp/coroutines.cc +++ b/gcc/cp/coroutines.cc @@ -353,10 +353,20 @@ coroutine_info_hasher::equal (coroutine_info *lhs, const compare_type& rhs) return lhs->function_decl == rhs; } +/* Initialize the coroutine info table, to hold state per coroutine decl, + if not already created. */ + +static void +create_coroutine_info_table () +{ + if (!coroutine_info_table) + coroutine_info_table = hash_table<coroutine_info_hasher>::create_ggc (11); +} + /* Get the existing coroutine_info for FN_DECL, or insert a new one if the entry does not yet exist. */ -coroutine_info * +static coroutine_info * get_or_insert_coroutine_info (tree fn_decl) { gcc_checking_assert (coroutine_info_table != NULL); @@ -375,7 +385,7 @@ get_or_insert_coroutine_info (tree fn_decl) /* Get the existing coroutine_info for FN_DECL, fail if it doesn't exist. */ -coroutine_info * +static coroutine_info * get_coroutine_info (tree fn_decl) { if (coroutine_info_table == NULL) @@ -757,11 +767,7 @@ ensure_coro_initialized (location_t loc) if (!void_coro_handle_address) return false; - /* A table to hold the state, per coroutine decl. */ - gcc_checking_assert (coroutine_info_table == NULL); - coroutine_info_table = - hash_table<coroutine_info_hasher>::create_ggc (11); - + create_coroutine_info_table (); if (coroutine_info_table == NULL) return false; @@ -873,11 +879,13 @@ coro_promise_type_found_p (tree fndecl, location_t loc) coro_info->self_h_proxy = build_lang_decl (VAR_DECL, coro_self_handle_id, coro_info->handle_type); + DECL_CONTEXT (coro_info->self_h_proxy) = fndecl; /* Build a proxy for the promise so that we can perform lookups. */ coro_info->promise_proxy = build_lang_decl (VAR_DECL, coro_promise_id, coro_info->promise_type); + DECL_CONTEXT (coro_info->promise_proxy) = fndecl; /* Note where we first saw a coroutine keyword. */ coro_info->first_coro_keyword = loc; @@ -902,6 +910,17 @@ coro_get_ramp_function (tree decl) return NULL_TREE; } +/* Given a DECL, an actor or destroyer, build a link from that to the ramp + function. Used by modules streaming. */ + +void +coro_set_ramp_function (tree decl, tree ramp) +{ + if (!to_ramp) + to_ramp = hash_map<tree, tree>::create_ggc (10); + to_ramp->put (decl, ramp); +} + /* Given the DECL for a ramp function (the user's original declaration) return the actor function if it has been defined. */ @@ -926,6 +945,27 @@ coro_get_destroy_function (tree decl) return NULL_TREE; } +/* For a given ramp function DECL, set the actor and destroy functions. + This is only used by modules streaming. */ + +void +coro_set_transform_functions (tree decl, tree actor, tree destroy) +{ + /* Only relevant with modules. */ + gcc_checking_assert (modules_p ()); + + /* This should only be called for newly streamed declarations. */ + gcc_checking_assert (!get_coroutine_info (decl)); + + /* This might be the first use of coroutine info in the TU, so + create the coroutine info table if needed. */ + create_coroutine_info_table (); + + coroutine_info *coroutine = get_or_insert_coroutine_info (decl); + coroutine->actor_decl = actor; + coroutine->destroy_decl = destroy; +} + /* Given a CO_AWAIT_EXPR AWAIT_EXPR, return its resume call. */ tree @@ -4393,14 +4433,13 @@ coro_build_actor_or_destroy_function (tree orig, tree fn_type, = build_lang_decl (FUNCTION_DECL, copy_node (DECL_NAME (orig)), fn_type); /* Allow for locating the ramp (original) function from this one. */ - if (!to_ramp) - to_ramp = hash_map<tree, tree>::create_ggc (10); - to_ramp->put (fn, orig); + coro_set_ramp_function (fn, orig); DECL_CONTEXT (fn) = DECL_CONTEXT (orig); DECL_SOURCE_LOCATION (fn) = loc; DECL_ARTIFICIAL (fn) = true; DECL_INITIAL (fn) = error_mark_node; + DECL_COROUTINE_P (fn) = true; tree id = get_identifier ("frame_ptr"); tree fp = build_lang_decl (PARM_DECL, id, coro_frame_ptr); diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index 1a58ea0562ae..6bd5b1696c4f 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -9150,6 +9150,10 @@ extern tree coro_get_ramp_function (tree); extern tree co_await_get_resume_call (tree await_expr); +/* Only for use by modules. */ +extern void coro_set_transform_functions (tree, tree, tree); +extern void coro_set_ramp_function (tree, tree); + /* Inline bodies. */ inline tree diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc index 174bebff01c6..e6b2b2f78581 100644 --- a/gcc/cp/module.cc +++ b/gcc/cp/module.cc @@ -2969,6 +2969,7 @@ static char const *const merge_kind_name[MK_hwm] = /* Mergeable entity location data. */ struct merge_key { cp_ref_qualifier ref_q : 2; + unsigned coro_disc : 2; /* Discriminator for coroutine transforms. */ unsigned index; tree ret; /* Return type, if appropriate. */ @@ -2977,7 +2978,7 @@ struct merge_key { tree constraints; /* Constraints. */ merge_key () - :ref_q (REF_QUAL_NONE), index (0), + :ref_q (REF_QUAL_NONE), coro_disc (0), index (0), ret (NULL_TREE), args (NULL_TREE), constraints (NULL_TREE) { @@ -4590,6 +4591,17 @@ dumper::impl::nested_name (tree t) else fputs ("#null#", stream); + if (t && TREE_CODE (t) == FUNCTION_DECL && DECL_COROUTINE_P (t)) + if (tree ramp = DECL_RAMP_FN (t)) + { + if (DECL_ACTOR_FN (ramp) == t) + fputs (".actor", stream); + else if (DECL_DESTROY_FN (ramp) == t) + fputs (".destroy", stream); + else + gcc_unreachable (); + } + if (origin >= 0) { const module_state *module = (*modules)[origin]; @@ -11712,6 +11724,25 @@ trees_in::decl_container () return container; } +/* Gets a 2-bit discriminator to distinguish coroutine actor or destroy + functions from a normal function. */ + +static int +get_coroutine_discriminator (tree inner) +{ + if (DECL_COROUTINE_P (inner)) + if (tree ramp = DECL_RAMP_FN (inner)) + { + if (DECL_ACTOR_FN (ramp) == inner) + return 1; + else if (DECL_DESTROY_FN (ramp) == inner) + return 2; + else + gcc_unreachable (); + } + return 0; +} + /* Write out key information about a mergeable DEP. Does not write the contents of DEP itself. The context has already been written. The container has already been streamed. */ @@ -11803,6 +11834,7 @@ trees_out::key_mergeable (int tag, merge_kind mk, tree decl, tree inner, tree fn_type = TREE_TYPE (inner); key.ref_q = type_memfn_rqual (fn_type); + key.coro_disc = get_coroutine_discriminator (inner); key.args = TYPE_ARG_TYPES (fn_type); if (tree reqs = get_constraints (inner)) @@ -11939,7 +11971,12 @@ trees_out::key_mergeable (int tag, merge_kind mk, tree decl, tree inner, tree_node (name); if (streaming_p ()) { - unsigned code = (key.ref_q << 0) | (key.index << 2); + /* Check we have enough bits for the index. */ + gcc_checking_assert (key.index < (1u << (sizeof (unsigned) * 8 - 4))); + + unsigned code = ((key.ref_q << 0) + | (key.coro_disc << 2) + | (key.index << 4)); u (code); } @@ -11963,8 +12000,8 @@ trees_out::key_mergeable (int tag, merge_kind mk, tree decl, tree inner, } } -/* DECL is a new declaration that may be duplicated in OVL. Use RET & - ARGS to find its clone, or NULL. If DECL's DECL_NAME is NULL, this +/* DECL is a new declaration that may be duplicated in OVL. Use KEY + to find its clone, or NULL. If DECL's DECL_NAME is NULL, this has been found by a proxy. It will be an enum type located by its first member. @@ -12019,6 +12056,7 @@ check_mergeable_decl (merge_kind mk, tree decl, tree ovl, merge_key const &key) || same_type_p (key.ret, fndecl_declared_return_type (m_inner))) && type_memfn_rqual (m_type) == key.ref_q && compparms (key.args, TYPE_ARG_TYPES (m_type)) + && get_coroutine_discriminator (m_inner) == key.coro_disc /* Reject if old is a "C" builtin and new is not "C". Matches decls_match behaviour. */ && (!DECL_IS_UNDECLARED_BUILTIN (m_inner) @@ -12141,7 +12179,8 @@ trees_in::key_mergeable (int tag, merge_kind mk, tree decl, tree inner, merge_key key; unsigned code = u (); key.ref_q = cp_ref_qualifier ((code >> 0) & 3); - key.index = code >> 2; + key.coro_disc = (code >> 2) & 3; + key.index = code >> 4; if (mk == MK_enum) key.ret = tree_node (); @@ -12874,6 +12913,12 @@ has_definition (tree decl) if (use_tpl < 2) return true; } + + /* Coroutine transform functions always need to be emitted + into the importing TU if the ramp function will be. */ + if (DECL_COROUTINE_P (decl)) + if (tree ramp = DECL_RAMP_FN (decl)) + return has_definition (ramp); break; case TYPE_DECL: @@ -13057,6 +13102,17 @@ trees_out::write_function_def (tree decl) state->write_location (*this, f->function_start_locus); state->write_location (*this, f->function_end_locus); } + + if (DECL_COROUTINE_P (decl)) + { + tree ramp = DECL_RAMP_FN (decl); + tree_node (ramp); + if (!ramp) + { + tree_node (DECL_ACTOR_FN (decl)); + tree_node (DECL_DESTROY_FN (decl)); + } + } } void @@ -13072,13 +13128,13 @@ trees_in::read_function_def (tree decl, tree maybe_template) tree initial = tree_node (); tree saved = tree_node (); tree context = tree_node (); - constexpr_fundef cexpr; post_process_data pdata {}; pdata.decl = maybe_template; tree maybe_dup = odr_duplicate (maybe_template, DECL_SAVED_TREE (decl)); bool installing = maybe_dup && !DECL_SAVED_TREE (decl); + constexpr_fundef cexpr; if (u ()) { cexpr.parms = chained_decls (); @@ -13090,7 +13146,6 @@ trees_in::read_function_def (tree decl, tree maybe_template) cexpr.decl = NULL_TREE; unsigned flags = u (); - if (flags & 2) { pdata.start_locus = state->read_location (*this); @@ -13101,6 +13156,21 @@ trees_in::read_function_def (tree decl, tree maybe_template) pdata.infinite_loop = flags & 32; } + tree coro_actor = NULL_TREE; + tree coro_destroy = NULL_TREE; + tree coro_ramp = NULL_TREE; + if (DECL_COROUTINE_P (decl)) + { + coro_ramp = tree_node (); + if (!coro_ramp) + { + coro_actor = tree_node (); + coro_destroy = tree_node (); + if ((coro_actor == NULL_TREE) != (coro_destroy == NULL_TREE)) + set_overrun (); + } + } + if (get_overrun ()) return NULL_TREE; @@ -13116,6 +13186,11 @@ trees_in::read_function_def (tree decl, tree maybe_template) if (cexpr.decl) register_constexpr_fundef (cexpr); + if (coro_ramp) + coro_set_ramp_function (decl, coro_ramp); + else if (coro_actor && coro_destroy) + coro_set_transform_functions (decl, coro_actor, coro_destroy); + if (DECL_LOCAL_DECL_P (decl)) /* Block-scope OMP UDRs aren't real functions, and don't need a function structure to be allocated or to be expanded. */ @@ -17572,6 +17647,7 @@ module_state::read_cluster (unsigned snum) cfun->language->returns_null = pdata.returns_null; cfun->language->returns_abnormally = pdata.returns_abnormally; cfun->language->infinite_loop = pdata.infinite_loop; + cfun->coroutine_component = DECL_COROUTINE_P (decl); /* Make sure we emit explicit instantiations. FIXME do we want to do this in expand_or_defer_fn instead? */ diff --git a/gcc/testsuite/g++.dg/modules/coro-1_a.C b/gcc/testsuite/g++.dg/modules/coro-1_a.C new file mode 100644 index 000000000000..165c643fd7f5 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/coro-1_a.C @@ -0,0 +1,34 @@ +// { dg-do compile { target c++20 } } +// { dg-additional-options "-fmodules -fdump-lang-module" } +// { dg-module-cmi M } + +module; +#include <coroutine> +export module M; + +struct simple_promise; +struct simple_coroutine : std::coroutine_handle<simple_promise> { + using promise_type = ::simple_promise; +}; +struct simple_promise { + simple_coroutine get_return_object() { return { simple_coroutine::from_promise(*this) }; } + std::suspend_always initial_suspend() noexcept { return {}; } + std::suspend_always final_suspend() noexcept { return {}; } + void return_void() {} + void unhandled_exception() {} +}; +export simple_coroutine coroutine() { + co_return; +} +export inline simple_coroutine inline_coroutine() { + co_return; +} +export template <typename T> simple_coroutine template_coroutine() { + co_return; +} + +// The actor and destroy functions should have definitions streamed only for inline coroutines +// { dg-final { scan-lang-dump-not {Writing definition function_decl:'::coroutine.actor'} module } } +// { dg-final { scan-lang-dump-not {Writing definition function_decl:'::coroutine.destroy'} module } } +// { dg-final { scan-lang-dump {Writing definition function_decl:'::inline_coroutine.actor'} module } } +// { dg-final { scan-lang-dump {Writing definition function_decl:'::inline_coroutine.destroy'} module } } diff --git a/gcc/testsuite/g++.dg/modules/coro-1_b.C b/gcc/testsuite/g++.dg/modules/coro-1_b.C new file mode 100644 index 000000000000..e1384a7d6390 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/coro-1_b.C @@ -0,0 +1,19 @@ +// { dg-module-do run { target c++20 } } +// { dg-additional-options "-fmodules" } + +#include <coroutine> +import M; + +int main() { + auto a = coroutine(); + a.resume(); + a.destroy(); + + auto b = inline_coroutine(); + b.resume(); + b.destroy(); + + auto c = template_coroutine<int>(); + c.resume(); + c.destroy(); +}
