Revert "[Coroutines] Fix premature conversion of return object"

This reverts commit 54225c457a336b1609c6d064b2b606a9238a28b9.
The lack of RVO causes compile errors in our code.
Reverting to unblock our integrate.

See D145639 for full discussion.

diff --git a/clang/include/clang/AST/StmtCXX.h b/clang/include/clang/AST/StmtCXX.h
index 05dfac2..2c71f86 100644 (file)
--- a/clang/include/clang/AST/StmtCXX.h
+++ b/clang/include/clang/AST/StmtCXX.h
@@ -326,7 +326,6 @@ class CoroutineBodyStmt final
     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.
@@ -353,7 +352,6 @@ public:
     Stmt *OnFallthrough = nullptr;
     Expr *Allocate = nullptr;
     Expr *Deallocate = nullptr;
-    Stmt *ResultDecl = nullptr;
     Expr *ReturnValue = nullptr;
     Stmt *ReturnStmt = nullptr;
     Stmt *ReturnStmtOnAllocFailure = nullptr;
@@ -406,7 +404,6 @@ public:
   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]);
   }

diff --git a/clang/lib/AST/StmtCXX.cpp b/clang/lib/AST/StmtCXX.cpp
index a3ae539..33b0421 100644 (file)
--- a/clang/lib/AST/StmtCXX.cpp
+++ b/clang/lib/AST/StmtCXX.cpp
@@ -117,7 +117,6 @@ CoroutineBodyStmt::CoroutineBodyStmt(CoroutineBodyStmt::CtorArgs const &Args)
   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] =

diff --git a/clang/lib/CodeGen/CGCoroutine.cpp b/clang/lib/CodeGen/CGCoroutine.cpp
index 38167cc..9b233c1 100644 (file)
--- a/clang/lib/CodeGen/CGCoroutine.cpp
+++ b/clang/lib/CodeGen/CGCoroutine.cpp
@@ -467,71 +467,6 @@ struct CallCoroDelete final : public EHScopeStack::Cleanup {
 };
 }
 
-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);
@@ -598,13 +533,6 @@ void CodeGenFunction::EmitCoroutineBody(const CoroutineBodyStmt &S) {
       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();
@@ -642,8 +570,23 @@ void CodeGenFunction::EmitCoroutineBody(const CoroutineBodyStmt &S) {
     // promise local variable was not emitted yet.
     CoroId->setArgOperand(1, PromiseAddrVoidPtr);
 
-    // Now we have the promise, initialize the GRO
-    GroManager.EmitGroInit();
+    // 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);
+    }
 
     EHStack.pushCleanup<CallCoroEnd>(EHCleanup);
 
@@ -706,8 +649,12 @@ void CodeGenFunction::EmitCoroutineBody(const CoroutineBodyStmt &S) {
   llvm::Function *CoroEnd = CGM.getIntrinsic(llvm::Intrinsic::coro_end);
   Builder.CreateCall(CoroEnd, {NullPtr, Builder.getFalse()});
 
-  if (Stmt *Ret = S.getReturnStmt())
+  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);
     EmitStmt(Ret);
+  }
 
   // LLVM require the frontend to mark the coroutine.
   CurFn->setPresplitCoroutine();

diff --git a/clang/lib/Sema/SemaCoroutine.cpp b/clang/lib/Sema/SemaCoroutine.cpp
index 3df2af2..0dcfbd5 100644 (file)
--- a/clang/lib/Sema/SemaCoroutine.cpp
+++ b/clang/lib/Sema/SemaCoroutine.cpp
@@ -1736,7 +1736,6 @@ bool CoroutineStmtBuilder::makeGroDeclAndReturnStmt() {
     if (Res.isInvalid())
       return false;
 
-    this->ResultDecl = Res.get();
     return true;
   }
 
@@ -1749,55 +1748,12 @@ bool CoroutineStmtBuilder::makeGroDeclAndReturnStmt() {
     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());
