[Coroutines] Fix premature conversion of return object

[bruno]

Summary

Fix https://github.com/llvm/llvm-project/issues/56532

Effectively, this reverts behavior introduced in https://reviews.llvm.org/D117087,
which did two things:

1. Change delayed to early conversion of return object.
2. Introduced RVO possibilities because of early conversion.

This patches fixes (1) and removes (2). I already worked on a follow up for (2)
in a separated patch. I believe it's important to split these two because if the RVO
causes any problems we can explore reverting (2) while maintaining (1).

Notes on some testcase changes:

• pr59221.cpp changed to -O1 so we can check that the front-end honors the value checked for. Sounds like -O3 without RVO is more likely to work with LLVM optimizations...
• Comment out delete members coroutine-no-move-ctor.cpp since behavior now requires copies again.

[ChuanqiXu]

This is basically a reverting of https://reviews.llvm.org/D117087. So it should be good according to our previous talk.

clang/include/clang/AST/StmtCXX.h

410–414

Maybe we can delete this.

clang/lib/Sema/SemaCoroutine.cpp

1752–1791

Let's remove this one.

[bruno]

Test failures are unrelated, thanks for the review @ChuanqiXu

[ilya-biryukov]

Reverting the RVO breaks some coroutine code we have. The short repro is https://gcc.godbolt.org/z/1543qc8Ee (code at the end of the post, very similar to the deleted constructor in warn-throw-out-noexcept-coro.cpp):
The RVO seems to be mandated by the standard and D145641 is in the works to fix this.

Could we revert this change and wait for D145641 to land before reverting existing RVO behavior?
This would unblock our compiler releases (we live at head). This looks like the right trade-off given that previous behavior was in Clang for more than a year after D117087 landed.

Reproducer (works in Clang 15 and GCC, fails in trunk):

#include <coroutine>

struct MyTask{
  struct promise_type {
    MyTask get_return_object() { return MyTask{std::coroutine_handle<promise_type>::from_promise(*this)}; }
    std::suspend_always initial_suspend() { return {}; }

    void unhandled_exception();
    void return_void() {} 

    auto await_transform(MyTask task) {
      struct Awaiter {
        bool await_ready() { return false; }
        std::coroutine_handle<promise_type> await_suspend(std::coroutine_handle<promise_type> h);
        std::nullptr_t await_resume();

        promise_type& caller;
        promise_type& callee;
      };

      return Awaiter{*this, task.handle.promise()};
    }
    
    auto final_suspend() noexcept {
      struct Awaiter {
        bool await_ready() noexcept { return false; }
        std::coroutine_handle<promise_type> await_suspend(std::coroutine_handle<promise_type> h) noexcept;
        void await_resume() noexcept;

        std::coroutine_handle<promise_type> to_resume;
      };

      return Awaiter{resume_when_done};
    }

    // The coroutine to resume when we're done.
    std::coroutine_handle<promise_type> resume_when_done;
  };

  explicit MyTask(std::coroutine_handle<promise_type>);
  MyTask(MyTask&&) = delete;

  // A handle for the coroutine that returned this task.
  std::coroutine_handle<promise_type> handle;
};

MyTask DoNothing() {
  co_return;
}

[ChuanqiXu]

The reproducer seems like what we're searching for tests in https://reviews.llvm.org/D145641. Reverting is common in Clang/LLVM. But let's wait for the response from @bruno temporarily. I think it makes sense to revert this one if @bruno don't response tomorrow. And we can commit this one and D145641 quickly the next time. Recommit is common too.

[ChuanqiXu]

The RVO seems to be mandated by the standard.

Yeah, I thought so... But WG21 decided to not make it in a recent meeting... So we can only make it as a non-mandated compiler optimization.

[ilya-biryukov]

In D145639#4198794, @ChuanqiXu wrote:

Reverting is common in Clang/LLVM. But let's wait for the response from @bruno temporarily. I think it makes sense to revert this one if @bruno don't response tomorrow. And we can commit this one and D145641 quickly the next time. Recommit is common too.

@bgraur, FYI, hope waiting for a day works for this case.

In D145639#4198815, @ChuanqiXu wrote:

The RVO seems to be mandated by the standard.

Yeah, I thought so... But WG21 decided to not make it in a recent meeting... So we can only make it as a non-mandated compiler optimization.

