From a126a1577ffcbf62d97723b35d343bdff014bb40 Mon Sep 17 00:00:00 2001 From: Iain Sandoe Date: Tue, 14 Apr 2020 20:37:12 +0100 Subject: [PATCH] coroutines: Fix compile error with symmetric transfers [PR94359] For symmetric transfers to work with C++20 coroutines, it is currently necessary to tail call the callee coroutine from resume method of the caller coroutine. The current codegen marks these resume calls as "MUST_TAIL_CALL" to indicate that the tail call is required for correctness. Unfortunately, several targets have ABI constraints that prevent an indirect tail-call, which results in the PRs compile error. The change here tests the target sibcall hook for the resume expression and only marks it as requiring a tail call if that's supported. This doesn't fix the underlying problem; that really a solution is needed to allow the tail-calls (or equivalent) to take place - but that will be deferred until next stage 1. gcc/cp/ChangeLog: 2020-04-14 Iain Sandoe PR c++/94359 * coroutines.cc (build_actor_fn): Check that the target can support the resume tailcall before mandating it. gcc/testsuite/ChangeLog: 2020-04-14 Iain Sandoe PR c++/94359 * g++.dg/coroutines/torture/symmetric-transfer-00-basic.C: Expect a run fail for targets without arbitrary indirect tail-calls. --- gcc/cp/ChangeLog | 6 +++++ gcc/cp/coroutines.cc | 22 +++++++++++++------ gcc/testsuite/ChangeLog | 7 ++++++ .../torture/symmetric-transfer-00-basic.C | 8 ++++++- 4 files changed, 35 insertions(+), 8 deletions(-) diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog index e34ef9303a4..4547674190e 100644 --- a/gcc/cp/ChangeLog +++ b/gcc/cp/ChangeLog @@ -1,3 +1,9 @@ +2020-04-14 Iain Sandoe + + PR c++/94359 + * coroutines.cc (build_actor_fn): Check that the target can + support the resume tailcall before mandating it. + 2020-04-14 Patrick Palka PR c++/85278 diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc index 57172853639..e4ba642d527 100644 --- a/gcc/cp/coroutines.cc +++ b/gcc/cp/coroutines.cc @@ -2376,14 +2376,22 @@ build_actor_fn (location_t loc, tree coro_frame_type, tree actor, tree fnbody, tree resume = build_call_expr_loc (loc, builtin_decl_explicit (BUILT_IN_CORO_RESUME), 1, addr); + /* In order to support an arbitrary number of coroutine continuations, + we must tail call them. However, some targets might not support this + for indirect calls, or calls between DSOs. + FIXME: see if there's an alternate strategy for such targets. */ /* Now we have the actual call, and we can mark it as a tail. */ CALL_EXPR_TAILCALL (resume) = true; - /* ... and for optimisation levels 0..1, mark it as requiring a tail-call - for correctness. It seems that doing this for optimisation levels that - normally perform tail-calling, confuses the ME (or it would be logical - to put this on unilaterally). */ - if (optimize < 2) - CALL_EXPR_MUST_TAIL_CALL (resume) = true; + /* Temporarily, switch cfun so that we can use the target hook. */ + push_struct_function (actor); + if (targetm.function_ok_for_sibcall (NULL_TREE, resume)) + { + /* ... and for optimisation levels 0..1, which do not normally tail- + -call, mark it as requiring a tail-call for correctness. */ + if (optimize < 2) + CALL_EXPR_MUST_TAIL_CALL (resume) = true; + } + pop_cfun (); resume = coro_build_cvt_void_expr_stmt (resume, loc); add_stmt (resume); @@ -3951,7 +3959,7 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer) push_deferring_access_checks (dk_no_check); - /* Actor ... */ + /* Build the actor... */ build_actor_fn (fn_start, coro_frame_type, actor, fnbody, orig, param_uses, &local_var_uses, param_dtor_list, initial_await, final_await, body_aw_points.await_number, frame_size); diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index c663b25e13c..3a3a1c59465 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,10 @@ +2020-04-14 Iain Sandoe + + PR c++/94359 + * g++.dg/coroutines/torture/symmetric-transfer-00-basic.C: + Expect a run fail for targets without arbitrary indirect + tail-calls. + 2020-04-14 Patrick Palka PR c++/93207 diff --git a/gcc/testsuite/g++.dg/coroutines/torture/symmetric-transfer-00-basic.C b/gcc/testsuite/g++.dg/coroutines/torture/symmetric-transfer-00-basic.C index 864846e365c..6f379c8e77a 100644 --- a/gcc/testsuite/g++.dg/coroutines/torture/symmetric-transfer-00-basic.C +++ b/gcc/testsuite/g++.dg/coroutines/torture/symmetric-transfer-00-basic.C @@ -1,4 +1,10 @@ -// { dg-do run } +// { dg-do run } +// See PR94359 - some targets are unable to make general indirect tailcalls +// for example, between different DSOs. +// { dg-xfail-run-if "" { hppa*-*-hpux11* } } +// { dg-xfail-run-if "" { ia64-*-linux-gnu } } +// { dg-xfail-run-if "" { { lp64 && { powerpc*-linux-gnu } } || { *-*-aix* } } } +// { dg-xfail-run-if "" { sparc*-*-* } } #if __has_include() -- 2.30.2