diff --git a/clang/include/clang/AST/StmtCXX.h b/clang/include/clang/AST/StmtCXX.h --- a/clang/include/clang/AST/StmtCXX.h +++ b/clang/include/clang/AST/StmtCXX.h @@ -326,6 +326,7 @@ 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. @@ -352,6 +353,7 @@ Stmt *OnFallthrough = nullptr; Expr *Allocate = nullptr; Expr *Deallocate = nullptr; + Stmt *ResultDecl = nullptr; Expr *ReturnValue = nullptr; Stmt *ReturnStmt = nullptr; Stmt *ReturnStmtOnAllocFailure = nullptr; @@ -404,6 +406,7 @@ Expr *getDeallocate() const { return cast_or_null(getStoredStmts()[SubStmt::Deallocate]); } + Stmt *getResultDecl() const { return getStoredStmts()[SubStmt::ResultDecl]; } Expr *getReturnValueInit() const { return cast(getStoredStmts()[SubStmt::ReturnValue]); } diff --git a/clang/lib/AST/StmtCXX.cpp b/clang/lib/AST/StmtCXX.cpp --- a/clang/lib/AST/StmtCXX.cpp +++ b/clang/lib/AST/StmtCXX.cpp @@ -117,6 +117,7 @@ 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 --- a/clang/lib/CodeGen/CGCoroutine.cpp +++ b/clang/lib/CodeGen/CGCoroutine.cpp @@ -467,6 +467,71 @@ }; } +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(S.getResultDecl()); + if (!GroDeclStmt) { + // If get_return_object returns void, no need to do an alloca. + return; + } + + auto *GroVarDecl = cast(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(&*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); @@ -533,6 +598,13 @@ 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(); @@ -570,23 +642,8 @@ // 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(EHCleanup); @@ -649,12 +706,8 @@ 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(Ret)->setRetValue(nullptr); + if (Stmt *Ret = S.getReturnStmt()) 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 --- a/clang/lib/Sema/SemaCoroutine.cpp +++ b/clang/lib/Sema/SemaCoroutine.cpp @@ -1736,6 +1736,7 @@ if (Res.isInvalid()) return false; + this->ResultDecl = Res.get(); return true; } @@ -1748,12 +1749,55 @@ return false; } - StmtResult ReturnStmt = S.BuildReturnStmt(Loc, ReturnValue); + // 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(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 --- a/clang/lib/Sema/TreeTransform.h +++ b/clang/lib/Sema/TreeTransform.h @@ -8051,6 +8051,12 @@ 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 --- a/clang/test/CodeGenCoroutines/coro-gro.cpp +++ b/clang/test/CodeGenCoroutines/coro-gro.cpp @@ -46,13 +46,14 @@ // 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(); @@ -68,7 +69,18 @@ // 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]] } diff --git a/clang/test/CodeGenCoroutines/pr59221.cpp b/clang/test/CodeGenCoroutines/pr59221.cpp --- 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 -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" diff --git a/clang/test/SemaCXX/coroutine-no-move-ctor.cpp b/clang/test/SemaCXX/coroutine-no-move-ctor.cpp --- a/clang/test/SemaCXX/coroutine-no-move-ctor.cpp +++ b/clang/test/SemaCXX/coroutine-no-move-ctor.cpp @@ -15,10 +15,13 @@ }; 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() { diff --git a/clang/test/SemaCXX/coroutines.cpp b/clang/test/SemaCXX/coroutines.cpp --- a/clang/test/SemaCXX/coroutines.cpp +++ b/clang/test/SemaCXX/coroutines.cpp @@ -934,7 +934,7 @@ 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}} } diff --git a/clang/test/SemaCXX/warn-throw-out-noexcept-coro.cpp b/clang/test/SemaCXX/warn-throw-out-noexcept-coro.cpp --- a/clang/test/SemaCXX/warn-throw-out-noexcept-coro.cpp +++ b/clang/test/SemaCXX/warn-throw-out-noexcept-coro.cpp @@ -13,8 +13,6 @@ explicit task(promise_type& p) { throw 1; p.return_val = this; } - task(const task&) = delete; - T value; };