coroutines: Fix compile error with symmetric transfers [PR94359]

Message ID ED4B8FAB-35C3-439A-B491-A303993063E4@sandoe.co.uk
State New
Headers show
Series
  • coroutines: Fix compile error with symmetric transfers [PR94359]
Related show

Commit Message

Iain Sandoe April 11, 2020, 2:46 p.m.
Hi Folks,
sorry for the long CC list - please feel free to ignore if you don’t care :)

I propose that this PR should be re-categorized as a “C++” one.

The reason is that this is not an oversight in the GCC implementation,
but a problem present in the general case.  Library implementors feel
strongly that absence of symmetric transfer on important targets is a 
Bad Thing.

-- 
2.17.1

Comments

Nathan Sidwell April 13, 2020, 2:18 p.m. | #1
On 4/11/20 10:46 AM, Iain Sandoe wrote:
> Hi Folks,

> sorry for the long CC list - please feel free to ignore if you don’t care :)

> 

> I propose that this PR should be re-categorized as a “C++” one.

> 

> The reason is that this is not an oversight in the GCC implementation,

> but a problem present in the general case.  Library implementors feel

> strongly that absence of symmetric transfer on important targets is a

> Bad Thing.

> 

> =======   possibilities to resolve this …..

> 

> We have, in the case of coroutine tail-calls, a useful factor in that all

> such calls pass a single pointer, to the coroutine state frame.  That

> frame is initially set up in the DSO that spawns a given coroutine actor

> function.  Thus, at the point the frame is built, we have access to the

> TOC, GOT, PLT for the relevant DSO.

> 

> It would seem excessive to resort to some kind of trampoline when we

> already have somewhere to put the data we need to restore.

> 

> So .. what I’d like to do is to prototype a solution (probably on PPC) and

> then take the necessary (coroutine) ABI amendments to the “coroutines

> ABI group” (I am the editor of the current doc - which doesn’t have any

> ps-component).

> 

> ======= band-aid to fix the PR for stage 4.

> 

> tested on x86_64-linux, darwin (no loss of tail-call)

> powerpc64-linux-gnu (tail call is OK on m32, and bypassed on m64)

> solaris2.11 (tail call is bypassed on m32 and 64).

> 

> OK for master?

> thanks

> Iain

> 

> ======= this is the commit message.

> 

> 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.


This is fine from my PoV for gcc 10.

nathan

-- 
Nathan Sidwell
Rainer Orth April 13, 2020, 2:25 p.m. | #2
Hi Iain,

> 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..8211e8250ff 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,5 @@

> -//  { dg-do run }

> +// { dg-do run }

> +// { dg-xfail-run-if "no indirect tailcall" { { lp64 && { powerpc64*-linux-gnu } } || { *-*-solaris2* *-*-aix* } } }

>  

>  #if __has_include(<coroutine>)


unfortunately, the dg-xfail-run-if is wrong.  E.g. it causes XPASSes on
i386-pc-solaris2.11.

You should base this on the cpu part of the triplet in general, not on
the OS.  Besides, according to gcc-testresults postings, the test FAILs
on other targets as well: armv8l-unknown-linux-gnueabihf, hppa*, and ia64.

Besides, unless you want to introduce an effective-target keyword (with
documentation in sourcebuild.texi), probably overkill for a single use,
you can have more than one dg-xfail-run-if line to improve readibility.

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University
Iain Sandoe April 13, 2020, 2:37 p.m. | #3
Hi Rainer,

Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> wrote:

>> 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..8211e8250ff 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,5 @@

>> -//  { dg-do run }

>> +// { dg-do run }

>> +// { dg-xfail-run-if "no indirect tailcall" { { lp64 && {  

>> powerpc64*-linux-gnu } } || { *-*-solaris2* *-*-aix* } } }

>>

>> #if __has_include(<coroutine>)

>

> unfortunately, the dg-xfail-run-if is wrong.  E.g. it causes XPASSes on

> i386-pc-solaris2.11.


.. so that behaves in a similar way to PPC?
fail on m64 pass on m32?
(I don’t have access to any x86 solaris testing)

