[Coroutine][Clang] Force emit lifetime intrinsics for Coroutines

[lxfind]

Summary

tl;dr Correct implementation of Corouintes requires having lifetime intrinsics available.

Coroutine functions are functions that can be suspended and resumed latter. To do so, data that need to stay alive after suspension must be put on the heap (i.e. the coroutine frame).
The optimizer is responsible for analyzing each AllocaInst and figure out whether it should be put on the stack or the frame.
In most cases, for data that we are unable to accurately analyze lifetime, we can just conservatively put them on the heap.
Unfortunately, there exists a few cases where certain data MUST be put on the stack, not on the heap. Without lifetime intrinsics, we are unable to correctly analyze those data's lifetime.

To dig into more details, there exists cases where at certain code points, the current coroutine frame may have already been destroyed. Hence no frame access would be allowed beyond that point.
The following is a common code pattern called "Symmetric Transfer" in coroutine:

auto tmp = await_suspend();
__builtin_coro_resume(tmp.address());
return;

In the above code example, await_suspend() returns a new coroutine handle, which we will obtain the address and then resume that coroutine. This essentially "transfered" from the current coroutine to a different coroutine.
During the call to await_suspend(), the current coroutine may be destroyed, which should be fine because we are not accessing any data afterwards.
However when LLVM is emitting IR for the above code, it needs to emit an AllocaInst for tmp. It will then call the address function on tmp. address function is a member function of coroutine, and there is no way for the LLVM optimizer to know that it does not capture the tmp pointer. So when the optimizer looks at it, it has to conservatively assume that tmp may escape and hence put it on the heap. Furthermore, in some cases address call would be inlined, which will generate a bunch of store/load instructions that move the tmp pointer around. Those stores will also make the compiler to think that tmp might escape.
A repro of crash can be found here: https://godbolt.org/z/KvPY66
To summarize, it's really difficult for the mid-end to figure out that the tmp data is short-lived.
I made some attempt in D98638, but it appears to be way too complex and is basically doing the same thing as inserting lifetime intrinsics in coroutines.

Also, for reference, we already force emitting lifetime intrinsics in O0 for AlwaysInliner: https://github.com/llvm/llvm-project/blob/main/llvm/lib/Passes/PassBuilder.cpp#L1893

I need to fix a few tests. But sending this out early for feedback.

[rjmccall]

I have no objection to trying to always emit lifetime intrinsics in coroutines since it has a less-trivial runtime cost. I am skeptical that it's reasonable to do this for *correctness*, however; I don't think the frontend unconditionally emits lifetime intrinsics. But since I think this fine to do regardless, I have no objection to the patch.

I think you just set ShouldEmitLifetimeMarkers correctly in the first place instead of adding this as an extra condition to every place that considers it, however.

[rjmccall]

In D99227#2646532, @rjmccall wrote:

I am skeptical that it's reasonable to do this for *correctness*, however; I don't think the frontend unconditionally emits lifetime intrinsics.

Sorry, I re-read this after posting, and it's not exactly clear what I was saying. There are a lot of situations where Clang doesn't emit lifetime intrinsics for every alloca it emits, or emits unnecessarily weak bounds. Certain LLVM transforms can also introduce allocas that don't have corresponding lifetime intrinsics. So I think it's problematic to consider it a correctness condition that we're emitting optimally-tight lifetimes.

[ChuanqiXu]

Only one problem I had for emitting lifetime markers even at O0 is that would action make allocas to be optimized even at O0? If so, I wonder if it confuses programmers since they may find some variables disappear surprisingly. Or there would be no optimization since every function would be marked with optnone attribute. I am not sure about this.

If I understand this problem correctly, this patch could fix problems for the return value of symmetric transfer and the gro that we discussed in D98638. Then D98638 may be unneeded. I prefer the implementation in this patch.

clang/lib/CodeGen/CGDecl.cpp

1318 ↗

