Rainer Orth <r...@cebitec.uni-bielefeld.de> wrote:
>> Rainer Orth <r...@cebitec.uni-bielefeld.de> wrote:
>>>> On Sat, Apr 11, 2020 at 03:46:18PM +0100, Iain Sandoe wrote:
>>>>> Unfortunately, several targets have ABI constraints that prevent
>>>>> an indirect tail-call, which results in the PRs compile error.
>>
>> So .. after scanning the current published testsuite results I have:
>>
>> // 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-*-solaris2* } }
>
> this is wrong: for one, it needs to allow for 64-bit-default SPARC
> configurations (i.e. sparcv9-*-* and sparc64-*-*), and AFAICS there's
> nothing Solaris-specific here. So this should be sparc*-*-* instead.
>
>> I’d say it would be a reasonable idea to get this in sooner rather than
>> later, we
>> can always increase the list (or a target maintainer can trivially add
>> their target)
>
> Fine with me. If the list gets much longer or the conditions more
> complex, it might still be worthwhile to use an effective-target keyword
> instead.
This is what I’ve pushed after re-checking on
x86_64-linux/darwin
sparc-solaris11
powerpc64-linux-gnu
I won’t be entirely surprised if the list needs amendment or additions
but that’s realistically what I can test.
thanks for the reviews,
Iain
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 <i...@sandoe.co.uk>
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 <i...@sandoe.co.uk>
PR c++/94359
* g++.dg/coroutines/torture/symmetric-transfer-00-basic.C:
Expect a run fail for targets without arbitrary indirect
tail-calls.
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/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(<coroutine>)
