Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?
-- >8 --
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 were always considered
non-inline, even for an inline ramp function, which meant that modules
didn't realise it needed to stream a definition.
- 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 of proxies.
(coro_set_ramp_function): New function.
(coro_set_transform_functions): New function.
(coro_build_actor_or_destroy_function): Use
coro_set_ramp_function; copy linkage from original function.
* cp-tree.h (coro_set_transform_functions): Declare.
(coro_set_ramp_function): Declare.
* decl2.cc (mark_used): Mark transform functions as used if we
use the ramp function.
* module.cc (struct merge_key): New field coro_disc.
(struct post_process_data): New field coroutine_component.
(get_coroutine_discriminator): New function.
(trees_out::key_mergeable): Write coroutine discriminator.
(check_mergeable_decl): Adjust comment, check for matching
coroutine discriminator.
(trees_in::key_mergeable): Read coroutine discriminator.
(trees_out::write_function_def): Write coroutine_component and
ramp/actor/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.
Signed-off-by: Nathaniel Shead <[email protected]>
---
gcc/cp/coroutines.cc | 64 ++++++++++++++++----
gcc/cp/cp-tree.h | 4 ++
gcc/cp/decl2.cc | 13 +++++
gcc/cp/module.cc | 78 ++++++++++++++++++++++---
gcc/testsuite/g++.dg/modules/coro-1_a.C | 28 +++++++++
gcc/testsuite/g++.dg/modules/coro-1_b.C | 19 ++++++
6 files changed, 189 insertions(+), 17 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/modules/coro-1_a.C
create mode 100644 gcc/testsuite/g++.dg/modules/coro-1_b.C
diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index f0485a95073..930d453dde2 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_assert (modules_p ());
+
+ /* This should only be called for newly streamed declarations. */
+ gcc_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,15 +4433,19 @@ 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;
+ /* Copy linkage from the original function. */
+ TREE_PUBLIC (fn) = TREE_PUBLIC (orig);
+ DECL_DECLARED_INLINE_P (fn) = DECL_DECLARED_INLINE_P (orig);
+ DECL_NOT_REALLY_EXTERN (fn) = DECL_NOT_REALLY_EXTERN (orig);
+ DECL_INTERFACE_KNOWN (fn) = DECL_INTERFACE_KNOWN (orig);
+
tree id = get_identifier ("frame_ptr");
tree fp = build_lang_decl (PARM_DECL, id, coro_frame_ptr);
DECL_ARTIFICIAL (fp) = true;
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index b8470fc256c..1d40b387d6e 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -9144,6 +9144,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/decl2.cc b/gcc/cp/decl2.cc
index e807eab1b8a..d50864b8f75 100644
--- a/gcc/cp/decl2.cc
+++ b/gcc/cp/decl2.cc
@@ -6480,6 +6480,19 @@ mark_used (tree decl, tsubst_flags_t complain /* =
tf_warning_or_error */)
return false;
}
+ /* For coroutines, we need to mark the transform functions as used,
+ if they exist yet. */
+ if (flag_coroutines
+ && TREE_CODE (decl) == FUNCTION_DECL
+ && DECL_COROUTINE_P (decl)
+ && DECL_RAMP_P (decl))
+ {
+ if (tree actor = DECL_ACTOR_FN (decl))
+ mark_used (actor);
+ if (tree destroy = DECL_DESTROY_FN (decl))
+ mark_used (destroy);
+ }
+
/* If DECL has a deduced return type, we need to instantiate it now to
find out its type. For OpenMP user defined reductions, we need them
instantiated for reduction clauses which inline them by hand directly.
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index af0730ba974..67fb1e4a22d 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)
{
@@ -2999,6 +3000,7 @@ struct post_process_data {
bool returns_null;
bool returns_abnormally;
bool infinite_loop;
+ bool coroutine_component;
};
/* Tree stream reader. Note that reading a stream doesn't mark the
@@ -11696,6 +11698,24 @@ 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 (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. */
@@ -11787,6 +11807,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))
@@ -11923,7 +11944,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);
}
@@ -11947,8 +11973,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.
@@ -12008,6 +12034,9 @@ check_mergeable_decl (merge_kind mk, tree decl, tree
ovl, merge_key const &key)
&& (!DECL_IS_UNDECLARED_BUILTIN (m_inner)
|| !DECL_EXTERN_C_P (m_inner)
|| DECL_EXTERN_C_P (d_inner))
+ /* Reject if we're not the same kind of coroutine function. */
+ && (!flag_coroutines
+ || key.coro_disc == get_coroutine_discriminator (m_inner))
/* Reject if one is a different member of a
guarded/pre/post fn set. */
&& (!flag_contracts
@@ -12125,7 +12154,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 ();
@@ -13031,6 +13061,8 @@ trees_out::write_function_def (tree decl)
flags |= 8 * f->language->returns_null;
flags |= 16 * f->language->returns_abnormally;
flags |= 32 * f->language->infinite_loop;
+ /* Set for coroutines. */
+ flags |= 64 * f->coroutine_component;
}
u (flags);
@@ -13041,6 +13073,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 (f && f->coroutine_component)
+ {
+ tree ramp = DECL_RAMP_FN (decl);
+ tree_node (ramp);
+ if (!ramp)
+ {
+ tree_node (DECL_ACTOR_FN (decl));
+ tree_node (DECL_DESTROY_FN (decl));
+ }
+ }
}
void
@@ -13056,13 +13099,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 ();
@@ -13074,7 +13117,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);
@@ -13083,6 +13125,22 @@ trees_in::read_function_def (tree decl, tree
maybe_template)
pdata.returns_null = flags & 8;
pdata.returns_abnormally = flags & 16;
pdata.infinite_loop = flags & 32;
+ pdata.coroutine_component = flags & 64;
+ }
+
+ tree coro_actor = NULL_TREE;
+ tree coro_destroy = NULL_TREE;
+ tree coro_ramp = NULL_TREE;
+ if (pdata.coroutine_component)
+ {
+ 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 ())
@@ -13100,6 +13158,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. */
@@ -17556,6 +17619,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 = pdata.coroutine_component;
/* 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 00000000000..ec2c8300a80
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/coro-1_a.C
@@ -0,0 +1,28 @@
+// { dg-do compile { target c++20 } }
+// { dg-additional-options "-fmodules" }
+// { 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;
+}
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 00000000000..e1384a7d639
--- /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();
+}
--
2.51.0