That's unexpected. Are there any wording change proposals published?
Our understanding comes from the reading of dcl.fct.def.coroutine/7

The expression promise.get_return_object() is used to initialize the glvalue result or prvalue result object of a call to a coroutine.

And we think RVO is mandated when initializing the result objects mentioned in general.
Also, from https://github.com/llvm/llvm-project/issues/56532:

Q: if the initialization does occur later, by what mechanism the prvalue result of get_return_object is forwarded to that initialization.
A: (see below)
Case 1/2. Same type of get_return_object and coroutine return type.
Constructed in the return slot.
Case 3. Different types:
(1) Constructed temporary object prior to initial suspend initialized with a call to get_return_object()
(2) when coroutine needs to to return to the caller and needs to construct return value for the coroutine it is initialized with expiring value of the temporary obtained in step 1.

[ChuanqiXu]

In D145639#4198837, @ilya-biryukov wrote:

In D145639#4198794, @ChuanqiXu wrote:

! In D145639#4198815, @ChuanqiXu wrote:
The RVO seems to be mandated by the standard.

Yeah, I thought so... But WG21 decided to not make it in a recent meeting... So we can only make it as a non-mandated compiler optimization.

That's unexpected. Are there any wording change proposals published?
Our understanding comes from the reading of dcl.fct.def.coroutine/7

The expression promise.get_return_object() is used to initialize the glvalue result or prvalue result object of a call to a coroutine.

And we think RVO is mandated when initializing the result objects mentioned in general.
Also, from https://github.com/llvm/llvm-project/issues/56532:

Q: if the initialization does occur later, by what mechanism the prvalue result of get_return_object is forwarded to that initialization.
A: (see below)
Case 1/2. Same type of get_return_object and coroutine return type.
Constructed in the return slot.
Case 3. Different types:
(1) Constructed temporary object prior to initial suspend initialized with a call to get_return_object()
(2) when coroutine needs to to return to the caller and needs to construct return value for the coroutine it is initialized with expiring value of the temporary obtained in step 1.

The formal file shouldn't be released yet. But we can find the issue report and the proposed solution in https://cplusplus.github.io/CWG/issues/2563.html.

The words you quoted is the recording of the wg21's discussion during the meeting. And that's the intention of the designers but it is not reflected to the wording. What issue 2563 discussed is that if we should delay the conversion. And the RVO is a conclusion of that. But according to the result, we should say the compiler is allowed to do RVO if the types matched but it is not required.

[bgraur]

What's the resolution here? Can we revert this and continue the discussions independently?
We can always re-land this change if the conclusion is that the approach here is the one that we want.

[ilya-biryukov]

I suspect D145641 won't land today in the EU working hours.
My suggestion would be to revert this patch and reland together with D145641.
This should be in line with @ChuanqiXu's suggestions to wait for a response today.

@bruno, @ChuanqiXu please let us know if you have any objections, otherwise we will land the revert in ~2 hours.

[bruno]

@bruno, @ChuanqiXu please let us know if you have any objections, otherwise we will land the revert in ~2 hours.

No sweat, I didn't see this in time. Thanks for the reduced testcase.

What's the resolution here? Can we revert this and continue the discussions independently?

Eager initialization breaks us, lack of RVO on matching types breaks others. Landing this + D145641 altogether seems like the best approach.

We can always re-land this change if the conclusion is that the approach here is the one that we want.

Once we wrap up D145641 I'll land both, as separated commits but in the same push.

[bruno]

Put dependency in place for D145641

clang/include/clang/AST/StmtCXX.h

enum SubStmt {
     Body,          ///< The body of the coroutine.
     Promise,       ///< The promise statement.
     InitSuspend,   ///< The initial suspend statement, run before the body.
     FinalSuspend,  ///< The final suspend statement, run after the body.
     OnException,   ///< Handler for exceptions thrown in the body.
     OnFallthrough, ///< Handler for control flow falling off the body.
     Allocate,      ///< Coroutine frame memory allocation.
     Deallocate,    ///< Coroutine frame memory deallocation.
+    ResultDecl,    ///< Declaration holding the result of get_return_object.
     ReturnValue,   ///< Return value for thunk function: p.get_return_object().
     ReturnStmt,    ///< Return statement for the thunk function.
     ReturnStmtOnAllocFailure, ///< Return statement if allocation failed.
     FirstParamMove ///< First offset for move construction of parameter copies.
   };
   unsigned NumParams;
 
   friend class ASTStmtReader;