(On Diff #332826)

Can we sure frontend would always call this API to emit lifetime start? I mean the frontend may call EmitIntrinsic or create lifetime.start intrinsic directly whether by IRBuilder::CreateXXX or Instrinsic::Create(...). I worry about if this would incur changes out of design.

Then if we add check in EmitLifetimeStart, why not we add check in EmitLfietimeEnd?

[lxfind]

I think you just set ShouldEmitLifetimeMarkers correctly in the first place instead of adding this as an extra condition to every place that considers it, however.

This was set when a CodeGenFunction is constructed, at that point it doesn't yet know if this function is a coroutine.
I could turn ShouldEmitLifetimeMarkers to non-const, and then modify it once it realizes it's a coroutine though, if that's better than the current approach.

Sorry, I re-read this after posting, and it's not exactly clear what I was saying. There are a lot of situations where Clang doesn't emit lifetime intrinsics for every alloca it emits, or emits unnecessarily weak bounds. Certain LLVM transforms can also introduce allocas that don't have corresponding lifetime intrinsics. So I think it's problematic to consider it a correctness condition that we're emitting optimally-tight lifetimes.

I tend to agree. Relying on lifetime for correctness seems fragile.
I wonder if there is a better way to inform optimizer that a "variable" is really a temporary value that should die at the end of an expression?
For instance, whenever we do something simple like:

foo().bar();
co_await ...

If we compile it under -O0 without lifetime intrinsics, the return value of foo() will always be put on the coroutine frame, unless the compiler knows in advance that bar() does not capture.
This becomes a problem if this code appears at a location where the current coroutine frame may be destroyed (but the code itself isn't wrong, it simply doesn't access the frame).
The case for symmetric transfer is exactly this situation.

An alternative to solve the problem for the case of symmetric transfer, is to change the design of symmetric transfer. For example, if we let await_suspend to return void* instead of coroutine_handle, we won't have this problem in the first place, because we no longer need to call address(). Maybe @lewissbaker can comment on the viability of that.

[lxfind]

In D99227#2646568, @ChuanqiXu wrote:

Only one problem I had for emitting lifetime markers even at O0 is that would action make allocas to be optimized even at O0? If so, I wonder if it confuses programmers since they may find some variables disappear surprisingly. Or there would be no optimization since every function would be marked with optnone attribute. I am not sure about this.

It will only cause variables to be put on the stack instead of on the frame, which shouldn't affect developer's view?

If I understand this problem correctly, this patch could fix problems for the return value of symmetric transfer and the gro that we discussed in D98638. Then D98638 may be unneeded. I prefer the implementation in this patch.

I doubt it can fix the gro problem. I will need to double check on that latter.

[lxfind]

clang/lib/CodeGen/CGDecl.cpp

1318 ↗

(On Diff #332826)

I searched in the codebase, and we always call this API to emit lifetime start in the front-end.
Also, for coroutine to behave correctly, we really only need SD_FullExpression to be able to emit it. Other cases are less critical.

Usually when it emits a LifetimeStart instruction, it will store it somewhere, and latter check on it to decide whether it needs to emit a lifetime end. That's when there is no checks needed for lifetime end.

[ChuanqiXu]

In D99227#2646719, @lxfind wrote:

In D99227#2646568, @ChuanqiXu wrote:

Only one problem I had for emitting lifetime markers even at O0 is that would action make allocas to be optimized even at O0? If so, I wonder if it confuses programmers since they may find some variables disappear surprisingly. Or there would be no optimization since every function would be marked with optnone attribute. I am not sure about this.

It will only cause variables to be put on the stack instead of on the frame, which shouldn't affect developer's view?

Yes, I am just worry about the variable marked with lifetime intrinsic would be optimized by other passes. But functions would get attribute optnone in O0, my worries may be redundant. Then it is Ok to me to emit lifetime intrinsics all the time.

[rjmccall]

Is it feasible to outline the initial segment that you don't want to be part of the coroutine, and then have coroutine splitting force that outlined function to be inlined into the ramp function? IIUC, you were saying that the splitting patch was difficult, but maybe thinking about it as outlining simplifies things. I know we had some nasty representational problems with the async lowering that we solved with outlining and force-inlining.

[rjmccall]

In D99227#2646710, @lxfind wrote:

I think you just set ShouldEmitLifetimeMarkers correctly in the first place instead of adding this as an extra condition to every place that considers it, however.

This was set when a CodeGenFunction is constructed, at that point it doesn't yet know if this function is a coroutine.
I could turn ShouldEmitLifetimeMarkers to non-const, and then modify it once it realizes it's a coroutine though, if that's better than the current approach.

That would be fine.

[lxfind]

In D99227#2646819, @rjmccall wrote:

Is it feasible to outline the initial segment that you don't want to be part of the coroutine, and then have coroutine splitting force that outlined function to be inlined into the ramp function? IIUC, you were saying that the splitting patch was difficult, but maybe thinking about it as outlining simplifies things. I know we had some nasty representational problems with the async lowering that we solved with outlining and force-inlining.

That's a good idea. I will think about it. Thanks!

[lxfind]

Address comments, and fix all tests

[rjmccall]

LGTM

clang/lib/CodeGen/CGCoroutine.cpp

void CodeGenFunction::EmitCoroutineBody(const CoroutineBodyStmt &S) {
   auto *FinalBB = createBasicBlock("coro.final");
   auto *RetBB = createBasicBlock("coro.ret");
 
   auto *CoroId = Builder.CreateCall(
       CGM.getIntrinsic(llvm::Intrinsic::coro_id),
       {Builder.getInt32(NewAlign), NullPtr, NullPtr, NullPtr});
   createCoroData(*this, CurCoro, CoroId);
   CurCoro.Data->SuspendBB = RetBB;
+  assert(ShouldEmitLifetimeMarkers &&
+         "Must emit lifetime intrinsics for coroutines");
 
   // Backend is allowed to elide memory allocations, to help it, emit
   // auto mem = coro.alloc() ? 0 : ... allocation code ...;
   auto *CoroAlloc = Builder.CreateCall(
       CGM.getIntrinsic(llvm::Intrinsic::coro_alloc), {CoroId});
 
   Builder.CreateCondBr(CoroAlloc, AllocBB, InitBB);
 

clang/lib/CodeGen/CodeGenFunction.h

private:
 
   /// Terminate funclets keyed by parent funclet pad.
   llvm::MapVector<llvm::Value *, llvm::BasicBlock *> TerminateFunclets;
 
   /// Largest vector width used in ths function. Will be used to create a
   /// function attribute.
   unsigned LargestVectorWidth = 0;
 
-  /// True if we need emit the life-time markers.
-  const bool ShouldEmitLifetimeMarkers;
+  /// True if we need emit the life-time markers. This is initially set in
+  /// the constructor, but could be overwritten to true if this is a coroutine.
+  bool ShouldEmitLifetimeMarkers;
 
   /// Add OpenCL kernel arg metadata and the kernel attribute metadata to
   /// the function metadata.
   void EmitOpenCLKernelMetadata(const FunctionDecl *FD,
                                 llvm::Function *Fn);
 
 public:
   CodeGenFunction(CodeGenModule &cgm, bool suppressNewContext=false);

clang/lib/CodeGen/CodeGenFunction.cpp

void CodeGenFunction::GenerateCode(GlobalDecl GD, llvm::Function *Fn,
   // If this is a function specialization then use the pattern body
   // as the location for the function.
   if (const FunctionDecl *SpecDecl = FD->getTemplateInstantiationPattern())
     if (SpecDecl->hasBody(SpecDecl))
       Loc = SpecDecl->getLocation();
 
   Stmt *Body = FD->getBody();
 
-  // Initialize helper which will detect jumps which can cause invalid lifetime
-  // markers.
-  if (Body && ShouldEmitLifetimeMarkers)
+  if (Body) {
+    // Coroutines always emit lifetime markers.
+    if (isa<CoroutineBodyStmt>(Body))
+      ShouldEmitLifetimeMarkers = true;
+
+    // Initialize helper which will detect jumps which can cause invalid
+    // lifetime markers.
+    if (ShouldEmitLifetimeMarkers)
       Bypasses.Init(Body);
+  }
 
   // Emit the standard function prologue.
   StartFunction(GD, ResTy, Fn, FnInfo, Args, Loc, BodyRange.getBegin());
 
   // Generate the body of the function.
   PGO.assignRegionCounters(GD, CurFn);
   if (isa<CXXDestructorDecl>(FD))
     EmitDestructorBody(Args);

clang/test/CodeGenCoroutines/coro-alloc.cpp

extern "C" int f4(promise_on_alloc_failure_tag) {
   // CHECK:   br label %[[RetBB:.+]]
 
   // CHECK: [[OKBB]]:
   // CHECK:   %[[OkRet:.+]] = call i32 @_ZNSt12experimental16coroutine_traitsIJi28promise_on_alloc_failure_tagEE12promise_type17get_return_objectEv(
   // CHECK:   store i32 %[[OkRet]], i32* %[[Gro]]
 
   // CHECK: %[[Tmp1:.*]] = load i32, i32* %[[Gro]]
   // CHECK-NEXT: store i32 %[[Tmp1]], i32* %[[RetVal]]
+  // CHECK-NEXT: %[[Gro_CAST:.+]] = bitcast i32* %[[Gro]] to i8*
+  // CHECK-NEXT: call void @llvm.lifetime.end.p0i8(i64 4, i8* %[[Gro_CAST]]) #2
   // CHECK-NEXT: br label %[[RetBB]]
 
   // CHECK: [[RetBB]]:
   // CHECK:   %[[LoadRet:.+]] = load i32, i32* %[[RetVal]], align 4
   // CHECK:   ret i32 %[[LoadRet]]
   co_return;
 }

clang/test/CodeGenCoroutines/coro-await-resume-eh.cpp

throwing_task f() {
   // CHECK: [[RESUMECATCH]]:
   // CHECK: invoke void @_ZN13throwing_task12promise_type19unhandled_exceptionEv
   // CHECK-NEXT: to label %[[RESUMEENDCATCH:.+]] unwind label
   // CHECK: [[RESUMEENDCATCH]]:
   // CHECK-NEXT: invoke void @__cxa_end_catch()
   // CHECK-NEXT: to label %[[RESUMEENDCATCHCONT:.+]] unwind label
   // CHECK: [[RESUMEENDCATCHCONT]]:
   // CHECK-NEXT: br label %[[RESUMETRYCONT]]
+  // CHECK: [[RESUMETRYCONT]]:
+  // CHECK-NEXT: br label %[[CLEANUP:.+]]
+  // CHECK: [[CLEANUP]]:
+  // CHECK: switch i32 %{{.+}}, label %{{.+}} [
+  // CHECK-NEXT: i32 0, label %[[CLEANUPCONT:.+]]
+  // CHECK-NEXT: ]
 
   // The variable RESUMETHREW is loaded and if true, then 'await_resume'
   // threw an exception and the coroutine body is skipped, and the final
   // suspend is executed immediately. Otherwise, the coroutine body is
   // executed, and then the final suspend.
-  // CHECK: [[RESUMETRYCONT]]:
+  // CHECK: [[CLEANUPCONT]]:
   // CHECK-NEXT: %[[RESUMETHREWLOAD:.+]] = load i1, i1* %[[RESUMETHREW]]
   // CHECK-NEXT: br i1 %[[RESUMETHREWLOAD]], label %[[RESUMEDCONT:.+]], label %[[RESUMEDBODY:.+]]
 
   // CHECK: [[RESUMEDBODY]]:
   // CHECK: invoke void @_ZN13throwing_task12promise_type11return_voidEv
   // CHECK-NEXT: to label %[[REDUMEDBODYCONT:.+]] unwind label
   // CHECK: [[REDUMEDBODYCONT]]:
   // CHECK-NEXT: br label %[[COROFINAL:.+]]
 
   // CHECK: [[RESUMEDCONT]]:
   // CHECK-NEXT: br label %[[COROFINAL]]
 
   // CHECK: [[COROFINAL]]:
-  // CHECK-NEXT: call void @_ZN13throwing_task12promise_type13final_suspendEv
+  // CHECK: call void @_ZN13throwing_task12promise_type13final_suspendEv
   co_return;
 }
 
 struct noexcept_awaitable {
   bool await_ready() { return true; }
   void await_suspend(coro::coroutine_handle<>) {}
   void await_resume() noexcept {}
 };

clang/test/CodeGenCoroutines/coro-await.cpp

 // CHECK-LABEL: @TestScalar(
 extern "C" void TestScalar() {
   UseScalar(co_await ScalarAwaiter{});
   // CHECK: %[[Result:.+]] = call i32 @_ZN13ScalarAwaiter12await_resumeEv(%struct.ScalarAwaiter*
   // CHECK: call void @UseScalar(i32 %[[Result]])
 
   int Val = co_await ScalarAwaiter{};
   // CHECK: %[[Result2:.+]] = call i32 @_ZN13ScalarAwaiter12await_resumeEv(%struct.ScalarAwaiter*
-  // CHECK: store i32 %[[Result2]], i32* %Val
+  // CHECK: store i32 %[[Result2]], i32* %[[TMP_EXPRCLEANUP:.+]],
+  // CHECK: %[[TMP:.+]] = load i32, i32* %[[TMP_EXPRCLEANUP]],
+  // CHECK: store i32 %[[TMP]], i32* %Val,
 
   co_await ScalarAwaiter{};
   // CHECK: call i32 @_ZN13ScalarAwaiter12await_resumeEv(%struct.ScalarAwaiter*
 }
 
 // Test operator co_await codegen.
 enum class MyInt: int {};
 ScalarAwaiter operator co_await(MyInt);

 void AwaitReturnsLValue(double) {
   AwaitResumeReturnsLValue a;
   // CHECK: %[[AVAR:.+]] = alloca %struct.AwaitResumeReturnsLValue,
   // CHECK: %[[XVAR:.+]] = alloca %struct.RefTag*,
 
   // CHECK: %[[YVAR:.+]] = alloca %struct.RefTag*,
   // CHECK-NEXT: %[[TMP1:.+]] = alloca %struct.AwaitResumeReturnsLValue,
 
+  // CHECK: %[[TMP_EXPRCLEANUP1:.+]] = alloca %struct.RefTag*,
   // CHECK: %[[ZVAR:.+]] = alloca %struct.RefTag*,
   // CHECK-NEXT: %[[TMP2:.+]] = alloca %struct.AwaitResumeReturnsLValue,
+  // CHECK: %[[TMP_EXPRCLEANUP2:.+]] = alloca %struct.RefTag*,
 
   // CHECK: %[[RES1:.+]] = call nonnull align 1 dereferenceable({{.*}}) %struct.RefTag* @_ZN24AwaitResumeReturnsLValue12await_resumeEv(%struct.AwaitResumeReturnsLValue* {{[^,]*}} %[[AVAR]])
   // CHECK-NEXT: store %struct.RefTag* %[[RES1]], %struct.RefTag** %[[XVAR]],
   RefTag& x = co_await a;
 
   // CHECK: %[[RES2:.+]] = call nonnull align 1 dereferenceable({{.*}}) %struct.RefTag* @_ZN24AwaitResumeReturnsLValue12await_resumeEv(%struct.AwaitResumeReturnsLValue* {{[^,]*}} %[[TMP1]])
-  // CHECK-NEXT: store %struct.RefTag* %[[RES2]], %struct.RefTag** %[[YVAR]],
+  // CHECK-NEXT: store %struct.RefTag* %[[RES2]], %struct.RefTag** %[[TMP_EXPRCLEANUP1]],
+  // CHECK: %[[LOAD_TMP1:.+]] = load %struct.RefTag*, %struct.RefTag** %[[TMP_EXPRCLEANUP1]],
+  // CHECK: store %struct.RefTag* %[[LOAD_TMP1]], %struct.RefTag** %[[YVAR]],
 
   RefTag& y = co_await AwaitResumeReturnsLValue{};
   // CHECK: %[[RES3:.+]] = call nonnull align 1 dereferenceable({{.*}}) %struct.RefTag* @_ZN24AwaitResumeReturnsLValue12await_resumeEv(%struct.AwaitResumeReturnsLValue* {{[^,]*}} %[[TMP2]])
-  // CHECK-NEXT: store %struct.RefTag* %[[RES3]], %struct.RefTag** %[[ZVAR]],
+  // CHECK-NEXT: store %struct.RefTag* %[[RES3]], %struct.RefTag** %[[TMP_EXPRCLEANUP2]],
+  // CHECK: %[[LOAD_TMP2:.+]] = load %struct.RefTag*, %struct.RefTag** %[[TMP_EXPRCLEANUP2]],
+  // CHECK: store %struct.RefTag* %[[LOAD_TMP2]], %struct.RefTag** %[[ZVAR]],
   RefTag& z = co_yield 42;
 }
 
 struct TailCallAwait {
   bool await_ready();
   std::experimental::coroutine_handle<> await_suspend(std::experimental::coroutine_handle<>);
   void await_resume();
 };

clang/test/CodeGenCoroutines/coro-dest-slot.cpp

struct promise_type {
     void return_void();
     static void unhandled_exception();
   };
 };
 
 extern "C" coro f(int) { co_return; }
 // Verify that cleanup.dest.slot is eliminated in a coroutine.
 // CHECK-LABEL: f(
+// CHECK: %[[INIT_SUSPEND:.+]] = call i8 @llvm.coro.suspend(
+// CHECK-NEXT: switch i8 %[[INIT_SUSPEND]], label
+// CHECK-NEXT:   i8 0, label %[[INIT_READY:.+]]
+// CHECK-NEXT:   i8 1, label %[[INIT_CLEANUP:.+]]
+// CHECK-NEXT: ]
+// CHECK: %[[CLEANUP_DEST0:.+]] = phi i32 [ 0, %[[INIT_READY]] ], [ 2, %[[INIT_CLEANUP]] ]
+
+// CHECK: %[[FINAL_SUSPEND:.+]] = call i8 @llvm.coro.suspend(
+// CHECK-NEXT: switch i8 %29, label %coro.ret [
+// CHECK-NEXT:   i8 0, label %[[FINAL_READY:.+]]
+// CHECK-NEXT:   i8 1, label %[[FINAL_CLEANUP:.+]]
+// CHECK-NEXT: ]
+
 // CHECK: call void @_ZNSt12experimental13coroutines_v113suspend_never12await_resumeEv(
-// CHECK: %[[CLEANUP_DEST:.+]] = phi i32 [ 0, %{{.+}} ], [ 2, %{{.+}} ], [ 2, %{{.+}} ]
+// CHECK: %[[CLEANUP_DEST1:.+]] = phi i32 [ 0, %[[FINAL_READY]] ], [ 2, %[[FINAL_CLEANUP]] ]
+// CHECK: %[[CLEANUP_DEST2:.+]] = phi i32 [ %[[CLEANUP_DEST0]], %{{.+}} ], [ %[[CLEANUP_DEST1]], %{{.+}} ], [ 0, %{{.+}} ]
 // CHECK: call i8* @llvm.coro.free(
-// CHECK: switch i32 %cleanup.dest.slot.0, label %{{.+}} [
+// CHECK: switch i32 %[[CLEANUP_DEST2]], label %{{.+}} [
 // CHECK-NEXT: i32 0
 // CHECK-NEXT: i32 2
 // CHECK-NEXT: ]

clang/test/CodeGenCoroutines/coro-params.cpp

 // CHECK: define{{.*}} void @_Z1fi8MoveOnly11MoveAndCopy(i32 %val, %struct.MoveOnly* %[[MoParam:.+]], %struct.MoveAndCopy* %[[McParam:.+]]) #0 personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to i8*
 void f(int val, MoveOnly moParam, MoveAndCopy mcParam) {
   // CHECK: %[[MoCopy:.+]] = alloca %struct.MoveOnly
   // CHECK: %[[McCopy:.+]] = alloca %struct.MoveAndCopy
   // CHECK: store i32 %val, i32* %[[ValAddr:.+]]
 
   // CHECK: call i8* @llvm.coro.begin(
   // CHECK: call void @_ZN8MoveOnlyC1EOS_(%struct.MoveOnly* {{[^,]*}} %[[MoCopy]], %struct.MoveOnly* nonnull align 4 dereferenceable(4) %[[MoParam]])
+  // CHECK-NEXT: bitcast %struct.MoveAndCopy* %[[McCopy]] to i8*
+  // CHECK-NEXT: call void @llvm.lifetime.start.p0i8(
   // CHECK-NEXT: call void @_ZN11MoveAndCopyC1EOS_(%struct.MoveAndCopy* {{[^,]*}} %[[McCopy]], %struct.MoveAndCopy* nonnull align 4 dereferenceable(4) %[[McParam]]) #
+  // CHECK-NEXT: bitcast %"struct.std::experimental::coroutine_traits<void, int, MoveOnly, MoveAndCopy>::promise_type"* %__promise to i8*
+  // CHECK-NEXT: call void @llvm.lifetime.start.p0i8(
   // CHECK-NEXT: invoke void @_ZNSt12experimental16coroutine_traitsIJvi8MoveOnly11MoveAndCopyEE12promise_typeC1Ev(
 
   // CHECK: call void @_ZN14suspend_always12await_resumeEv(
   // CHECK: %[[IntParam:.+]] = load i32, i32* %val1
   // CHECK: %[[MoGep:.+]] = getelementptr inbounds %struct.MoveOnly, %struct.MoveOnly* %[[MoCopy]], i32 0, i32 0
   // CHECK: %[[MoVal:.+]] = load i32, i32* %[[MoGep]]
   // CHECK: %[[McGep:.+]] =  getelementptr inbounds %struct.MoveAndCopy, %struct.MoveAndCopy* %[[McCopy]], i32 0, i32 0
   // CHECK: %[[McVal:.+]] = load i32, i32* %[[McGep]]
   // CHECK: call void @_Z7consumeiii(i32 %[[IntParam]], i32 %[[MoVal]], i32 %[[McVal]])
 
   consume(val, moParam.val, mcParam.val);
   co_return;
 
   // Skip to final suspend:
   // CHECK: call void @_ZNSt12experimental16coroutine_traitsIJvi8MoveOnly11MoveAndCopyEE12promise_type13final_suspendEv(
   // CHECK: call void @_ZN14suspend_always12await_resumeEv(
 
   // Destroy promise, then parameter copies:
-  // CHECK: call void @_ZNSt12experimental16coroutine_traitsIJvi8MoveOnly11MoveAndCopyEE12promise_typeD1Ev(%"struct.std::experimental::coroutine_traits<void, int, MoveOnly, MoveAndCopy>::promise_type"* {{[^,]*}} %__promise) #2
+  // CHECK: call void @_ZNSt12experimental16coroutine_traitsIJvi8MoveOnly11MoveAndCopyEE12promise_typeD1Ev(%"struct.std::experimental::coroutine_traits<void, int, MoveOnly, MoveAndCopy>::promise_type"* {{[^,]*}} %__promise)
+  // CHECK-NEXT: bitcast %"struct.std::experimental::coroutine_traits<void, int, MoveOnly, MoveAndCopy>::promise_type"* %__promise to i8*
+  // CHECK-NEXT: call void @llvm.lifetime.end.p0i8(
   // CHECK-NEXT: call void @_ZN11MoveAndCopyD1Ev(%struct.MoveAndCopy* {{[^,]*}} %[[McCopy]])
+  // CHECK-NEXT: bitcast %struct.MoveAndCopy* %[[McCopy]] to i8*
+  // CHECK-NEXT: call void @llvm.lifetime.end.p0i8(
   // CHECK-NEXT: call void @_ZN8MoveOnlyD1Ev(%struct.MoveOnly* {{[^,]*}} %[[MoCopy]]
+  // CHECK-NEXT: bitcast %struct.MoveOnly* %[[MoCopy]] to i8*
+  // CHECK-NEXT: call void @llvm.lifetime.end.p0i8(
+  // CHECK-NEXT: bitcast i32* %{{.+}} to i8*
+  // CHECK-NEXT: call void @llvm.lifetime.end.p0i8(
   // CHECK-NEXT: call i8* @llvm.coro.free(
 }
 
 // CHECK-LABEL: void @_Z16dependent_paramsI1A1BEvT_T0_S3_(%struct.A* %x, %struct.B* %0, %struct.B* %y)
 template <typename T, typename U>
 void dependent_params(T x, U, U y) {
   // CHECK: %[[x_copy:.+]] = alloca %struct.A
   // CHECK-NEXT: %[[unnamed_copy:.+]] = alloca %struct.B
   // CHECK-NEXT: %[[y_copy:.+]] = alloca %struct.B
 
   // CHECK: call i8* @llvm.coro.begin
+  // CHECK-NEXT: bitcast %struct.A* %[[x_copy]] to i8*
+  // CHECK-NEXT: call void @llvm.lifetime.start.p0i8(
   // CHECK-NEXT: call void @_ZN1AC1EOS_(%struct.A* {{[^,]*}} %[[x_copy]], %struct.A* nonnull align 4 dereferenceable(512) %x)
+  // CHECK-NEXT: bitcast %struct.B* %[[unnamed_copy]] to i8*
+  // CHECK-NEXT: call void @llvm.lifetime.start.p0i8(
   // CHECK-NEXT: call void @_ZN1BC1EOS_(%struct.B* {{[^,]*}} %[[unnamed_copy]], %struct.B* nonnull align 4 dereferenceable(512) %0)
+  // CHECK-NEXT: %10 = bitcast %struct.B* %[[y_copy]] to i8*
+  // CHECK-NEXT: call void @llvm.lifetime.start.p0i8(
   // CHECK-NEXT: call void @_ZN1BC1EOS_(%struct.B* {{[^,]*}} %[[y_copy]], %struct.B* nonnull align 4 dereferenceable(512) %y)
+  // CHECK-NEXT: bitcast %"struct.std::experimental::coroutine_traits<void, A, B, B>::promise_type"* %__promise to i8*
+  // CHECK-NEXT: call void @llvm.lifetime.start.p0i8(
   // CHECK-NEXT: invoke void @_ZNSt12experimental16coroutine_traitsIJv1A1BS2_EE12promise_typeC1Ev(
 
   co_return;
 }
 
 struct A {
   int WontFitIntoRegisterForSure[128];
   A();

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 -triple x86_64-unknown-linux-gnu -fcoroutines-ts -std=c++14 -O0 -emit-llvm %s -o - -disable-llvm-passes | FileCheck %s
 
 #include "Inputs/coroutine.h"
 
 namespace coro = std::experimental::coroutines_v1;
 
 struct detached_task {
   struct promise_type {
     detached_task get_return_object() noexcept {

 };
 
 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:         %{{.+}} = call token @llvm.coro.save(i8* null)
+// CHECK:         %[[HDL_CAST1:.+]] = bitcast %"struct.std::experimental::coroutines_v1::coroutine_handle.0"* %[[HDL:.+]] to i8*
+// CHECK:         call void @llvm.lifetime.start.p0i8(i64 8, i8* %[[HDL_CAST1]])
+// CHECK:         %[[CALL:.+]] = call i8* @_ZN13detached_task12promise_type13final_awaiter13await_suspendENSt12experimental13coroutines_v116coroutine_handleIS0_EE(
+// CHECK:         %[[HDL_CAST2:.+]] = getelementptr inbounds %"struct.std::experimental::coroutines_v1::coroutine_handle.0", %"struct.std::experimental::coroutines_v1::coroutine_handle.0"* %[[HDL]], i32 0, i32 0
+// CHECK:         store i8* %[[CALL]], i8** %[[HDL_CAST2]], align 8
+// CHECK:         %[[HDL_TRANSFER:.+]] = call i8* @_ZNKSt12experimental13coroutines_v116coroutine_handleIvE7addressEv(%"struct.std::experimental::coroutines_v1::coroutine_handle.0"* nonnull dereferenceable(8) %[[HDL]])
+// CHECK:         %[[HDL_CAST3:.+]] = bitcast %"struct.std::experimental::coroutines_v1::coroutine_handle.0"* %[[HDL]] to i8*
+// CHECK:         call void @llvm.lifetime.end.p0i8(i64 8, i8* %[[HDL_CAST3]])
+// CHECK:         call void @llvm.coro.resume(i8* %[[HDL_TRANSFER]])

clang/test/CodeGenCoroutines/coro-unhandled-exception.cpp

 // CHECK:   %[[CATCHTOK:.+]] = catchpad within [[CATCHSWTOK:.+]]
 // CHECK:   call void @"?unhandled_exception@promise_type@coro_t@@QEAAXXZ"
 // CHECK:   catchret from %[[CATCHTOK]] to label %[[CATCHRETDEST:.+]]
 // CHECK: [[CATCHRETDEST]]:
 // CHECK-NEXT: br label %[[TRYCONT:.+]]
 // CHECK: [[TRYCONT]]:
 // CHECK-NEXT: br label %[[COROFIN:.+]]
 // CHECK: [[COROFIN]]:
+// CHECK-NEXT: bitcast %"struct.std::experimental::coroutines_v1::suspend_never"* %{{.+}} to i8*
+// CHECK-NEXT: call void @llvm.lifetime.start.p0i8(
 // CHECK-NEXT: call void @"?final_suspend@promise_type@coro_t@@QEAA?AUsuspend_never@coroutines_v1@experimental@std@@XZ"(
 
 // CHECK-LPAD: @_Z1fv(
 // CHECK-LPAD:   invoke void @_Z9may_throwv()
 // CHECK-LPAD:       to label %[[CONT:.+]] unwind label %[[CLEANUP:.+]]
 // CHECK-LPAD: [[CLEANUP]]:
 // CHECK-LPAD:   call void @_ZN7CleanupD1Ev(%struct.Cleanup* {{[^,]*}} %x) #2
 // CHECK-LPAD:   br label %[[CATCH:.+]]
 
 // CHECK-LPAD: [[CATCH]]:
 // CHECK-LPAD:    call i8* @__cxa_begin_catch
 // CHECK-LPAD:    call void @_ZN6coro_t12promise_type19unhandled_exceptionEv(%"struct.coro_t::promise_type"* {{[^,]*}} %__promise) #2
 // CHECK-LPAD:    invoke void @__cxa_end_catch()
 // CHECK-LPAD-NEXT:  to label %[[CATCHRETDEST:.+]] unwind label
 // CHECK-LPAD: [[CATCHRETDEST]]:
 // CHECK-LPAD-NEXT: br label %[[TRYCONT:.+]]
 // CHECK-LPAD: [[TRYCONT]]:
 // CHECK-LPAD: br label %[[COROFIN:.+]]
 // CHECK-LPAD: [[COROFIN]]:
+// CHECK-LPAD-NEXT: bitcast %"struct.std::experimental::coroutines_v1::suspend_never"* %{{.+}} to i8*
+// CHECK-LPAD-NEXT: call void @llvm.lifetime.start.p0i8(
 // CHECK-LPAD-NEXT: call void @_ZN6coro_t12promise_type13final_suspendEv(