-
+  StmtResult ReturnStmt = S.BuildReturnStmt(Loc, ReturnValue);
   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;
 }

diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index e9b35f6..21789f9 100644 (file)
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -8066,12 +8066,6 @@ TreeTransform<Derived>::TransformCoroutineBodyStmt(CoroutineBodyStmt *S) {
       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())

diff --git a/clang/test/CodeGenCoroutines/coro-gro.cpp b/clang/test/CodeGenCoroutines/coro-gro.cpp
index ddcf112..fad75c9 100644 (file)
--- a/clang/test/CodeGenCoroutines/coro-gro.cpp
+++ b/clang/test/CodeGenCoroutines/coro-gro.cpp
@@ -46,14 +46,13 @@ 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(
-  // CHECK: store i1 true, ptr %[[GroActive]]
+  // 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]]
 
   Cleanup cleanup;
   doSomething();
@@ -69,18 +68,7 @@ int f() {
   // CHECK: %[[Mem:.+]] = call ptr @llvm.coro.free(
   // CHECK: call void @_ZdlPv(ptr noundef %[[Mem]])
 
-  // 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: coro.ret:
   // CHECK:   %[[LoadRet:.+]] = load i32, ptr %[[RetVal]]
   // CHECK:   ret i32 %[[LoadRet]]
 }

diff --git a/clang/test/CodeGenCoroutines/pr59221.cpp b/clang/test/CodeGenCoroutines/pr59221.cpp
index e0e3de5..ae5f6fd 100644 (file)
--- a/clang/test/CodeGenCoroutines/pr59221.cpp
+++ b/clang/test/CodeGenCoroutines/pr59221.cpp
@@ -2,7 +2,7 @@
 //
 // REQUIRES: x86-registered-target
 //
-// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -std=c++20 %s -O1 -S -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -std=c++20 %s -O3 -S -emit-llvm -o - | FileCheck %s
 
 #include "Inputs/coroutine.h"
 

diff --git a/clang/test/SemaCXX/coroutine-no-move-ctor.cpp b/clang/test/SemaCXX/coroutine-no-move-ctor.cpp
index 08933f4..824dea3 100644 (file)
--- a/clang/test/SemaCXX/coroutine-no-move-ctor.cpp
+++ b/clang/test/SemaCXX/coroutine-no-move-ctor.cpp
@@ -15,13 +15,10 @@ public:
   };
   using promise_type = invoker_promise;
   invoker() {}
-  // 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(const invoker &) = delete;
+  invoker &operator=(const invoker &) = delete;
+  invoker(invoker &&) = delete;
+  invoker &operator=(invoker &&) = delete;
 };
 
 invoker f() {

diff --git a/clang/test/SemaCXX/coroutines.cpp b/clang/test/SemaCXX/coroutines.cpp
index 782f4b2..e480c0d 100644 (file)
--- a/clang/test/SemaCXX/coroutines.cpp
+++ b/clang/test/SemaCXX/coroutines.cpp
@@ -934,7 +934,7 @@ struct std::coroutine_traits<int, mismatch_gro_type_tag2> {
 
 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 lvalue of type 'void *'}}
+  // cxx14_20-error@-2 {{cannot initialize return object of type 'int' with an rvalue of type 'void *'}}
   co_return; //expected-note {{function is a coroutine due to use of 'co_return' here}}
 }
 

diff --git a/clang/test/SemaCXX/warn-throw-out-noexcept-coro.cpp b/clang/test/SemaCXX/warn-throw-out-noexcept-coro.cpp
index e96aae4..4d52bdc 100644 (file)
--- a/clang/test/SemaCXX/warn-throw-out-noexcept-coro.cpp
+++ b/clang/test/SemaCXX/warn-throw-out-noexcept-coro.cpp
@@ -13,6 +13,8 @@ struct task {
 
     explicit task(promise_type& p) { throw 1; p.return_val = this; }
 
+    task(const task&) = delete;
+
     T value;
 };