struct CtorArgs {
     Stmt *Body = nullptr;
     Stmt *Promise = nullptr;
     Expr *InitialSuspend = nullptr;
     Expr *FinalSuspend = nullptr;
     Stmt *OnException = nullptr;
     Stmt *OnFallthrough = nullptr;
     Expr *Allocate = nullptr;
     Expr *Deallocate = nullptr;
+    Stmt *ResultDecl = nullptr;
     Expr *ReturnValue = nullptr;
     Stmt *ReturnStmt = nullptr;
     Stmt *ReturnStmtOnAllocFailure = nullptr;
     ArrayRef<Stmt *> ParamMoves;
   };
 
 private:
 
public:
   }
 
   Expr *getAllocate() const {
     return cast_or_null<Expr>(getStoredStmts()[SubStmt::Allocate]);
   }
   Expr *getDeallocate() const {
     return cast_or_null<Expr>(getStoredStmts()[SubStmt::Deallocate]);
   }
+  Stmt *getResultDecl() const { return getStoredStmts()[SubStmt::ResultDecl]; }
   Expr *getReturnValueInit() const {
     return cast<Expr>(getStoredStmts()[SubStmt::ReturnValue]);
   }
   Expr *getReturnValue() const {
     assert(getReturnStmt());
     auto *RS = cast<clang::ReturnStmt>(getReturnStmt());
     return RS->getRetValue();
   }
   Stmt *getReturnStmt() const { return getStoredStmts()[SubStmt::ReturnStmt]; }
   Stmt *getReturnStmtOnAllocFailure() const {
     return getStoredStmts()[SubStmt::ReturnStmtOnAllocFailure];
   }
   ArrayRef<Stmt const *> getParamMoves() const {
     return {getStoredStmts() + SubStmt::FirstParamMove, NumParams};
   }
 

clang/lib/AST/StmtCXX.cpp

CoroutineBodyStmt::CoroutineBodyStmt(CoroutineBodyStmt::CtorArgs const &Args)
   SubStmts[CoroutineBodyStmt::Body] = Args.Body;
   SubStmts[CoroutineBodyStmt::Promise] = Args.Promise;
   SubStmts[CoroutineBodyStmt::InitSuspend] = Args.InitialSuspend;
   SubStmts[CoroutineBodyStmt::FinalSuspend] = Args.FinalSuspend;
   SubStmts[CoroutineBodyStmt::OnException] = Args.OnException;
   SubStmts[CoroutineBodyStmt::OnFallthrough] = Args.OnFallthrough;
   SubStmts[CoroutineBodyStmt::Allocate] = Args.Allocate;
   SubStmts[CoroutineBodyStmt::Deallocate] = Args.Deallocate;
+  SubStmts[CoroutineBodyStmt::ResultDecl] = Args.ResultDecl;
   SubStmts[CoroutineBodyStmt::ReturnValue] = Args.ReturnValue;
   SubStmts[CoroutineBodyStmt::ReturnStmt] = Args.ReturnStmt;
   SubStmts[CoroutineBodyStmt::ReturnStmtOnAllocFailure] =
       Args.ReturnStmtOnAllocFailure;
   std::copy(Args.ParamMoves.begin(), Args.ParamMoves.end(),
             const_cast<Stmt **>(getParamMoves().data()));
 }

clang/lib/CodeGen/CGCoroutine.cpp

void Emit(CodeGenFunction &CGF, Flags) override {
     // No longer need old terminator.
     InsertPt->eraseFromParent();
     CGF.Builder.SetInsertPoint(AfterFreeBB);
   }
   explicit CallCoroDelete(Stmt *DeallocStmt) : Deallocate(DeallocStmt) {}
 };
 }
 