according to my testing, sparc solaris fails for m32 and m64 (so the  
condition doesn’t need any multilib discriminator there)

> You should base this on the cpu part of the triplet in general, not on

> the OS.  Besides, according to gcc-testresults postings, the test FAILs

> on other targets as well: armv8l-unknown-linux-gnueabihf, hppa*, and ia64.

>

> Besides, unless you want to introduce an effective-target keyword (with

> documentation in sourcebuild.texi), probably overkill for a single use,

> you can have more than one dg-xfail-run-if line to improve readibility.


I’ll take a look at those  and make multiple xfail-run-if’s
(one per target might be the neatest, and easier for a target maintainer to  
add if one is missed).

thanks
Iain
Rainer Orth April 13, 2020, 2:41 p.m. | #4
Hi Iain,

> Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> wrote:

>

>>> 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..8211e8250ff 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,5 @@

>>> -//  { dg-do run }

>>> +// { dg-do run }

>>> +// { dg-xfail-run-if "no indirect tailcall" { { lp64 && {

>>> powerpc64*-linux-gnu } } || { *-*-solaris2* *-*-aix* } } }

>>>

>>> #if __has_include(<coroutine>)

>>

>> unfortunately, the dg-xfail-run-if is wrong.  E.g. it causes XPASSes on

>> i386-pc-solaris2.11.

>

> .. so that behaves in a similar way to PPC?

> fail on m64 pass on m32?

> (I don’t have access to any x86 solaris testing)


no, it PASSes for 32 and 64-bit alike, just as e.g. Linux/x86_64.

> according to my testing, sparc solaris fails for m32 and m64 (so the

> condition doesn’t need any multilib discriminator there)


Right, same here.

>> You should base this on the cpu part of the triplet in general, not on

>> the OS.  Besides, according to gcc-testresults postings, the test FAILs

>> on other targets as well: armv8l-unknown-linux-gnueabihf, hppa*, and ia64.

>>

>> Besides, unless you want to introduce an effective-target keyword (with

>> documentation in sourcebuild.texi), probably overkill for a single use,

>> you can have more than one dg-xfail-run-if line to improve readibility.

>

> I’ll take a look at those  and make multiple xfail-run-if’s

> (one per target might be the neatest, and easier for a target maintainer to

> add if one is missed).


I thought about separate ones for cases that need additional conditions
and an alphabetical list of target triplets (just <cpu wildcard>-*-*)
for the rest.

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University
Segher Boessenkool April 14, 2020, 9:23 a.m. | #5
Hi!

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.


> 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..8211e8250ff 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,5 @@

> -//  { dg-do run }

> +// { dg-do run }

> +// { dg-xfail-run-if "no indirect tailcall" { { lp64 && { powerpc64*-linux-gnu } } || { *-*-solaris2* *-*-aix* } } }


lp64 && powerpc*-*-linux (we have biarch compilers :-) )

The problem is not that there is no indirect tailcall; the problem is
that no tailcall can be done to a routine that (potentially) has a
different TOC.

From rs6000_function_ok_for_sibcall:
  Under the AIX or ELFv2 ABIs we can't allow calls to non-local
  functions, because the callee may have a different TOC pointer to
  the caller and there's no way to ensure we restore the TOC when
  we return.


Segher
Rainer Orth April 14, 2020, 9:57 a.m. | #6
Hi Segher,

> 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.

>

>> 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..8211e8250ff 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,5 @@

>> -//  { dg-do run }

>> +// { dg-do run }

>> +// { dg-xfail-run-if "no indirect tailcall" { { lp64 && {

>> powerpc64*-linux-gnu } } || { *-*-solaris2* *-*-aix* } } }

>

> lp64 && powerpc*-*-linux (we have biarch compilers :-) )

>

> The problem is not that there is no indirect tailcall; the problem is

> that no tailcall can be done to a routine that (potentially) has a

> different TOC.

>

> From rs6000_function_ok_for_sibcall:

>   Under the AIX or ELFv2 ABIs we can't allow calls to non-local

