[RFC][Coroutine] Force stack allocation after await_suspend() call

[lxfind]

Summary

One of the challenges with the alloca analysis in CoroSplit is that in a few cases we need to make sure the allocas must be put on the stack, not on the frame.
One of the cases is symmetric transfer. Symmetric transfer is a newly introduced feature in C++ coroutines that allows for immediate transfer to a different coroutine when the current coroutine is suspended.
The await_suspend() call will return a coroutine handle type, and when that happens, the compiler should generate code to resume the returned handle. Like this:

coroutine_handle tmp = awaiter.await_suspend();
__builtin_coro_resume(tmp.address());

It's very common that after the call to await_suspend(), the current coroutine frame is already destroyed, which means we should not be accessing the coroutine frame from there.
And we shouldn't because we we use here is a temporary variable which will be short-lived. However in a debug build when we don't have lifetime intrinsics, it's very hard for the compiler to determine that tmp doesn't escape. This bug can be reproduced in this example: https://godbolt.org/z/KvPY66
It results in a TSAN failure because we are accessing the heap after it's destroyed.

There are two specific challenges here:

1. If the address() function call is not inlined (this should be the default case with -O0), we will have a function call that takes tmp as a pointer. The compiler does not know that the address call will not capture. This will lead to tmp being put on the frame. We could potentially special handle the address function in either front-end or CoroSplit, but both are fragile (we will need to do some name pattern matching).
2. If the address() function call is inlined (in some versions of libc++, address seems to have "always_inline" attribute), we will end up with a series of store/load instructions. For a naive analysis, a store of the pointer will also be treated as escape. To solve that problem, I introduced D91305, which tries to match this specific store/load pattern and be able to deal with it. It looks very hacky.

To solve this problem once for all, and provide a framework for solving similar problems in the future, this patch introduces 2 new intrinsics to mark a region where all data accessed must be put on the stack.
In the case of symmetric transfer, in order to be able to insert code during front-end codegen right after the await_suspend call, we need to split the Suspend subnode CoroutineSuspendExpr at await_suspend call, as the new AwaitSuspendCall subnode.
Then we create a OpaqueValueExpr to wrap around AwaitSuspendCall, and use it to continue build the rest of the Suspend subnode. OpaqueValueExpr is necessary because we don't want to emit the await_suspend call twice. OpaqueValueExpr serves as a stopper in codegen.
If there is no symmetric transfer, the new nodes will be nullptr.
After this patch, now right after the await_suspend() call, we will see a llvm.coro.forcestack.begin() intrinsic, and then right before coro.suspend(), we will see a llvm.coro.forcestack.end() intrinsic.
CoroSplit will then be able to use this information to decide whether some data must be put on the stack.
We are also able to remove the code that tries to match the special store/load instruction sequence.

[ChuanqiXu]

It looks like there are two things this patch wants to do:

1. Don't put the temporary generated by symmetric-transfer on the coroutine frame.
2. Offer a mechanism to force some values (it is easy to extend Alloca to Value) to put in the stack instead of the coroutine frame.

I am a little confused about the first problem. Would it cause the program to crash? (e.g., we access the fields of coroutine frame after the frame gets destroyed). Or it just wastes some storage?
And I want to ask about the change of the AST nodes and SemaCoroutine. Can we know if a CoroutineSuspendExpr stands for a symmetric-transfer? If yes, it seems we can only do changes in CodeGen part.

Then I agree to introduce new intrinsic to hint the middle end to put some values on the stack. And the design of @llvm.coro.forcestack.begin() and @llvm.coro.forcestack.end() is a little strange to me. It says they mark a region where only data from the local stack can be accessed. But it looks error-prone since it is hard for the front-end to decide whether all the access of the region should be put on the stack. I think we could introduce only one intrinisic @llvm.coro.forcestack(Value* v), we can use the argument to mark the value need to be put on the stack.

And about the problem you mentioned in D96922: "The lifetime of %coro.gro" starts early and %coro.gro" would be used after coro.end (Possibly the destructor?) which would cause the program to access destroyed coroutine frame". It looks like the mechanism could solve this problem by a call to @llvm.coro.forcestack(%coro.gro).

clang/include/clang/AST/ExprCXX.h

4695

It looks strange for the change of CoroutineSuspendExpr at the first glance. It is easy to understand the coroutine suspend expression is consists of three parts: Ready, Suspend and resume. It is written in the language documentation. And the new added AwaitSuspendCall is confusing.

[lxfind]

In D98638#2628082, @ChuanqiXu wrote:

It looks like there are two things this patch wants to do:

1. Don't put the temporary generated by symmetric-transfer on the coroutine frame.
2. Offer a mechanism to force some values (it is easy to extend Alloca to Value) to put in the stack instead of the coroutine frame.

I am a little confused about the first problem. Would it cause the program to crash? (e.g., we access the fields of coroutine frame after the frame gets destroyed). Or it just wastes some storage?
And I want to ask about the change of the AST nodes and SemaCoroutine. Can we know if a CoroutineSuspendExpr stands for a symmetric-transfer? If yes, it seems we can only do changes in CodeGen part.

It will result in a crash, because we will be accessing memory that's already freed. If you run:

bin/clang -fcoroutines-ts -std=c++14 -stdlib=libc++ ../clang/test/CodeGenCoroutines/coro-symmetric-transfer-01.cpp -o - -emit-llvm -S -Xclang -disable-llvm-passes

You can see that in the final.suspend basic block, there are IRs like this:

  %call19 = call i8* @_ZN13detached_task12promise_type13final_awaiter13await_suspendENSt12experimental13coroutines_v116coroutine_handleIS0_EE(%"struct.detached_task::promise_type::final_awaiter"* nonnull dereferenceable(1) %ref.tm
p10, i8* %22) #2
  %coerce.dive20 = getelementptr inbounds %"struct.std::experimental::coroutines_v1::coroutine_handle.0", %"struct.std::experimental::coroutines_v1::coroutine_handle.0"* %coerce, i32 0, i32 0
  store i8* %call19, i8** %coerce.dive20, align 8
  %call21 = call i8* @_ZNKSt12experimental13coroutines_v116coroutine_handleIvE7addressEv(%"struct.std::experimental::coroutines_v1::coroutine_handle.0"* nonnull dereferenceable(8) %coerce) #2
  call void @llvm.coro.resume(i8* %call21)

The temporary variable %coerce will be put on the frame because it's used by the call to address function and LLVM thinks it may escape. But the call to await_suspend() (the first line) in reality could destroy the current coroutine frame. Hence after the call to await_suspend, it will be accessing the frame, leading to memory corruption.

Then I agree to introduce new intrinsic to hint the middle end to put some values on the stack. And the design of @llvm.coro.forcestack.begin() and @llvm.coro.forcestack.end() is a little strange to me. It says they mark a region where only data from the local stack can be accessed. But it looks error-prone since it is hard for the front-end to decide whether all the access of the region should be put on the stack. I think we could introduce only one intrinisic @llvm.coro.forcestack(Value* v), we can use the argument to mark the value need to be put on the stack.

This is a good idea. Let me play with it. Thanks!

And about the problem you mentioned in D96922: "The lifetime of %coro.gro" starts early and %coro.gro" would be used after coro.end (Possibly the destructor?) which would cause the program to access destroyed coroutine frame". It looks like the mechanism could solve this problem by a call to @llvm.coro.forcestack(%coro.gro).

[lxfind]

clang/include/clang/AST/ExprCXX.h

4695

I agree. But this seems to be the only way to break up Suspend at the point of await_suspend call so that we can insert instructions during CodeGen. Open to ideas though.

[lxfind]

In D98638#2628082, @ChuanqiXu wrote:

I am a little confused about the first problem. Would it cause the program to crash? (e.g., we access the fields of coroutine frame after the frame gets destroyed). Or it just wastes some storage?

This is a repro of the crash (in TSAN mode): https://godbolt.org/z/KvPY66

[lxfind]

clang/include/clang/AST/ExprCXX.h

4695

One potential way to make this more clear is to rename these two nodes as: Suspend and Transfer.

[ChuanqiXu]

In D98638#2630607, @lxfind wrote:

In D98638#2628082, @ChuanqiXu wrote:

I am a little confused about the first problem. Would it cause the program to crash? (e.g., we access the fields of coroutine frame after the frame gets destroyed). Or it just wastes some storage?

This is a repro of the crash (in TSAN mode): https://godbolt.org/z/KvPY66

Oh I got it. The program would crash since handle is destroyed in final_awaiter::await_suspend explicitly:

std::coroutine_handle<> await_suspend(std::coroutine_handle<promise_type> h) noexcept {
       h.destroy();
       return std::noop_coroutine();
}

And the normal symmetric transfer wouldn't destroy the handle (although it depends on the implementation of await_suspend).
So the problem met in this patch is program maybe crash with symmetric transfer with destroy coroutine handle explicitly (in the final awaiter normally) instead of normal symmetric transfer.

The explicitly destruction for the coroutine handle in the await_suspend of final awaiter is a normal pattern to enable the Coro-elide optimization. There is a discuss before in cafe-dev: http://clang-developers.42468.n3.nabble.com/Miscompilation-heap-use-after-free-in-C-coroutines-td4070320.html. It looks like it is a subsequent problem.

Here what I want to say is we shouldn't handle all the symmetric transfer from the above analysis. And we shouldn't change the ASTNodes and Sema part. We need to solve about the above pattern. It is not easy to give a solution since user could implement symmetric transfer in final awaiter without destroying the handle, which is more common.

My unfinished idea is to emit an intrinsic called @llvm.coro.finalize before we emit the promise_type::final_suspend. Then the @llvm.coro.finalize marks the end of the lifetime for current coroutine frame. And all the analysis in CoroFrame shouldn't consider use after @llvm.coro.finalize (We could emit warning for some cases). But this idea is also problematic, it makes the semantics of coroutine intrinsic more chaos. Just image that how a newbie feels when he see @llvm.coro.end, @llvm.coro.destroy and @llvm.coro.finalize. And we can't use @llvm.coro.end @llvm.coro.destroy since they have other semantics (llvm.coro.destroy means deletion and llvm.coro.end would be used to split the coroutine). Also, the idea of @llvm.coro.finalize seems available to solve the problem about%gro mentioned above.