+namespace {
+struct GetReturnObjectManager {
+  CodeGenFunction &CGF;
+  CGBuilderTy &Builder;
+  const CoroutineBodyStmt &S;
+
+  Address GroActiveFlag;
+  CodeGenFunction::AutoVarEmission GroEmission;
+
+  GetReturnObjectManager(CodeGenFunction &CGF, const CoroutineBodyStmt &S)
+      : CGF(CGF), Builder(CGF.Builder), S(S), GroActiveFlag(Address::invalid()),
+        GroEmission(CodeGenFunction::AutoVarEmission::invalid()) {}
+
+  // The gro variable has to outlive coroutine frame and coroutine promise, but,
+  // it can only be initialized after coroutine promise was created, thus, we
+  // split its emission in two parts. EmitGroAlloca emits an alloca and sets up
+  // cleanups. Later when coroutine promise is available we initialize the gro
+  // and sets the flag that the cleanup is now active.
+  void EmitGroAlloca() {
+    auto *GroDeclStmt = dyn_cast<DeclStmt>(S.getResultDecl());
+    if (!GroDeclStmt) {
+      // If get_return_object returns void, no need to do an alloca.
+      return;
+    }
+
+    auto *GroVarDecl = cast<VarDecl>(GroDeclStmt->getSingleDecl());
+
+    // Set GRO flag that it is not initialized yet
+    GroActiveFlag = CGF.CreateTempAlloca(Builder.getInt1Ty(), CharUnits::One(),
+                                         "gro.active");
+    Builder.CreateStore(Builder.getFalse(), GroActiveFlag);
+
+    GroEmission = CGF.EmitAutoVarAlloca(*GroVarDecl);
+
+    // Remember the top of EHStack before emitting the cleanup.
+    auto old_top = CGF.EHStack.stable_begin();
+    CGF.EmitAutoVarCleanups(GroEmission);
+    auto top = CGF.EHStack.stable_begin();
+
+    // Make the cleanup conditional on gro.active
+    for (auto b = CGF.EHStack.find(top), e = CGF.EHStack.find(old_top); b != e;
+         b++) {
+      if (auto *Cleanup = dyn_cast<EHCleanupScope>(&*b)) {
+        assert(!Cleanup->hasActiveFlag() && "cleanup already has active flag?");
+        Cleanup->setActiveFlag(GroActiveFlag);
+        Cleanup->setTestFlagInEHCleanup();
+        Cleanup->setTestFlagInNormalCleanup();
+      }
+    }
+  }
+
+  void EmitGroInit() {
+    if (!GroActiveFlag.isValid()) {
+      // No Gro variable was allocated. Simply emit the call to
+      // get_return_object.
+      CGF.EmitStmt(S.getResultDecl());
+      return;
+    }
+
+    CGF.EmitAutoVarInit(GroEmission);
+    Builder.CreateStore(Builder.getTrue(), GroActiveFlag);
+  }
+};
+} // namespace
+
 static void emitBodyAndFallthrough(CodeGenFunction &CGF,
                                    const CoroutineBodyStmt &S, Stmt *Body) {
   CGF.EmitStmt(Body);
   const bool CanFallthrough = CGF.Builder.GetInsertBlock();
   if (CanFallthrough)
     if (Stmt *OnFallthrough = S.getFallthroughHandler())
       CGF.EmitStmt(OnFallthrough);
 }