>   functions, because the callee may have a different TOC pointer to

>   the caller and there's no way to ensure we restore the TOC when

>   we return.


so shouldn't the above be

  lp64 && powerpc*-*-*

instead to cover AIX, too?  Or are there other PowerPC ABIs that don't
have this issue?

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University
Iain Sandoe April 14, 2020, 10:14 a.m. | #7
Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> wrote:

> Hi Segher,

>

>> 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.

>>

>>> 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..8211e8250ff 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,5 @@

>>> -//  { dg-do run }

>>> +// { dg-do run }

>>> +// { dg-xfail-run-if "no indirect tailcall" { { lp64 && {

>>> powerpc64*-linux-gnu } } || { *-*-solaris2* *-*-aix* } } }

>>

>> lp64 && powerpc*-*-linux (we have biarch compilers :-) )

>>

>> The problem is not that there is no indirect tailcall; the problem is

>> that no tailcall can be done to a routine that (potentially) has a

>> different TOC.

>>

>> From rs6000_function_ok_for_sibcall:

>>  Under the AIX or ELFv2 ABIs we can't allow calls to non-local

>>  functions, because the callee may have a different TOC pointer to

>>  the caller and there's no way to ensure we restore the TOC when

>>  we return.

>

> so shouldn't the above be

>

>  lp64 && powerpc*-*-*

>

> instead to cover AIX, too?  Or are there other PowerPC ABIs that don't

> have this issue?


Just for once, Darwin (both PPC and X86) has no problem (doesn’t use the TOC
and has a per-function “got”), so that’s one.

Iain
Iain Sandoe April 14, 2020, 2:02 p.m. | #8
Hi Rainer,

Rainer Orth <ro@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* } }

* I don’t see the armv8l- fail in the results.
* hppa-linux seems OK.
* I’m avoiding writing powerpc and rs6000 aix, since I expect the result to  
be
the same for both.

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)

happy to test on the platforms I have access to if this seems a reasonable  
start
thanks
Iain
Rainer Orth April 14, 2020, 2:12 p.m. | #9
Hi Iain,

> Rainer Orth <ro@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.

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University
Iain Sandoe April 14, 2020, 7:49 p.m. | #10
Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> wrote:

>> Rainer Orth <ro@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  <iain@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  <iain@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>)

Patch

=======   possibilities to resolve this …..

We have, in the case of coroutine tail-calls, a useful factor in that all
such calls pass a single pointer, to the coroutine state frame.  That 
frame is initially set up in the DSO that spawns a given coroutine actor
function.  Thus, at the point the frame is built, we have access to the 
TOC, GOT, PLT for the relevant DSO.

It would seem excessive to resort to some kind of trampoline when we
already have somewhere to put the data we need to restore.

So .. what I’d like to do is to prototype a solution (probably on PPC) and
then take the necessary (coroutine) ABI amendments to the “coroutines
ABI group” (I am the editor of the current doc - which doesn’t have any 
ps-component).

======= band-aid to fix the PR for stage 4.

tested on x86_64-linux, darwin (no loss of tail-call)
powerpc64-linux-gnu (tail call is OK on m32, and bypassed on m64)
solaris2.11 (tail call is bypassed on m32 and 64).

OK for master?
thanks
Iain

======= this is the commit message.

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-09  Iain Sandoe  <iain@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-09  Iain Sandoe  <iain@sandoe.co.uk>

	* g++.dg/coroutines/torture/symmetric-transfer-00-basic.C:
	Expect a run fail for targets without indirect tail-calls.
---
 gcc/cp/coroutines.cc                          | 22 +++++++++++++------
 .../torture/symmetric-transfer-00-basic.C     |  3 ++-
 2 files changed, 17 insertions(+), 8 deletions(-)

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..8211e8250ff 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,5 @@ 
-//  { dg-do run }
+// { dg-do run }
+// { dg-xfail-run-if "no indirect tailcall" { { lp64 && { powerpc64*-linux-gnu } } || { *-*-solaris2* *-*-aix* } } }
 
 #if __has_include(<coroutine>)