It seems to be a workaround to use @llvm.coro.forcestack(%result_of_final_await_suspend) . Since I wondering if there are other corner cases as the %gro. My opinion about '@llvm.coro.forcestack' is that we could use it as a patch if we find any holes that is hard to handle immediately. But we also need to find a solution to solve problems more fundamentally.

[lxfind]

Here what I want to say is we shouldn't handle all the symmetric transfer from the above analysis. And we shouldn't change the ASTNodes and Sema part. We need to solve about the above pattern. It is not easy to give a solution since user could implement symmetric transfer in final awaiter without destroying the handle, which is more common.

Just to clarify, in case there are any confusions around this. This patch would work no matter whether the coroutine frame is destroyed or not during await_suspend(). It simply makes sure that the temporary handle returned by await_suspend will be put in the stack instead of heap, and it will always be safe to do so, no matter what happens.
Whether or not the current coroutine frame would be destroyed completely depend on the implementation of await_suspend. So we cannot predict or know in advance. Therefore, the temporary handle returned by await_suspend must be put on the stack. I don't really see any other solutions other than this.

It seems to be a workaround to use @llvm.coro.forcestack(%result_of_final_await_suspend) . Since I wondering if there are other corner cases as the %gro. My opinion about '@llvm.coro.forcestack' is that we could use it as a patch if we find any holes that is hard to handle immediately. But we also need to find a solution to solve problems more fundamentally.

Yes as I mentioned in the description, there are really only two cases, one is after await_suspend call, and one is gro. gro is easy to handle and I will likely send a separate patch latter. But this problem with await_suspend is particularly challenging to solve.

What do you think is the fundamental problem, though?

[lxfind]

Well, I guess another potential solution is to force emitting lifetime intrinsics for this part of coroutine in the front-end.
Like this:

diff --git a/clang/lib/CodeGen/CGDecl.cpp b/clang/lib/CodeGen/CGDecl.cpp
index 243d93a8c165..ef76e8dcb7c9 100644
--- a/clang/lib/CodeGen/CGDecl.cpp
+++ b/clang/lib/CodeGen/CGDecl.cpp
@@ -1317,7 +1317,7 @@ void CodeGenFunction::EmitAutoVarDecl(const VarDecl &D) {
 /// otherwise
 llvm::Value *CodeGenFunction::EmitLifetimeStart(uint64_t Size,
                                                 llvm::Value *Addr) {
-  if (!ShouldEmitLifetimeMarkers)
+  if (!ShouldEmitLifetimeMarkers && !isCoroutine())
     return nullptr;
 
   assert(Addr->getType()->getPointerAddressSpace() ==
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index 18f1468dcb86..2e6e6808db7f 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -535,7 +535,7 @@ EmitMaterializeTemporaryExpr(const MaterializeTemporaryExpr *M) {
       break;
 
     case SD_FullExpression: {
-      if (!ShouldEmitLifetimeMarkers)
+      if (!ShouldEmitLifetimeMarkers && !isCoroutine())
         break;
 
       // Avoid creating a conditional cleanup just to hold an llvm.lifetime.end

[ChuanqiXu]

In D98638#2630778, @lxfind wrote:

What do you think is the fundamental problem, though?

It is hard to give a formal description for the problem. Let me try to explain it.
What I want to say here is about rules that decide whether a value should be put on the coroutine frame.
Initially, we put values on the frame for whose uses are crossing suspend points with their definition.
Then, we put values on the frame for whose uses are crossing suspend points with their definition and uses are not escaped.
In this patch, we want to put values on the frame for whose uses are crossing suspend points with their definition and uses are not escaped but except the result of symmetric transfer and %gro.
Then we need to answer the question: how can we prove that the result of symmetric transfer and %gro are the only exceptions from the above rules. Or how can we know the list of exceptions wouldn't get longer and longer in the future?

Then go back to the example in the summary. From my point of view, the key problem is that our escape analysis isn't powerful enough. I don't ask us to do excellent escape analysis. It may beyond our abilities. I just want to say how can we know the result of symmetric transfer and %gro are the only exceptions.

In D98638#2630778, @lxfind wrote:

Whether or not the current coroutine frame would be destroyed completely depend on the implementation of await_suspend. So we cannot predict or know in advance. Therefore, the temporary handle returned by await_suspend must be put on the stack. I don't really see any other solutions other than this.

OK. Although the main stream implementation of await_suspend only destroy the coroutine handle in the final awaiter, the compiler can't assume the normal await_suspend won't destroy it. So I agree to guard the result of the await_suspend to make it put on the stack. At least, it would reduce the size of the coroutine frame.

Then if we want to put the result of the await_suspend in the stack, I think we can do it under CodeGen part only. It should be easy to judge the return type of await_suspend and create a call to llvm.coro.forcestack to the return value of await_suspend.

In D98638#2630778, @lxfind wrote:

Well, I guess another potential solution is to force emitting lifetime intrinsics for this part of coroutine in the front-end.

I am not sure if this is a good idea. May it break the guide principle in LLVM? This need to be reviewed by others.

[lxfind]

Then we need to answer the question: how can we prove that the result of symmetric transfer and %gro are the only exceptions from the above rules. Or how can we know the list of exceptions wouldn't get longer and longer in the future?

Then go back to the example in the summary. From my point of view, the key problem is that our escape analysis isn't powerful enough. I don't ask us to do excellent escape analysis. It may beyond our abilities. I just want to say how can we know the result of symmetric transfer and %gro are the only exceptions.

That's a fair point. I agree that we have no guarantee these are the only two cases.
It does seem to me that coroutine implementation somewhat relies on proper lifetime markers so that data are being put correctly, which may be the fundamental problem we are trying to solve.

In D98638#2630778, @lxfind wrote:

Whether or not the current coroutine frame would be destroyed completely depend on the implementation of await_suspend. So we cannot predict or know in advance. Therefore, the temporary handle returned by await_suspend must be put on the stack. I don't really see any other solutions other than this.

OK. Although the main stream implementation of await_suspend only destroy the coroutine handle in the final awaiter, the compiler can't assume the normal await_suspend won't destroy it. So I agree to guard the result of the await_suspend to make it put on the stack. At least, it would reduce the size of the coroutine frame.

Then if we want to put the result of the await_suspend in the stack, I think we can do it under CodeGen part only. It should be easy to judge the return type of await_suspend and create a call to llvm.coro.forcestack to the return value of await_suspend.

In D98638#2630778, @lxfind wrote:

Well, I guess another potential solution is to force emitting lifetime intrinsics for this part of coroutine in the front-end.

I am not sure if this is a good idea. May it break the guide principle in LLVM? This need to be reviewed by others.

[lxfind]

Then if we want to put the result of the await_suspend in the stack, I think we can do it under CodeGen part only. It should be easy to judge the return type of await_suspend and create a call to llvm.coro.forcestack to the return value of await_suspend.

We probably could, but it would be very very tedious.
During CodeGen, we only have the AST that's calling __builtin_coro_resume, which we will call Emit as a whole.
So we need to manually match the AST 2 levels down to find the await_suspend call, get its name, and then walk through the emitted IR to find a call with the same name, and then find the tmp that's used to store the return value of the call, and then emit llvm.coro.forcestack.

[ChuanqiXu]

In D98638#2630864, @lxfind wrote:

That's a fair point. I agree that we have no guarantee these are the only two cases.
It does seem to me that coroutine implementation somewhat relies on proper lifetime markers so that data are being put correctly, which may be the fundamental problem we are trying to solve.

It is hard to prove it. This topic need more discuss and more folks get involved. But it is really valuable. I can't remember how many patches we had to judge whether values should be put on the coroutine frame. I am OK to emit lifetime markers even at O0. But I think you need to ask for other's opinion.

In D98638#2630864, @lxfind wrote:

We probably could, but it would be very very tedious.
During CodeGen, we only have the AST that's calling __builtin_coro_resume, which we will call Emit as a whole.
So we need to manually match the AST 2 levels down to find the await_suspend call, get its name, and then walk through the emitted IR to find a call with the same name, and then find the tmp that's used to store the return value of the call, and then emit llvm.coro.forcestack.

Can't we did as inline comments?

clang/lib/CodeGen/CGCoroutine.cpp

221

can we rewrite it into:

else if (SuspendRet != nullptr && SuspendRet->getType()->isClassType()) {
     // generate:
     // llvm.coro.forcestack(SuspendRet)
}

[ChuanqiXu]

clang/lib/CodeGen/CGCoroutine.cpp

221

Sorry I find we can't did it directly. As you said, we need to traverse down SuspendRet. And I still think we should did it only at CodeGen part since it looks not so hard. I guess we could make it in above 10~15 lines of codes.

[lxfind]

Can't we did as inline comments?

No, because it would have already been too late. SuspendExpr returns the result of __builtin_coro_resume(awaiter.await_suspend().address()), which is different from the result of awaiter.await_suspend().
We need to be able to control the placement of awaiter.await_suspend(), which is why I had to break up the AST at that boundary.

[lxfind]

clang/lib/CodeGen/CGCoroutine.cpp

221

Traversing down AST isn't the hard part. The hard part is to search the emitted IR, and look for the temporary alloca used to store the returned handle.

[ChuanqiXu]

clang/lib/CodeGen/CGCoroutine.cpp

221

Yes, I get your point. If we want to traverse the emitted IR, we could only search for the use-chain backward, which is also very odd. Let's see if there is other ways to modify the ASTNodes to make it more naturally.

[bruno]

Hi Xun, great to see more improvements in this area.

clang/lib/CodeGen/CGCoroutine.cpp

221

I'm curious whether did you consider annotating instructions with some new custom metadata instead of using intrinsics? If so, what would be the tradeoff? For example, if you could conditionally attach metadata some "begin" metadata here:

auto *SaveCall = Builder.CreateCall(CoroSave, {NullPtr});

and "end" metadata here:

auto *SuspendResult = Builder.CreateCall(CoroSuspend, {SaveCall, Builder.getInt1(IsFinalSuspend)});

clang/test/CodeGenCoroutines/coro-symmetric-transfer-01.cpp

53

Nice tests. The codegen should live in a different file from the AST dump one, you can put the later in test/clang/SemaCXX or tes/clang/AST.

llvm/include/llvm/IR/Intrinsics.td

1308

This change seems unrelated to this patch.

llvm/lib/Transforms/Coroutines/CoroFrame.cpp

2083

collectForceStacks is only called once from a function that already traverses all instructions, can you take advantage of that to collect llvm::Intrinsic::coro_forcestack_begin/end?

2085

Do such intrinsics never get removed? What happens when this hits a backend?

[junparser]

In D98638#2630786, @lxfind wrote:

Well, I guess another potential solution is to force emitting lifetime intrinsics for this part of coroutine in the front-end.
Like this:

diff --git a/clang/lib/CodeGen/CGDecl.cpp b/clang/lib/CodeGen/CGDecl.cpp
index 243d93a8c165..ef76e8dcb7c9 100644
--- a/clang/lib/CodeGen/CGDecl.cpp
+++ b/clang/lib/CodeGen/CGDecl.cpp
@@ -1317,7 +1317,7 @@ void CodeGenFunction::EmitAutoVarDecl(const VarDecl &D) {
 /// otherwise
 llvm::Value *CodeGenFunction::EmitLifetimeStart(uint64_t Size,
                                                 llvm::Value *Addr) {
-  if (!ShouldEmitLifetimeMarkers)
+  if (!ShouldEmitLifetimeMarkers && !isCoroutine())
     return nullptr;
 
   assert(Addr->getType()->getPointerAddressSpace() ==
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index 18f1468dcb86..2e6e6808db7f 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -535,7 +535,7 @@ EmitMaterializeTemporaryExpr(const MaterializeTemporaryExpr *M) {
       break;
 
     case SD_FullExpression: {
-      if (!ShouldEmitLifetimeMarkers)
+      if (!ShouldEmitLifetimeMarkers && !isCoroutine())
         break;
 
       // Avoid creating a conditional cleanup just to hold an llvm.lifetime.end

We have already allowed to emit lifetime intrinsics for always inlined function under O2, so IMOO emitting lifetime intrinsics for coroutine function is OK since stack coloring has less effect on coroutine function.

[lxfind]

@bruno Thanks for the review!

clang/lib/CodeGen/CGCoroutine.cpp

221

The "end" part could probably be done through metadata. But I'm not sure how to do it for the "begin" part. The "begin" part needs to happen after the emission of S.getAwaitSuspendCallExpr().

llvm/lib/Transforms/Coroutines/CoroFrame.cpp

2085

They are added to the list of DeadInstructions after collected. So they will all be removed at the end of the pass.

[lxfind]

Abandoning in favor of D99227

clang/include/clang/AST/ExprCXX.h

 ///  -- execution of the coroutine is suspended
 ///  -- the 'suspend' expression is evaluated
 ///     -- if the 'suspend' expression returns 'false', the coroutine is
 ///        resumed
 ///     -- otherwise, control passes back to the resumer.
 /// If the coroutine is not suspended, or when it is resumed, the 'resume'
 /// expression is evaluated, and its result is the result of the overall
 /// expression.
+/// When there is a symmetric transfer, i.e. await_suspend call returns a
+/// coroutine handler, AwaitSuspendCall will be the AST that represents
+/// the call to await_suspend; OVESuspend will be a OpaqueValueExpr wrapping
+/// around AwaitSuspendCall, and Suspend will be the transfer call built
+/// on top of OVESuspend. That is, we used a OpaqueValueExpr to divide Suspend
+/// at the end of await_suspend call, so that we can emit instructions right
+/// after the await_suspend call. Specifically, we emit coro.forcestack.begin
+/// intrinsic to indicate that from that point on, any data accessed must not
+/// be put on the coroutine frame, but on the stack. This is a critical hint to
+/// the CoroSplit pass that any alloca used here untill coro.forcestack.end
+/// shall remain on the stack. This is necessary because the await_suspend call
+/// could potentially destroy the current frame, and there is no stable way
+/// to guanarantee that the compiler can always put the temporaries used
+/// afterwards on the stack.
 class CoroutineSuspendExpr : public Expr {
   friend class ASTStmtReader;
 
   SourceLocation KeywordLoc;
 
-  enum SubExpr { Common, Ready, Suspend, Resume, Count };
+  enum SubExpr { Common, Ready, AwaitSuspendCall, Suspend, Resume, Count };
 
   Stmt *SubExprs[SubExpr::Count];
-  OpaqueValueExpr *OpaqueValue = nullptr;
+  OpaqueValueExpr *OVECommon = nullptr;
+  OpaqueValueExpr *OVESuspend = nullptr;
 
 public:
   CoroutineSuspendExpr(StmtClass SC, SourceLocation KeywordLoc, Expr *Common,
-                       Expr *Ready, Expr *Suspend, Expr *Resume,
-                       OpaqueValueExpr *OpaqueValue)
+                       Expr *Ready, Expr *AwaitSuspendCall, Expr *Suspend,
+                       Expr *Resume, OpaqueValueExpr *OVECommon,
+                       OpaqueValueExpr *OVESuspend)
       : Expr(SC, Resume->getType(), Resume->getValueKind(),
              Resume->getObjectKind()),
-        KeywordLoc(KeywordLoc), OpaqueValue(OpaqueValue) {
+        KeywordLoc(KeywordLoc), OVECommon(OVECommon), OVESuspend(OVESuspend) {
     SubExprs[SubExpr::Common] = Common;
     SubExprs[SubExpr::Ready] = Ready;
+    SubExprs[SubExpr::AwaitSuspendCall] = AwaitSuspendCall;
     SubExprs[SubExpr::Suspend] = Suspend;
     SubExprs[SubExpr::Resume] = Resume;
+    assert((!AwaitSuspendCall) == (!OVESuspend) &&
+           "AwaitSuspendCall and OVESuspend must be provided together");
     setDependence(computeDependence(this));
   }
 
   CoroutineSuspendExpr(StmtClass SC, SourceLocation KeywordLoc, QualType Ty,
                        Expr *Common)
       : Expr(SC, Ty, VK_RValue, OK_Ordinary), KeywordLoc(KeywordLoc) {
     assert(Common->isTypeDependent() && Ty->isDependentType() &&
            "wrong constructor for non-dependent co_await/co_yield expression");
     SubExprs[SubExpr::Common] = Common;
     SubExprs[SubExpr::Ready] = nullptr;
+    SubExprs[SubExpr::AwaitSuspendCall] = nullptr;
     SubExprs[SubExpr::Suspend] = nullptr;
     SubExprs[SubExpr::Resume] = nullptr;
     setDependence(computeDependence(this));
   }
 
   CoroutineSuspendExpr(StmtClass SC, EmptyShell Empty) : Expr(SC, Empty) {
     SubExprs[SubExpr::Common] = nullptr;
     SubExprs[SubExpr::Ready] = nullptr;
+    SubExprs[SubExpr::AwaitSuspendCall] = nullptr;
     SubExprs[SubExpr::Suspend] = nullptr;
     SubExprs[SubExpr::Resume] = nullptr;
   }
 
   SourceLocation getKeywordLoc() const { return KeywordLoc; }
 
   Expr *getCommonExpr() const {
     return static_cast<Expr*>(SubExprs[SubExpr::Common]);
   }
 
   /// getOpaqueValue - Return the opaque value placeholder.
-  OpaqueValueExpr *getOpaqueValue() const { return OpaqueValue; }
+  OpaqueValueExpr *getOpaqueValueCommon() const { return OVECommon; }
 
   Expr *getReadyExpr() const {
     return static_cast<Expr*>(SubExprs[SubExpr::Ready]);
   }
 
+  Expr *getAwaitSuspendCallExpr() const {
+    return static_cast<Expr *>(SubExprs[SubExpr::AwaitSuspendCall]);
+  }
+
   Expr *getSuspendExpr() const {
     return static_cast<Expr*>(SubExprs[SubExpr::Suspend]);
   }
 
+  OpaqueValueExpr *getOpaqueValueSuspend() const { return OVESuspend; }
+
   Expr *getResumeExpr() const {
     return static_cast<Expr*>(SubExprs[SubExpr::Resume]);
   }
 
   SourceLocation getBeginLoc() const LLVM_READONLY { return KeywordLoc; }
 
   SourceLocation getEndLoc() const LLVM_READONLY {
     return getCommonExpr()->getEndLoc();

 };
 
 /// Represents a 'co_await' expression.
 class CoawaitExpr : public CoroutineSuspendExpr {
   friend class ASTStmtReader;
 
 public:
   CoawaitExpr(SourceLocation CoawaitLoc, Expr *Operand, Expr *Ready,
-              Expr *Suspend, Expr *Resume, OpaqueValueExpr *OpaqueValue,
+              Expr *AwaitSuspendCall, Expr *Suspend, Expr *Resume,
+              OpaqueValueExpr *OVECommon, OpaqueValueExpr *OVESuspend,
               bool IsImplicit = false)
       : CoroutineSuspendExpr(CoawaitExprClass, CoawaitLoc, Operand, Ready,
-                             Suspend, Resume, OpaqueValue) {
+                             AwaitSuspendCall, Suspend, Resume, OVECommon,
+                             OVESuspend) {
     CoawaitBits.IsImplicit = IsImplicit;
   }
 
   CoawaitExpr(SourceLocation CoawaitLoc, QualType Ty, Expr *Operand,
               bool IsImplicit = false)
       : CoroutineSuspendExpr(CoawaitExprClass, CoawaitLoc, Ty, Operand) {
     CoawaitBits.IsImplicit = IsImplicit;
   }

 };
 
 /// Represents a 'co_yield' expression.
 class CoyieldExpr : public CoroutineSuspendExpr {
   friend class ASTStmtReader;
 
 public:
   CoyieldExpr(SourceLocation CoyieldLoc, Expr *Operand, Expr *Ready,
-              Expr *Suspend, Expr *Resume, OpaqueValueExpr *OpaqueValue)
+              Expr *AwaitSuspendCall, Expr *Suspend, Expr *Resume,
+              OpaqueValueExpr *OVECommon, OpaqueValueExpr *OVESuspend)
       : CoroutineSuspendExpr(CoyieldExprClass, CoyieldLoc, Operand, Ready,
-                             Suspend, Resume, OpaqueValue) {}
+                             AwaitSuspendCall, Suspend, Resume, OVECommon,
+                             OVESuspend) {}
   CoyieldExpr(SourceLocation CoyieldLoc, QualType Ty, Expr *Operand)
       : CoroutineSuspendExpr(CoyieldExprClass, CoyieldLoc, Ty, Operand) {}
   CoyieldExpr(EmptyShell Empty)
       : CoroutineSuspendExpr(CoyieldExprClass, Empty) {}
 
   Expr *getOperand() const {
     // FIXME: Dig out the actual operand or store it.
     return getCommonExpr();

clang/lib/CodeGen/CGCoroutine.cpp

namespace {
   };
 }
 static LValueOrRValue emitSuspendExpression(CodeGenFunction &CGF, CGCoroData &Coro,
                                     CoroutineSuspendExpr const &S,
                                     AwaitKind Kind, AggValueSlot aggSlot,
                                     bool ignoreResult, bool forLValue) {
   auto *E = S.getCommonExpr();
 
-  auto Binder =
-      CodeGenFunction::OpaqueValueMappingData::bind(CGF, S.getOpaqueValue(), E);
+  auto Binder = CodeGenFunction::OpaqueValueMappingData::bind(
+      CGF, S.getOpaqueValueCommon(), E);
   auto UnbindOnExit = llvm::make_scope_exit([&] { Binder.unbind(CGF); });
 
   auto Prefix = buildSuspendPrefixStr(Coro, Kind);
   BasicBlock *ReadyBlock = CGF.createBasicBlock(Prefix + Twine(".ready"));
   BasicBlock *SuspendBlock = CGF.createBasicBlock(Prefix + Twine(".suspend"));
   BasicBlock *CleanupBlock = CGF.createBasicBlock(Prefix + Twine(".cleanup"));
 
   // If expression is ready, no need to suspend.
   CGF.EmitBranchOnBoolExpr(S.getReadyExpr(), ReadyBlock, SuspendBlock, 0);
 
   // Otherwise, emit suspend logic.
   CGF.EmitBlock(SuspendBlock);
 
   auto &Builder = CGF.Builder;
   llvm::Function *CoroSave = CGF.CGM.getIntrinsic(llvm::Intrinsic::coro_save);
   auto *NullPtr = llvm::ConstantPointerNull::get(CGF.CGM.Int8PtrTy);
   auto *SaveCall = Builder.CreateCall(CoroSave, {NullPtr});
 
+  CodeGenFunction::OpaqueValueMappingData SuspendOVEBinder;
+  auto UnbindSuspendOVEOnExit = llvm::make_scope_exit([&] {
+    if (SuspendOVEBinder.isValid())
+      SuspendOVEBinder.unbind(CGF);
+  });
+
+  llvm::CallInst *ForcestackStart = nullptr;
+  if (auto *OVESuspend = S.getOpaqueValueSuspend()) {
+    SuspendOVEBinder = CodeGenFunction::OpaqueValueMappingData::bind(
+        CGF, OVESuspend, S.getAwaitSuspendCallExpr());
+    ForcestackStart = Builder.CreateCall(
+        CGF.CGM.getIntrinsic(llvm::Intrinsic::coro_forcestack_begin));
+  }
   auto *SuspendRet = CGF.EmitScalarExpr(S.getSuspendExpr());
   if (SuspendRet != nullptr && SuspendRet->getType()->isIntegerTy(1)) {
     // Veto suspension if requested by bool returning await_suspend.
     BasicBlock *RealSuspendBlock =
         CGF.createBasicBlock(Prefix + Twine(".suspend.bool"));
     CGF.Builder.CreateCondBr(SuspendRet, RealSuspendBlock, ReadyBlock);
     CGF.EmitBlock(RealSuspendBlock);
+  } else if (ForcestackStart) {
+    Builder.CreateCall(
+        CGF.CGM.getIntrinsic(llvm::Intrinsic::coro_forcestack_end),
+        {ForcestackStart});
   }
 
   // Emit the suspend point.
   const bool IsFinalSuspend = (Kind == AwaitKind::Final);
   llvm::Function *CoroSuspend =
       CGF.CGM.getIntrinsic(llvm::Intrinsic::coro_suspend);
   auto *SuspendResult = Builder.CreateCall(
       CoroSuspend, {SaveCall, Builder.getInt1(IsFinalSuspend)});

clang/lib/Sema/SemaCoroutine.cpp

ExprResult FromAddr =
       S.BuildDeclarationNameExpr(SS, Found, /*NeedsADL=*/false);
   if (FromAddr.isInvalid())
     return ExprError();
 
   return S.BuildCallExpr(nullptr, FromAddr.get(), Loc, FramePtr, Loc);
 }
 
 struct ReadySuspendResumeResult {
-  enum AwaitCallType { ACT_Ready, ACT_Suspend, ACT_Resume };
-  Expr *Results[3];
-  OpaqueValueExpr *OpaqueValue;
+  enum AwaitCallType {
+    ACT_Ready = 0,
+    ACT_AwaitSuspendCall,
+    ACT_Suspend,
+    ACT_Resume,
+    ACT_Count
+  };
+  Expr *Results[ACT_Count];
+  OpaqueValueExpr *OVECommon;
+  OpaqueValueExpr *OVESuspend;
   bool IsInvalid;
 };
+using ACT = ReadySuspendResumeResult::AwaitCallType;
 
 static ExprResult buildMemberCall(Sema &S, Expr *Base, SourceLocation Loc,
                                   StringRef Name, MultiExprArg Args) {
   DeclarationNameInfo NameInfo(&S.PP.getIdentifierTable().get(Name), Loc);
 
   // FIXME: Fix BuildMemberReferenceExpr to take a const CXXScopeSpec&.
   CXXScopeSpec SS;
   ExprResult Result = S.BuildMemberReferenceExpr(

 /// expression.
 static ReadySuspendResumeResult buildCoawaitCalls(Sema &S, VarDecl *CoroPromise,
                                                   SourceLocation Loc, Expr *E) {
   OpaqueValueExpr *Operand = new (S.Context)
       OpaqueValueExpr(Loc, E->getType(), VK_LValue, E->getObjectKind(), E);
 
   // Assume valid until we see otherwise.
   // Further operations are responsible for setting IsInalid to true.
-  ReadySuspendResumeResult Calls = {{}, Operand, /*IsInvalid=*/false};
-
-  using ACT = ReadySuspendResumeResult::AwaitCallType;
+  ReadySuspendResumeResult Calls = {{}, Operand, nullptr, /*IsInvalid=*/false};
 
   auto BuildSubExpr = [&](ACT CallType, StringRef Func,
                           MultiExprArg Arg) -> Expr * {
     ExprResult Result = buildMemberCall(S, Operand, Loc, Func, Arg);
     if (Result.isInvalid()) {
       Calls.IsInvalid = true;
       return nullptr;
     }
static ReadySuspendResumeResult buildCoawaitCalls(Sema &S, VarDecl *CoroPromise,
   if (!AwaitSuspend)
     return Calls;
   if (!AwaitSuspend->getType()->isDependentType()) {
     // [expr.await]p3 [...]
     //   - await-suspend is the expression e.await_suspend(h), which shall be
     //     a prvalue of type void, bool, or std::coroutine_handle<Z> for some
     //     type Z.
     QualType RetType = AwaitSuspend->getCallReturnType(S.Context);
+    OpaqueValueExpr *OVESuspend = new (S.Context) OpaqueValueExpr(
+        Loc, AwaitSuspend->getType(), AwaitSuspend->getValueKind(),
+        AwaitSuspend->getObjectKind(), AwaitSuspend);
 
     // Experimental support for coroutine_handle returning await_suspend.
-    if (Expr *TailCallSuspend =
-            maybeTailCall(S, RetType, AwaitSuspend, Loc))
+    if (Expr *TailCallSuspend = maybeTailCall(S, RetType, OVESuspend, Loc)) {
       // Note that we don't wrap the expression with ExprWithCleanups here
       // because that might interfere with tailcall contract (e.g. inserting
       // clean up instructions in-between tailcall and return). Instead
       // ExprWithCleanups is wrapped within maybeTailCall() prior to the resume
       // call.
+      Calls.OVESuspend = OVESuspend;
+      Calls.Results[ACT::ACT_AwaitSuspendCall] = AwaitSuspend;
       Calls.Results[ACT::ACT_Suspend] = TailCallSuspend;
-    else {
+    } else {
       // non-class prvalues always have cv-unqualified types
       if (RetType->isReferenceType() ||
           (!RetType->isBooleanType() && !RetType->isVoidType())) {
         S.Diag(AwaitSuspend->getCalleeDecl()->getLocation(),
                diag::err_await_suspend_invalid_return_type)
             << RetType;
         S.Diag(Loc, diag::note_coroutine_promise_call_implicitly_required)
             << AwaitSuspend->getDirectCallee();
         Calls.IsInvalid = true;
-      } else
+      } else {
+        Calls.OVESuspend = nullptr;
+        Calls.Results[ACT::ACT_AwaitSuspendCall] = nullptr;
         Calls.Results[ACT::ACT_Suspend] =
             S.MaybeCreateExprWithCleanups(AwaitSuspend);
       }
     }
+  }
 
   BuildSubExpr(ACT::ACT_Resume, "await_resume", None);
 
   // Make sure the awaiter object gets a chance to be cleaned up.
   S.Cleanup.setExprNeedsCleanups(true);
 
   return Calls;
 }
ExprResult Sema::BuildResolvedCoawaitExpr(SourceLocation Loc, Expr *E,
   SourceLocation CallLoc = E->getExprLoc();
 
   // Build the await_ready, await_suspend, await_resume calls.
   ReadySuspendResumeResult RSS = buildCoawaitCalls(
       *this, Coroutine->CoroutinePromise, CallLoc, E);
   if (RSS.IsInvalid)
     return ExprError();
 
-  Expr *Res =
-      new (Context) CoawaitExpr(Loc, E, RSS.Results[0], RSS.Results[1],
-                                RSS.Results[2], RSS.OpaqueValue, IsImplicit);
+  Expr *Res = new (Context) CoawaitExpr(
+      Loc, E, RSS.Results[ACT::ACT_Ready],
+      RSS.Results[ACT::ACT_AwaitSuspendCall], RSS.Results[ACT::ACT_Suspend],
+      RSS.Results[ACT::ACT_Resume], RSS.OVECommon, RSS.OVESuspend, IsImplicit);
 
   return Res;
 }
 
 ExprResult Sema::ActOnCoyieldExpr(Scope *S, SourceLocation Loc, Expr *E) {
   if (!ActOnCoroutineBodyStart(S, Loc, "co_yield")) {
     CorrectDelayedTyposInExpr(E);
     return ExprError();
ExprResult Sema::BuildCoyieldExpr(SourceLocation Loc, Expr *E) {
   if (E->getValueKind() == VK_RValue)
     E = CreateMaterializeTemporaryExpr(E->getType(), E, true);
 
   // Build the await_ready, await_suspend, await_resume calls.
   ReadySuspendResumeResult RSS = buildCoawaitCalls(
       *this, Coroutine->CoroutinePromise, Loc, E);
   if (RSS.IsInvalid)
     return ExprError();
-
-  Expr *Res =
-      new (Context) CoyieldExpr(Loc, E, RSS.Results[0], RSS.Results[1],
-                                RSS.Results[2], RSS.OpaqueValue);
+  Expr *Res = new (Context) CoyieldExpr(
+      Loc, E, RSS.Results[ACT::ACT_Ready],
+      RSS.Results[ACT::ACT_AwaitSuspendCall], RSS.Results[ACT::ACT_Suspend],
+      RSS.Results[ACT::ACT_Resume], RSS.OVECommon, RSS.OVESuspend);
 
   return Res;
 }
 
 StmtResult Sema::ActOnCoreturnStmt(Scope *S, SourceLocation Loc, Expr *E) {
   if (!ActOnCoroutineBodyStart(S, Loc, "co_return")) {
     CorrectDelayedTyposInExpr(E);
     return StmtError();

clang/lib/Serialization/ASTReaderStmt.cpp

void ASTStmtReader::VisitCoreturnStmt(CoreturnStmt *S) {
   S->IsImplicit = Record.readInt() != 0;
 }
 
 void ASTStmtReader::VisitCoawaitExpr(CoawaitExpr *E) {
   VisitExpr(E);
   E->KeywordLoc = readSourceLocation();
   for (auto &SubExpr: E->SubExprs)
     SubExpr = Record.readSubStmt();
-  E->OpaqueValue = cast_or_null<OpaqueValueExpr>(Record.readSubStmt());
+  E->OVECommon = cast_or_null<OpaqueValueExpr>(Record.readSubStmt());
+  E->OVESuspend = cast_or_null<OpaqueValueExpr>(Record.readSubStmt());
   E->setIsImplicit(Record.readInt() != 0);
 }
 
 void ASTStmtReader::VisitCoyieldExpr(CoyieldExpr *E) {
   VisitExpr(E);
   E->KeywordLoc = readSourceLocation();
   for (auto &SubExpr: E->SubExprs)
     SubExpr = Record.readSubStmt();
-  E->OpaqueValue = cast_or_null<OpaqueValueExpr>(Record.readSubStmt());
+  E->OVECommon = cast_or_null<OpaqueValueExpr>(Record.readSubStmt());
+  E->OVESuspend = cast_or_null<OpaqueValueExpr>(Record.readSubStmt());
 }
 
 void ASTStmtReader::VisitDependentCoawaitExpr(DependentCoawaitExpr *E) {
   VisitExpr(E);
   E->KeywordLoc = readSourceLocation();
   for (auto &SubExpr: E->SubExprs)
     SubExpr = Record.readSubStmt();
 }

clang/lib/Serialization/ASTWriterStmt.cpp

void ASTStmtWriter::VisitCoreturnStmt(CoreturnStmt *S) {
   Code = serialization::STMT_CORETURN;
 }
 
 void ASTStmtWriter::VisitCoroutineSuspendExpr(CoroutineSuspendExpr *E) {
   VisitExpr(E);
   Record.AddSourceLocation(E->getKeywordLoc());
   for (Stmt *S : E->children())
     Record.AddStmt(S);
-  Record.AddStmt(E->getOpaqueValue());
+  Record.AddStmt(E->getOpaqueValueCommon());
+  Record.AddStmt(E->getOpaqueValueSuspend());
 }
 
 void ASTStmtWriter::VisitCoawaitExpr(CoawaitExpr *E) {
   VisitCoroutineSuspendExpr(E);
   Record.push_back(E->isImplicit());
   Code = serialization::EXPR_COAWAIT;
 }
 

clang/test/CodeGenCoroutines/coro-symmetric-transfer-01.cpp

-// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fcoroutines-ts -std=c++14 -O1 -emit-llvm %s -o - -disable-llvm-passes | FileCheck %s
+// RUN: %clang_cc1 -fcoroutines-ts -std=c++14 -O0 -emit-llvm %s -ast-dump | FileCheck %s
+// RUN: %clang_cc1 -fcoroutines-ts -std=c++14 -O0 -emit-llvm %s -o - -disable-llvm-passes | FileCheck --check-prefix=CHECK-PRESPLIT %s
+// RUN: %clang_cc1 -fcoroutines-ts -std=c++14 -O0 -emit-llvm %s -o - | FileCheck --check-prefix=CHECK-POSTSPLIT %s
 
 #include "Inputs/coroutine.h"
 
 namespace coro = std::experimental::coroutines_v1;
 
 struct detached_task {
   struct promise_type {
     detached_task get_return_object() noexcept {
struct detached_task {
 
   coro::coroutine_handle<promise_type> coro_;
 };
 
 detached_task foo() {
   co_return;
 }
 
-// check that the lifetime of the coroutine handle used to obtain the address is contained within single basic block, and hence does not live across suspension points.
-// CHECK-LABEL: final.suspend:
-// CHECK:         %[[PTR1:.+]] = bitcast %"struct.std::experimental::coroutines_v1::coroutine_handle.0"* %[[ADDR_TMP:.+]] to i8*
-// CHECK-NEXT:    call void @llvm.lifetime.start.p0i8(i64 8, i8* %[[PTR1]])
-// CHECK:         call i8* @{{.*address.*}}(%"struct.std::experimental::coroutines_v1::coroutine_handle.0"* {{[^,]*}} %[[ADDR_TMP]])
-// CHECK-NEXT:    %[[PTR2:.+]] = bitcast %"struct.std::experimental::coroutines_v1::coroutine_handle.0"* %[[ADDR_TMP]] to i8*
-// CHECK-NEXT:    call void @llvm.lifetime.end.p0i8(i64 8, i8* %[[PTR2]])
+// CHECK-LABEL: |-FunctionDecl {{.*}} foo 'detached_task ()'
+// first ExprWithCleanups is the initial await
+// CHECK:       |   |-ExprWithCleanups {{.*}} 'void'
+// second ExprWithCleanups is the final await
+// CHECK:       |   |-ExprWithCleanups {{.*}} 'void'
+// AST for the await_suspend call
+// CHECK:       |   |   |-CXXMemberCallExpr {{.*}} 'coro::coroutine_handle<>':'std::experimental::coroutine_handle<>'
+// CHECK:       |   |   | |-MemberExpr {{.*}} '<bound member function type>' .await_suspend {{.*}}
+// CHECK:       |   |   | | `-OpaqueValueExpr {{.*}} 'detached_task::promise_type::final_awaiter' lvalue
+// AST for the symmetric transferred suspend
+// CHECK:       |   |   |-CallExpr {{.*}} 'void'
+// CHECK:       |   |   | |-ImplicitCastExpr {{.*}} 'void (*)(void *)' <FunctionToPointerDecay>
+// CHECK:       |   |   | | `-DeclRefExpr {{.*}} 'void (void *)' lvalue Function {{.*}} '__builtin_coro_resume' 'void (void *)'
+// CHECK:       |   |   | `-ExprWithCleanups {{.*}} 'void *'
+// CHECK:       |   |   |   `-CXXMemberCallExpr {{.*}} 'void *'
+// CHECK:       |   |   |     `-MemberExpr {{.*}} '<bound member function type>' .address {{.*}}
+// CHECK:       |   |   |       `-ImplicitCastExpr {{.*}} 'const std::experimental::coroutine_handle<>' xvalue <NoOp>
+// CHECK:       |   |   |         `-MaterializeTemporaryExpr {{.*}} 'coro::coroutine_handle<>':'std::experimental::coroutine_handle<>' xvalue
+// Below we are wrapping the await_suspend call with a OpaqueValueExpr
+// CHECK:       |   |   |           `-OpaqueValueExpr {{.*}} 'coro::coroutine_handle<>':'std::experimental::coroutine_handle<>'
+// CHECK:       |   |   |             `-CXXMemberCallExpr {{.*}} 'coro::coroutine_handle<>':'std::experimental::coroutine_handle<>'
+// CHECK:       |   |   |               |-MemberExpr {{.*}} '<bound member function type>' .await_suspend {{.*}}
+
+// CHECK-PRESPLIT-LABEL: define dso_local void @_Z3foov(
+// CHECK-PRESPLIT:       entry:
+// CHECK-PRESPLIT:         %coerce = alloca %"struct.std::experimental::coroutines_v1::coroutine_handle.0", align 8
+// CHECK-PRESPLIT:       final.suspend:
+// CHECK-PRESPLIT:         %[[HANDLE:.*]] = call i8* @{{.*await_suspend.*}}
+// CHECK-PRESPLIT-NEXT:    %[[COERCE1:.*]] = getelementptr inbounds %"struct.std::experimental::coroutines_v1::coroutine_handle.0", %"struct.std::experimental::coroutines_v1::coroutine_handle.0"* %coerce, i32 0, i32 0
+// CHECK-PRESPLIT-NEXT:    store i8* %[[HANDLE]], i8** %[[COERCE1]], align 8
+// CHECK-PRESPLIT-NEXT:    %[[FORCESTACK:.*]] = call i8* @llvm.coro.forcestack.begin()
+// CHECK-PRESPLIT-NEXT:    %[[ADDR:.*]] = call i8* @{{.*address.*}}(%"struct.std::experimental::coroutines_v1::coroutine_handle.0"* nonnull dereferenceable(8) %coerce)
+// CHECK-PRESPLIT-NEXT:    call void @llvm.coro.resume(i8* %[[ADDR]])
+// CHECK-PRESPLIT-NEXT:    call void @llvm.coro.forcestack.end(i8* %[[FORCESTACK]])
+// CHECK-PRESPLIT-NEXT:    %16 = call i8 @llvm.coro.suspend(token %{{.*}}, i1 true)
+
+// CHECK-POSTSPLIT:       %_Z3foov.Frame = type { void (%_Z3foov.Frame*)*, void (%_Z3foov.Frame*)*, %"struct.detached_task::promise_type", i1, %"struct.std::experimental::coroutines_v1::suspend_always", %"struct.detached_task::promise_type::final_awaiter" }
+// CHECK-POSTSPLIT-LABEL: define internal fastcc void @_Z3foov.resume(
+// CHECK-POSTSPLIT:       entry:
+// CHECK-POSTSPLIT:         %coerce = alloca %"struct.std::experimental::coroutines_v1::coroutine_handle.0", align 8
+
+// CHECK-POSTSPLIT:         %[[HANDLE:.*]] = call i8* @{{.*await_suspend.*}}
+// CHECK-POSTSPLIT:         %[[COERCE1:.*]] = getelementptr inbounds %"struct.std::experimental::coroutines_v1::coroutine_handle.0", %"struct.std::experimental::coroutines_v1::coroutine_handle.0"* %coerce, i32 0, i32 0
+// CHECK-POSTSPLIT:         store i8* %[[HANDLE]], i8** %[[COERCE1]], align 8
+// CHECK-POSTSPLIT:         %[[ADDR:.*]] = call i8* @{{.*address.*}}(%"struct.std::experimental::coroutines_v1::coroutine_handle.0"* nonnull dereferenceable(8) %coerce)

clang/test/CodeGenCoroutines/coro-symmetric-transfer-02.cpp

-// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fcoroutines-ts -std=c++14 -O1 -emit-llvm %s -o - -disable-llvm-passes | FileCheck %s
+// RUN: %clang_cc1 -fcoroutines-ts -std=c++14 -O0 -emit-llvm %s -ast-dump | FileCheck %s
 
 #include "Inputs/coroutine.h"
 
 namespace coro = std::experimental::coroutines_v1;
 
 struct Task {
   struct promise_type {
     Task get_return_object() noexcept {
Task bar() {
   case 2:
     co_await foo();
     break;
   default:
     break;
   }
 }
 
-// CHECK-LABEL: define{{.*}} void @_Z3barv
-// CHECK:         %[[MODE:.+]] = load i32, i32* %mode
-// CHECK-NEXT:    switch i32 %[[MODE]], label %{{.+}} [
-// CHECK-NEXT:      i32 1, label %[[CASE1:.+]]
-// CHECK-NEXT:      i32 2, label %[[CASE2:.+]]
-// CHECK-NEXT:    ]
-
-// CHECK:       [[CASE1]]:
-// CHECK:         br i1 %{{.+}}, label %[[CASE1_AWAIT_READY:.+]], label %[[CASE1_AWAIT_SUSPEND:.+]]
-// CHECK:       [[CASE1_AWAIT_SUSPEND]]:
-// CHECK-NEXT:    %{{.+}} = call token @llvm.coro.save(i8* null)
-// CHECK-NEXT:    %[[HANDLE11:.+]] = bitcast %"struct.std::experimental::coroutines_v1::coroutine_handle"* %[[TMP1:.+]] to i8*
-// CHECK-NEXT:    call void @llvm.lifetime.start.p0i8(i64 8, i8* %[[HANDLE11]])
-
-// CHECK:         %[[HANDLE12:.+]] = bitcast %"struct.std::experimental::coroutines_v1::coroutine_handle"* %[[TMP1]] to i8*
-// CHECK-NEXT:    call void @llvm.lifetime.end.p0i8(i64 8, i8* %[[HANDLE12]])
-// CHECK-NEXT:    call void @llvm.coro.resume
-// CHECK-NEXT:    %{{.+}} = call i8 @llvm.coro.suspend
-// CHECK-NEXT:    switch i8 %{{.+}}, label %coro.ret [
-// CHECK-NEXT:      i8 0, label %[[CASE1_AWAIT_READY]]
-// CHECK-NEXT:      i8 1, label %[[CASE1_AWAIT_CLEANUP:.+]]
-// CHECK-NEXT:    ]
-// CHECK:       [[CASE1_AWAIT_CLEANUP]]:
-// make sure that the awaiter eventually gets cleaned up.
-// CHECK:         call void @{{.+Awaiter.+}}
-
-// CHECK:       [[CASE2]]:
-// CHECK:         br i1 %{{.+}}, label %[[CASE2_AWAIT_READY:.+]], label %[[CASE2_AWAIT_SUSPEND:.+]]
-// CHECK:       [[CASE2_AWAIT_SUSPEND]]:
-// CHECK-NEXT:    %{{.+}} = call token @llvm.coro.save(i8* null)
-// CHECK-NEXT:    %[[HANDLE21:.+]] = bitcast %"struct.std::experimental::coroutines_v1::coroutine_handle"* %[[TMP2:.+]] to i8*
-// CHECK-NEXT:    call void @llvm.lifetime.start.p0i8(i64 8, i8* %[[HANDLE21]])
-
-// CHECK:         %[[HANDLE22:.+]] = bitcast %"struct.std::experimental::coroutines_v1::coroutine_handle"* %[[TMP2]] to i8*
-// CHECK-NEXT:    call void @llvm.lifetime.end.p0i8(i64 8, i8* %[[HANDLE22]])
-// CHECK-NEXT:    call void @llvm.coro.resume
-// CHECK-NEXT:    %{{.+}} = call i8 @llvm.coro.suspend
-// CHECK-NEXT:    switch i8 %{{.+}}, label %coro.ret [
-// CHECK-NEXT:      i8 0, label %[[CASE2_AWAIT_READY]]
-// CHECK-NEXT:      i8 1, label %[[CASE2_AWAIT_CLEANUP:.+]]
-// CHECK-NEXT:    ]
-// CHECK:       [[CASE2_AWAIT_CLEANUP]]:
-// make sure that the awaiter eventually gets cleaned up.
-// CHECK:         call void @{{.+Awaiter.+}}
+// CHECK-LABEL: `-FunctionDecl {{.*}} bar 'Task ()'
+// CHECK:       | `-SwitchStmt {{.*}}
+// first case
+// CHECK:       |     |-CaseStmt {{.*}}
+// await_suspend call
+// CHECK:       |     |     |-CXXMemberCallExpr {{.*}} 'Task::handle_t':'std::experimental::coroutine_handle<Task::promise_type>'
+// CHECK:       |     |     | |-MemberExpr {{.*}} '<bound member function type>' .await_suspend {{.*}}
+// symmetric transfered suspend, which wraps around await_suspend call with a OpaqueValueExpr
+// CHECK:       |     |     |-CallExpr {{.*}} 'void'
+// CHECK:       |     |     | |-ImplicitCastExpr {{.*}} 'void (*)(void *)' <FunctionToPointerDecay>
+// CHECK:       |     |     | | `-DeclRefExpr {{.*}} 'void (void *)' lvalue Function {{.*}} '__builtin_coro_resume' 'void (void *)'
+// CHECK:       |     |     | `-ExprWithCleanups {{.*}} 'void *'
+// CHECK:       |     |     |   `-CXXMemberCallExpr {{.*}} 'void *'
+// CHECK:       |     |     |     `-MemberExpr {{.*}} '<bound member function type>' .address {{.*}}
+// CHECK:       |     |     |       `-ImplicitCastExpr {{.*}} 'const std::experimental::coroutine_handle<>' xvalue <UncheckedDerivedToBase (coroutine_handle)>
+// CHECK:       |     |     |         `-MaterializeTemporaryExpr {{.*}} 'Task::handle_t':'std::experimental::coroutine_handle<Task::promise_type>' xvalue
+// CHECK:       |     |     |           `-OpaqueValueExpr {{.*}} 'Task::handle_t':'std::experimental::coroutine_handle<Task::promise_type>'
+// CHECK:       |     |     |             `-CXXMemberCallExpr {{.*}} 'Task::handle_t':'std::experimental::coroutine_handle<Task::promise_type>'
+// CHECK:       |     |     |               |-MemberExpr {{.*}} '<bound member function type>' .await_suspend {{.*}}
+// second case
+// CHECK:       |     |-CaseStmt {{.*}}
+// CHECK:       |     |     |-CXXMemberCallExpr {{.*}} 'Task::handle_t':'std::experimental::coroutine_handle<Task::promise_type>'
+// CHECK:       |     |     | |-MemberExpr {{.*}} '<bound member function type>' .await_suspend {{.*}}
+// CHECK:       |     |     |-CallExpr {{.*}} 'void'
+// CHECK:       |     |     | |-ImplicitCastExpr {{.*}} 'void (*)(void *)' <FunctionToPointerDecay>
+// CHECK:       |     |     | | `-DeclRefExpr {{.*}} 'void (void *)' lvalue Function {{.*}} '__builtin_coro_resume' 'void (void *)'
+// CHECK:       |     |     | `-ExprWithCleanups {{.*}} 'void *'
+// CHECK:       |     |     |   `-CXXMemberCallExpr {{.*}} 'void *'
+// CHECK:       |     |     |     `-MemberExpr {{.*}} '<bound member function type>' .address {{.*}}
+// CHECK:       |     |     |       `-ImplicitCastExpr {{.*}} 'const std::experimental::coroutine_handle<>' xvalue <UncheckedDerivedToBase (coroutine_handle)>
+// CHECK:       |     |     |         `-MaterializeTemporaryExpr {{.*}} 'Task::handle_t':'std::experimental::coroutine_handle<Task::promise_type>' xvalue
+// CHECK:       |     |     |           `-OpaqueValueExpr {{.*}} 'Task::handle_t':'std::experimental::coroutine_handle<Task::promise_type>'
+// CHECK:       |     |     |             `-CXXMemberCallExpr {{.*}} 'Task::handle_t':'std::experimental::coroutine_handle<Task::promise_type>'
+// CHECK:       |     |     |               |-MemberExpr {{.*}} '<bound member function type>' .await_suspend {{.*}}

llvm/docs/Coroutines.rst

.. code-block:: text
   }
 
 The optimizer can replace coro.param(a',a) with `i1 false` and replace all uses
 of `a` with `a'`, since it is not used after suspend.
 
 The optimizer must replace coro.param(b', b) with `i1 true`, since `b` is used
 after suspend and therefore, it has to reside in the coroutine frame.
 
+.. _coro.forcestack.begin:
+
+'llvm.coro.forcestack.begin' Intrinsic
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+::
+
+  declare i8* @llvm.coro.forcestack.begin()
+
+Overview:
+"""""""""
+
+The '``llvm.coro.forcestack.begin``' intrinsic,  paird with '``llvm.coro.forcestack.end``',
+are emitted by the front end to mark a region where only data from the local stack can be
+accessed, i.e. no coroutine frame access is allowed. It's introduced to help the optimizer
+make correct decisions on where to put certain data.
+
+Arguments:
+""""""""""
+
+None.
+
+Semantics:
+""""""""""
+
+Marks the beginning of a region where any use of alloca must remain on the stack during
+CoroSplit and cannot be put on the coroutine frame. This is needed to aid the implementation
+of symmetric transfer. After the call to '``await_suspend``' returns a handle, the current
+coroutine frame may have already been destroyed, hence we can no longer access the frame.
+However in order to perform symmetric transfer on the handle, the compiler needs to use
+a few temporaries and also invoke the '``address``' function on coroutine class. The compiler
+is not capable of determining that these operations never lead to escape, and hence will
+end up putting them on the frame.
+
+.. _coro.forcestack.end:
+
+'llvm.coro.forcestack.end' Intrinsic
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+::
+
+  declare i8* @llvm.coro.forcestack.end()
+
+Overview:
+"""""""""
+
+Marker to end the region started by '``llvm.coro.forcestack.begin``'.
+
+Arguments:
+""""""""""
+
+None.
+
+Semantics:
+""""""""""
+
+Refer to the semantics of '``llvm.coro.forcestack.begin``'.
+
 Coroutine Transformation Passes
 ===============================
 CoroEarly
 ---------
 The pass CoroEarly lowers coroutine intrinsics that hide the details of the
 structure of the coroutine frame, but, otherwise not needed to be preserved to
 help later coroutine passes. This pass lowers `coro.frame`_, `coro.done`_, 
 and `coro.promise`_ intrinsics.

llvm/include/llvm/IR/Intrinsics.td

def int_coro_alloca_alloc : Intrinsic<[llvm_token_ty],
                                       [llvm_anyint_ty, llvm_i32_ty], []>;
 def int_coro_alloca_get : Intrinsic<[llvm_ptr_ty], [llvm_token_ty], []>;
 def int_coro_alloca_free : Intrinsic<[], [llvm_token_ty], []>;
 
 def int_coro_param : Intrinsic<[llvm_i1_ty], [llvm_ptr_ty, llvm_ptr_ty],
                                [IntrNoMem, ReadNone<ArgIndex<0>>,
                                 ReadNone<ArgIndex<1>>]>;
 
+def int_coro_forcestack_begin : Intrinsic<[llvm_ptr_ty], [], [IntrNoMem]>;
+def int_coro_forcestack_end : Intrinsic<[], [llvm_ptr_ty], [IntrNoMem]>;
+
 // Coroutine Manipulation Intrinsics.
 
 def int_coro_resume : Intrinsic<[], [llvm_ptr_ty], [Throws]>;
 def int_coro_destroy : Intrinsic<[], [llvm_ptr_ty], [Throws]>;
 def int_coro_done : Intrinsic<[llvm_i1_ty], [llvm_ptr_ty],
                               [IntrArgMemOnly, ReadOnly<ArgIndex<0>>,
                                NoCapture<ArgIndex<0>>]>;
 def int_coro_promise : Intrinsic<[llvm_ptr_ty],

 
 // This instruction has no actual effect, though it is treated by the optimizer
 // has having opaque side effects. This may be inserted into loops to ensure
 // that they are not removed even if they turn out to be empty, for languages
 // which specify that infinite loops must be preserved.
 def int_sideeffect : DefaultAttrsIntrinsic<[], [], [IntrInaccessibleMemOnly, IntrWillReturn]>;
 
 // The pseudoprobe intrinsic works as a place holder to the block it probes.
-// Like the sideeffect intrinsic defined above, this intrinsic is treated by the 
-// optimizer as having opaque side effects so that it won't be get rid of or moved 
+// Like the sideeffect intrinsic defined above, this intrinsic is treated by the
+// optimizer as having opaque side effects so that it won't be get rid of or moved
 // out of the block it probes.
 def int_pseudoprobe : Intrinsic<[], [llvm_i64_ty, llvm_i64_ty, llvm_i32_ty, llvm_i64_ty],
                                     [IntrInaccessibleMemOnly, IntrWillReturn]>;
 
 // Intrinsics to support half precision floating point format
 let IntrProperties = [IntrNoMem, IntrWillReturn] in {
 def int_convert_to_fp16   : DefaultAttrsIntrinsic<[llvm_i16_ty], [llvm_anyfloat_ty]>;
 def int_convert_from_fp16 : DefaultAttrsIntrinsic<[llvm_anyfloat_ty], [llvm_i16_ty]>;

llvm/lib/Transforms/Coroutines/CoroFrame.cpp

case coro::ABI::Async: {
     }
     break;
   }
   }
 
   return FrameTy;
 }
 
+using ForceStackList =
+    SmallVector<std::pair<IntrinsicInst *, IntrinsicInst *>, 4>;
+
 // We use a pointer use visitor to track how an alloca is being used.
 // The goal is to be able to answer the following three questions:
 // 1. Should this alloca be allocated on the frame instead.
 // 2. Could the content of the alloca be modified prior to CoroBegn, which would
 // require copying the data from alloca to the frame after CoroBegin.
 // 3. Is there any alias created for this alloca prior to CoroBegin, but used
 // after CoroBegin. In that case, we will need to recreate the alias after
 // CoroBegin based off the frame. To answer question 1, we track two things:

 // offset is unknown (e.g. when you have a PHINode that takes in different
 // offset values). We cannot handle unknown offsets and will assert. This is the
 // potential issue left out. An ideal solution would likely require a
 // significant redesign.
 namespace {
 struct AllocaUseVisitor : PtrUseVisitor<AllocaUseVisitor> {
   using Base = PtrUseVisitor<AllocaUseVisitor>;
   AllocaUseVisitor(const DataLayout &DL, const DominatorTree &DT,
-                   const CoroBeginInst &CB, const SuspendCrossingInfo &Checker)
-      : PtrUseVisitor(DL), DT(DT), CoroBegin(CB), Checker(Checker) {}
+                   const CoroBeginInst &CB, const SuspendCrossingInfo &Checker,
+                   const ForceStackList &ForceStacks)
+      : PtrUseVisitor(DL), DT(DT), CoroBegin(CB), Checker(Checker),
+        ForceStacks(ForceStacks) {}
 
   void visit(Instruction &I) {
+    for (const auto &P : ForceStacks)
+      if (DT.dominates(P.first, &I) && DT.dominates(&I, P.second)) {
+        ShouldLiveOnFrame = false;
+        PI.setAborted(&I);
+        return;
+      }
     Users.insert(&I);
     Base::visit(I);
     // If the pointer is escaped prior to CoroBegin, we have to assume it would
     // be written into before CoroBegin as well.
     if (PI.isEscaped() && !DT.dominates(&CoroBegin, PI.getEscapingInst())) {
       MayWriteBeforeCoroBegin = true;
     }
   }
void visitSelectInst(SelectInst &I) {
     handleAlias(I);
   }
 
   void visitStoreInst(StoreInst &SI) {
     // Regardless whether the alias of the alloca is the value operand or the
     // pointer operand, we need to assume the alloca is been written.
     handleMayWrite(SI);
 
-    if (SI.getValueOperand() != U->get())
-      return;
-
-    // We are storing the pointer into a memory location, potentially escaping.
-    // As an optimization, we try to detect simple cases where it doesn't
-    // actually escape, for example:
-    //   %ptr = alloca ..
-    //   %addr = alloca ..
-    //   store %ptr, %addr
-    //   %x = load %addr
-    //   ..
-    // If %addr is only used by loading from it, we could simply treat %x as
-    // another alias of %ptr, and not considering %ptr being escaped.
-    auto IsSimpleStoreThenLoad = [&]() {
-      auto *AI = dyn_cast<AllocaInst>(SI.getPointerOperand());
-      // If the memory location we are storing to is not an alloca, it
-      // could be an alias of some other memory locations, which is difficult
-      // to analyze.
-      if (!AI)
-        return false;
-      // StoreAliases contains aliases of the memory location stored into.
-      SmallVector<Instruction *, 4> StoreAliases = {AI};
-      while (!StoreAliases.empty()) {
-        Instruction *I = StoreAliases.pop_back_val();
-        for (User *U : I->users()) {
-          // If we are loading from the memory location, we are creating an
-          // alias of the original pointer.
-          if (auto *LI = dyn_cast<LoadInst>(U)) {
-            enqueueUsers(*LI);
-            handleAlias(*LI);
-            continue;
-          }
-          // If we are overriding the memory location, the pointer certainly
-          // won't escape.
-          if (auto *S = dyn_cast<StoreInst>(U))
-            if (S->getPointerOperand() == I)
-              continue;
-          if (auto *II = dyn_cast<IntrinsicInst>(U))
-            if (II->isLifetimeStartOrEnd())
-              continue;
-          // BitCastInst creats aliases of the memory location being stored
-          // into.
-          if (auto *BI = dyn_cast<BitCastInst>(U)) {
-            StoreAliases.push_back(BI);
-            continue;
-          }
-          return false;
-        }
-      }
-
-      return true;
-    };
-
-    if (!IsSimpleStoreThenLoad())
+    if (SI.getValueOperand() == U->get())
       PI.setEscaped(&SI);
   }
 
   // All mem intrinsics modify the data.
   void visitMemIntrinsic(MemIntrinsic &MI) { handleMayWrite(MI); }
 
   void visitBitCastInst(BitCastInst &BC) {
     Base::visitBitCastInst(BC);
for (const auto &P : AliasOffetMap)
                            "created before CoroBegin.");
     return AliasOffetMap;
   }
 
 private:
   const DominatorTree &DT;
   const CoroBeginInst &CoroBegin;
   const SuspendCrossingInfo &Checker;
+  const ForceStackList &ForceStacks;
   // All alias to the original AllocaInst, created before CoroBegin and used
   // after CoroBegin. Each entry contains the instruction and the offset in the
   // original Alloca. They need to be recreated after CoroBegin off the frame.
   DenseMap<Instruction *, llvm::Optional<APInt>> AliasOffetMap{};
   SmallPtrSet<Instruction *, 4> Users{};
   SmallPtrSet<IntrinsicInst *, 2> LifetimeStarts{};
   bool MayWriteBeforeCoroBegin{false};
 
for (BasicBlock *DomBB : DomSet) {
           S->eraseFromParent();
 
         break;
       }
     }
   }
 }
 
+static ForceStackList collectForceStacks(Function &F) {
+  ForceStackList ForceStacks;
+  for (auto &I : instructions(F))
+    if (auto *II = dyn_cast<IntrinsicInst>(&I))
+      if (II->getIntrinsicID() == llvm::Intrinsic::coro_forcestack_begin) {
+        assert(II->getNumUses() == 1 &&
+               "Each coro_forcestack_begin intrinsic must be used by one "
+               "coro_forcestack_end intrinsic");
+        auto *End = cast<IntrinsicInst>(II->user_back());
+        assert(End->getIntrinsicID() == llvm::Intrinsic::coro_forcestack_end &&
+               "Each coro_forcestack_begin intrinsic must be used by one "
+               "coro_forcestack_end intrinsic");
+        ForceStacks.emplace_back(II, End);
+      }
+  return ForceStacks;
+}
+
 static void collectFrameAllocas(Function &F, coro::Shape &Shape,
                                 const SuspendCrossingInfo &Checker,
-                                SmallVectorImpl<AllocaInfo> &Allocas) {
+                                SmallVectorImpl<AllocaInfo> &Allocas,
+                                const ForceStackList &ForceStacks) {
   for (Instruction &I : instructions(F)) {
     auto *AI = dyn_cast<AllocaInst>(&I);
     if (!AI)
       continue;
     // The PromiseAlloca will be specially handled since it needs to be in a
     // fixed position in the frame.
     if (AI == Shape.SwitchLowering.PromiseAlloca) {
       continue;
     }
     DominatorTree DT(F);
     AllocaUseVisitor Visitor{F.getParent()->getDataLayout(), DT,
-                             *Shape.CoroBegin, Checker};
+                             *Shape.CoroBegin, Checker, ForceStacks};
     Visitor.visitPtr(*AI);
     if (!Visitor.getShouldLiveOnFrame())
       continue;
     Allocas.emplace_back(AI, Visitor.getAliasesCopy(),
                          Visitor.getMayWriteBeforeCoroBegin());
   }
 }
 
for (int Repeat = 0; Repeat < 4; ++Repeat) {
       // point.
       LLVM_DEBUG(dumpSpills("Materializations", Spills));
       rewriteMaterializableInstructions(Builder, Spills);
       Spills.clear();
     }
   }
 
   sinkLifetimeStartMarkers(F, Shape, Checker);
-  if (Shape.ABI != coro::ABI::Async || !Shape.CoroSuspends.empty())
-    collectFrameAllocas(F, Shape, Checker, FrameData.Allocas);
+  auto ForceStacks = collectForceStacks(F);
+  if (Shape.ABI != coro::ABI::Async || !Shape.CoroSuspends.empty()) {
+    collectFrameAllocas(F, Shape, Checker, FrameData.Allocas, ForceStacks);
+    for (const auto &P : ForceStacks) {
+      DeadInstructions.push_back(P.second);
+      DeadInstructions.push_back(P.first);
+    }
+  }
   LLVM_DEBUG(dumpAllocas(FrameData.Allocas));
 
   // Collect the spills for arguments and other not-materializable values.
   for (Argument &A : F.args())
     for (User *U : A.users())
       if (Checker.isDefinitionAcrossSuspend(A, U))
         FrameData.Spills[&A].push_back(cast<Instruction>(U));
 

llvm/test/Transforms/Coroutines/coro-alloca-06.ll

-; Test that in some simple cases allocas will not live on the frame even
-; though their pointers are stored.
+; Test that even though some stores may seem to escape pointers,
+; they can be put on the stack as long as they are within forcestack range.
 ; RUN: opt < %s -coro-split -S | FileCheck %s
 ; RUN: opt < %s -passes=coro-split -S | FileCheck %s
 
 %handle = type { i8* }
 
 define i8* @f() "coroutine.presplit"="1" {
 entry:
   %0 = alloca %"handle", align 8
   %1 = alloca %"handle"*, align 8
   %id = call token @llvm.coro.id(i32 0, i8* null, i8* null, i8* null)
   %size = call i32 @llvm.coro.size.i32()
   %alloc = call i8* @malloc(i32 %size)
   %hdl = call i8* @llvm.coro.begin(token %id, i8* %alloc)
   br label %tricky
 
 tricky:
   %2 = call i8* @await_suspend()
   %3 = getelementptr inbounds %"handle", %"handle"* %0, i32 0, i32 0
   store i8* %2, i8** %3, align 8
-  %4 = bitcast %"handle"** %1 to i8*
-  call void @llvm.lifetime.start.p0i8(i64 8, i8* %4)
+  %4 = call i8* @llvm.coro.forcestack.begin()
   store %"handle"* %0, %"handle"** %1, align 8
   %5 = load %"handle"*, %"handle"** %1, align 8
   %6 = getelementptr inbounds %"handle", %"handle"* %5, i32 0, i32 0
   %7 = load i8*, i8** %6, align 8
-  %8 = bitcast %"handle"** %1 to i8*
-  call void @llvm.lifetime.end.p0i8(i64 8, i8* %8)
+  call void @llvm.coro.forcestack.end(i8* %4)
   br label %finish
 
 finish:
   %sp1 = call i8 @llvm.coro.suspend(token none, i1 false)
   switch i8 %sp1, label %suspend [i8 0, label %resume
   i8 1, label %cleanup]
 resume:
   br label %cleanup

 ; CHECK-LABEL: @f(
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    [[TMP0:%.*]] = alloca [[HANDLE:%.*]], align 8
 ; CHECK-NEXT:    [[TMP1:%.*]] = alloca %handle*, align 8
 
 ; CHECK:         [[TMP2:%.*]] = call i8* @await_suspend()
 ; CHECK-NEXT:    [[TMP3:%.*]] = getelementptr inbounds [[HANDLE]], %handle* [[TMP0]], i32 0, i32 0
 ; CHECK-NEXT:    store i8* [[TMP2]], i8** [[TMP3]], align 8
-; CHECK-NEXT:    [[TMP4:%.*]] = bitcast %handle** [[TMP1]] to i8*
-; CHECK-NEXT:    call void @llvm.lifetime.start.p0i8(i64 8, i8* [[TMP4]])
 ; CHECK-NEXT:    store %handle* [[TMP0]], %handle** [[TMP1]], align 8
-; CHECK-NEXT:    call void @llvm.lifetime.end.p0i8(i64 8, i8* [[TMP4]])
 ;
 
 declare i8* @llvm.coro.free(token, i8*)
 declare i32 @llvm.coro.size.i32()
 declare i8  @llvm.coro.suspend(token, i1)
 declare void @llvm.coro.resume(i8*)
 declare void @llvm.coro.destroy(i8*)
 
 declare token @llvm.coro.id(i32, i8*, i8*, i8*)
 declare i1 @llvm.coro.alloc(token)
 declare i8* @llvm.coro.begin(token, i8*)
 declare i1 @llvm.coro.end(i8*, i1)
+declare i8* @llvm.coro.forcestack.begin()
+declare void @llvm.coro.forcestack.end(i8*)
 
 declare void @llvm.lifetime.start.p0i8(i64, i8* nocapture)
 declare void @llvm.lifetime.end.p0i8(i64, i8* nocapture)
 
 declare i8* @await_suspend()
 declare void @print(i32* nocapture)
 declare noalias i8* @malloc(i32)
 declare void @free(i8*)