void CodeGenFunction::EmitCoroutineBody(const CoroutineBodyStmt &S) {
   // Pass the result of the allocation to coro.begin.
   auto *Phi = Builder.CreatePHI(VoidPtrTy, 2);
   Phi->addIncoming(NullPtr, EntryBB);
   Phi->addIncoming(AllocateCall, AllocOrInvokeContBB);
   auto *CoroBegin = Builder.CreateCall(
       CGM.getIntrinsic(llvm::Intrinsic::coro_begin), {CoroId, Phi});
   CurCoro.Data->CoroBegin = CoroBegin;
 
+  // We need to emit `get_­return_­object` first. According to:
+  // [dcl.fct.def.coroutine]p7
+  // The call to get_­return_­object is sequenced before the call to
+  // initial_­suspend and is invoked at most once.
+  GetReturnObjectManager GroManager(*this, S);
+  GroManager.EmitGroAlloca();
+
   CurCoro.Data->CleanupJD = getJumpDestInCurrentScope(RetBB);
   {
     CGDebugInfo *DI = getDebugInfo();
     ParamReferenceReplacerRAII ParamReplacer(LocalDeclMap);
     CodeGenFunction::RunCleanupsScope ResumeScope(*this);
     EHStack.pushCleanup<CallCoroDelete>(NormalAndEHCleanup, S.getDeallocate());
 
     // Create mapping between parameters and copy-params for coroutine function.
CurCoro.Data->CleanupJD = getJumpDestInCurrentScope(RetBB);
 
     Address PromiseAddr = GetAddrOfLocalVar(S.getPromiseDecl());
     auto *PromiseAddrVoidPtr =
         new llvm::BitCastInst(PromiseAddr.getPointer(), VoidPtrTy, "", CoroId);
     // Update CoroId to refer to the promise. We could not do it earlier because
     // promise local variable was not emitted yet.
     CoroId->setArgOperand(1, PromiseAddrVoidPtr);
 
-    // ReturnValue should be valid as long as the coroutine's return type
-    // is not void. The assertion could help us to reduce the check later.
-    assert(ReturnValue.isValid() == (bool)S.getReturnStmt());
-    // Now we have the promise, initialize the GRO.
-    // We need to emit `get_return_object` first. According to:
-    // [dcl.fct.def.coroutine]p7
-    // The call to get_return_­object is sequenced before the call to
-    // initial_suspend and is invoked at most once.
-    //
-    // So we couldn't emit return value when we emit return statment,
-    // otherwise the call to get_return_object wouldn't be in front
-    // of initial_suspend.
-    if (ReturnValue.isValid()) {
-      EmitAnyExprToMem(S.getReturnValue(), ReturnValue,
-                       S.getReturnValue()->getType().getQualifiers(),
-                       /*IsInit*/ true);
-    }
+    // Now we have the promise, initialize the GRO
+    GroManager.EmitGroInit();
 
     EHStack.pushCleanup<CallCoroEnd>(EHCleanup);
 
     CurCoro.Data->CurrentAwaitKind = AwaitKind::Init;
     CurCoro.Data->ExceptionHandler = S.getExceptionHandler();
     EmitStmt(S.getInitSuspendStmt());
     CurCoro.Data->FinalJD = getJumpDestInCurrentScope(FinalBB);
 
void CodeGenFunction::EmitCoroutineBody(const CoroutineBodyStmt &S) {
   }
 
   EmitBlock(RetBB);
   // Emit coro.end before getReturnStmt (and parameter destructors), since
   // resume and destroy parts of the coroutine should not include them.
   llvm::Function *CoroEnd = CGM.getIntrinsic(llvm::Intrinsic::coro_end);
   Builder.CreateCall(CoroEnd, {NullPtr, Builder.getFalse()});
 
-  if (Stmt *Ret = S.getReturnStmt()) {
-    // Since we already emitted the return value above, so we shouldn't
-    // emit it again here.
-    cast<ReturnStmt>(Ret)->setRetValue(nullptr);
+  if (Stmt *Ret = S.getReturnStmt())
     EmitStmt(Ret);
-  }
 
   // LLVM require the frontend to mark the coroutine.
   CurFn->setPresplitCoroutine();
 }
 
 // Emit coroutine intrinsic and patch up arguments of the token type.
 RValue CodeGenFunction::EmitCoroutineIntrinsic(const CallExpr *E,
                                                unsigned int IID) {

clang/lib/Sema/SemaCoroutine.cpp

assert(!FnRetType->isDependentType() &&
          "get_return_object type must no longer be dependent");
 
   if (FnRetType->isVoidType()) {
     ExprResult Res =
         S.ActOnFinishFullExpr(this->ReturnValue, Loc, /*DiscardedValue*/ false);
     if (Res.isInvalid())
       return false;
 
+    this->ResultDecl = Res.get();
     return true;
   }
 
   if (GroType->isVoidType()) {
     // Trigger a nice error message.
     InitializedEntity Entity =
         InitializedEntity::InitializeResult(Loc, FnRetType);
     S.PerformCopyInitialization(Entity, SourceLocation(), ReturnValue);
     noteMemberDeclaredHere(S, ReturnValue, Fn);
     return false;
   }
 
-  StmtResult ReturnStmt = S.BuildReturnStmt(Loc, ReturnValue);
+  auto *GroDecl = VarDecl::Create(
+      S.Context, &FD, FD.getLocation(), FD.getLocation(),
+      &S.PP.getIdentifierTable().get("__coro_gro"), GroType,
+      S.Context.getTrivialTypeSourceInfo(GroType, Loc), SC_None);
+  GroDecl->setImplicit();
+
+  S.CheckVariableDeclarationType(GroDecl);
+  if (GroDecl->isInvalidDecl())
+    return false;
+
+  InitializedEntity Entity = InitializedEntity::InitializeVariable(GroDecl);
+  ExprResult Res =
+      S.PerformCopyInitialization(Entity, SourceLocation(), ReturnValue);
+  if (Res.isInvalid())
+    return false;
+
+  Res = S.ActOnFinishFullExpr(Res.get(), /*DiscardedValue*/ false);
+  if (Res.isInvalid())
+    return false;
+
+  S.AddInitializerToDecl(GroDecl, Res.get(),
+                         /*DirectInit=*/false);
+
+  S.FinalizeDeclaration(GroDecl);
+
+  // Form a declaration statement for the return declaration, so that AST
+  // visitors can more easily find it.
+  StmtResult GroDeclStmt =
+      S.ActOnDeclStmt(S.ConvertDeclToDeclGroup(GroDecl), Loc, Loc);
+  if (GroDeclStmt.isInvalid())
+    return false;
+
+  this->ResultDecl = GroDeclStmt.get();
+
+  ExprResult declRef = S.BuildDeclRefExpr(GroDecl, GroType, VK_LValue, Loc);
+  if (declRef.isInvalid())
+    return false;
+
+  StmtResult ReturnStmt = S.BuildReturnStmt(Loc, declRef.get());
+
   if (ReturnStmt.isInvalid()) {
     noteMemberDeclaredHere(S, ReturnValue, Fn);
     return false;
   }
 
+  if (cast<clang::ReturnStmt>(ReturnStmt.get())->getNRVOCandidate() == GroDecl)
+    GroDecl->setNRVOVariable(true);
+
   this->ReturnStmt = ReturnStmt.get();
   return true;
 }
 
 // Create a static_cast\<T&&>(expr).
 static Expr *castForMoving(Sema &S, Expr *E, QualType T = QualType()) {
   if (T.isNull())
     T = E->getType();

clang/lib/Sema/TreeTransform.h

if (AllocRes.isInvalid())
       return StmtError();
     Builder.Allocate = AllocRes.get();
 
     ExprResult DeallocRes = getDerived().TransformExpr(S->getDeallocate());
     if (DeallocRes.isInvalid())
       return StmtError();
     Builder.Deallocate = DeallocRes.get();
 
+    assert(S->getResultDecl() && "ResultDecl must already be built");
+    StmtResult ResultDecl = getDerived().TransformStmt(S->getResultDecl());
+    if (ResultDecl.isInvalid())
+      return StmtError();
+    Builder.ResultDecl = ResultDecl.get();
+
     if (auto *ReturnStmt = S->getReturnStmt()) {
       StmtResult Res = getDerived().TransformStmt(ReturnStmt);
       if (Res.isInvalid())
         return StmtError();
       Builder.ReturnStmt = Res.get();
     }
   }
 

clang/test/CodeGenCoroutines/coro-gro.cpp

 };
 
 struct Cleanup { ~Cleanup(); };
 void doSomething() noexcept;
 
 // CHECK: define{{.*}} i32 @_Z1fv(
 int f() {
   // CHECK: %[[RetVal:.+]] = alloca i32
+  // CHECK: %[[GroActive:.+]] = alloca i1
 
   // CHECK: %[[Size:.+]] = call i64 @llvm.coro.size.i64()
   // CHECK: call noalias noundef nonnull ptr @_Znwm(i64 noundef %[[Size]])
+  // CHECK: store i1 false, ptr %[[GroActive]]
   // CHECK: call void @_ZNSt16coroutine_traitsIJiEE12promise_typeC1Ev(
-  // CHECK: call void @_ZNSt16coroutine_traitsIJiEE12promise_type17get_return_objectEv(ptr sret(%struct.GroType) align {{[0-9]+}} %[[GRO:.+]],
-  // CHECK: %[[Conv:.+]] = call noundef i32 @_ZN7GroTypecviEv({{.*}}[[GRO]]
-  // CHECK: store i32 %[[Conv]], ptr %[[RetVal]]
+  // CHECK: call void @_ZNSt16coroutine_traitsIJiEE12promise_type17get_return_objectEv(
+  // CHECK: store i1 true, ptr %[[GroActive]]
 
   Cleanup cleanup;
   doSomething();
   co_return;
 
   // CHECK: call void @_Z11doSomethingv(
   // CHECK: call void @_ZNSt16coroutine_traitsIJiEE12promise_type11return_voidEv(
   // CHECK: call void @_ZN7CleanupD1Ev(
 
   // Destroy promise and free the memory.
 
   // CHECK: call void @_ZNSt16coroutine_traitsIJiEE12promise_typeD1Ev(
   // CHECK: %[[Mem:.+]] = call ptr @llvm.coro.free(
   // CHECK: call void @_ZdlPv(ptr noundef %[[Mem]])
 
-  // CHECK: coro.ret:
+  // Initialize retval from Gro and destroy Gro
+
+  // CHECK: %[[Conv:.+]] = call noundef i32 @_ZN7GroTypecviEv(
+  // CHECK: store i32 %[[Conv]], ptr %[[RetVal]]
+  // CHECK: %[[IsActive:.+]] = load i1, ptr %[[GroActive]]
+  // CHECK: br i1 %[[IsActive]], label %[[CleanupGro:.+]], label %[[Done:.+]]
+
+  // CHECK: [[CleanupGro]]:
+  // CHECK:   call void @_ZN7GroTypeD1Ev(
+  // CHECK:   br label %[[Done]]
+
+  // CHECK: [[Done]]:
   // CHECK:   %[[LoadRet:.+]] = load i32, ptr %[[RetVal]]
   // CHECK:   ret i32 %[[LoadRet]]
 }

clang/test/CodeGenCoroutines/pr59221.cpp

 // Test for PR59221. Tests the compiler wouldn't misoptimize the final result.
 //
 // REQUIRES: x86-registered-target
 //
-// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -std=c++20 %s -O3 -S -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -std=c++20 %s -O1 -S -emit-llvm -o - | FileCheck %s
 
 #include "Inputs/coroutine.h"
 
 template <typename T> struct task {
   struct promise_type {
     T value{123};
     std::coroutine_handle<> caller{std::noop_coroutine()};
     

clang/test/SemaCXX/coroutine-no-move-ctor.cpp

public:
     invoker get_return_object() { return invoker{}; }
     auto initial_suspend() { return std::suspend_never{}; }
     auto final_suspend() noexcept { return std::suspend_never{}; }
     void return_void() {}
     void unhandled_exception() {}
   };
   using promise_type = invoker_promise;
   invoker() {}
-  invoker(const invoker &) = delete;
-  invoker &operator=(const invoker &) = delete;
-  invoker(invoker &&) = delete;
-  invoker &operator=(invoker &&) = delete;
+  // TODO: implement RVO for get_return_object type matching
+  // function return type.
+  //
+  // invoker(const invoker &) = delete;
+  // invoker &operator=(const invoker &) = delete;
+  // invoker(invoker &&) = delete;
+  // invoker &operator=(invoker &&) = delete;
 };
 
 invoker f() {
   co_return;
 }

clang/test/SemaCXX/coroutines.cpp

struct promise_type {
     suspend_always final_suspend() noexcept { return {}; }
     void return_void() {}
     void unhandled_exception();
   };
 };
 
 extern "C" int f(mismatch_gro_type_tag2) {
   // cxx2b-error@-1 {{cannot initialize return object of type 'int' with an rvalue of type 'void *'}}
-  // cxx14_20-error@-2 {{cannot initialize return object of type 'int' with an rvalue of type 'void *'}}
+  // cxx14_20-error@-2 {{cannot initialize return object of type 'int' with an lvalue of type 'void *'}}
   co_return; //expected-note {{function is a coroutine due to use of 'co_return' here}}
 }
 
 struct mismatch_gro_type_tag3 {};
 template <>
 struct std::coroutine_traits<int, mismatch_gro_type_tag3> {
   struct promise_type {
     int get_return_object() {}

clang/test/SemaCXX/warn-throw-out-noexcept-coro.cpp

 // RUN: %clang_cc1 -std=c++20 %s -fcxx-exceptions -fsyntax-only -Wexceptions -verify -fdeclspec
 
 #include "Inputs/std-coroutine.h"
 
 // expected-no-diagnostics
 
 template <typename T>
 struct promise;
 
 template <typename T>
 struct task {
     using promise_type = promise<T>;
 
     explicit task(promise_type& p) { throw 1; p.return_val = this; }
 
-    task(const task&) = delete;
-
     T value;
 };
 
 template <typename T>
 struct promise {
     task<T> get_return_object() { return task{*this}; }
 
     std::suspend_never initial_suspend() const noexcept { return {}; }