diff --git a/clang/lib/CodeGen/CGCoroutine.cpp b/clang/lib/CodeGen/CGCoroutine.cpp index b76450152203d..569070123e368 100644 --- a/clang/lib/CodeGen/CGCoroutine.cpp +++ b/clang/lib/CodeGen/CGCoroutine.cpp @@ -53,6 +53,10 @@ struct clang::CodeGen::CGCoroData { // handler, this is null. llvm::Value *ResumeEHVar = nullptr; + // Stores the i1 that identify if control flows off the end of the coroutine + // body + llvm::PHINode *BodyDone = nullptr; + // Stores the jump destination just before the coroutine memory is freed. // This is the destination that every suspend point jumps to for the cleanup // branch. @@ -599,14 +603,18 @@ namespace { struct CallCoroDelete final : public EHScopeStack::Cleanup { Stmt *Deallocate; - // Emit "if (coro.free(CoroId, CoroBegin)) Deallocate;" + void Emit(CodeGenFunction &CGF, Flags F) override { + // Cleanup always happens on unwind. No need for pre-cleanup check. + if (F.isForNormalCleanup()) + EmitPreCleanup(CGF); + // Emit "if (coro.free(CoroId, CoroBegin)) Deallocate;" + + // Note: That deallocation will be emitted twice: once for a normal exit and + // once for exceptional exit. This usage is safe because Deallocate does not + // contain any declarations. The SubStmtBuilder::makeNewAndDeleteExpr() + // builds a single call to a deallocation function which is safe to emit + // multiple times. - // Note: That deallocation will be emitted twice: once for a normal exit and - // once for exceptional exit. This usage is safe because Deallocate does not - // contain any declarations. The SubStmtBuilder::makeNewAndDeleteExpr() - // builds a single call to a deallocation function which is safe to emit - // multiple times. - void Emit(CodeGenFunction &CGF, Flags) override { // Remember the current point, as we are going to emit deallocation code // first to get to coro.free instruction that is an argument to a delete // call. @@ -642,6 +650,24 @@ struct CallCoroDelete final : public EHScopeStack::Cleanup { CGF.Builder.SetInsertPoint(AfterFreeBB); } explicit CallCoroDelete(Stmt *DeallocStmt) : Deallocate(DeallocStmt) {} + +private: + void EmitPreCleanup(CodeGenFunction &CGF) { + auto &Builder = CGF.Builder; + auto &Data = *CGF.CurCoro.Data; + auto *BodyDone = Data.BodyDone; + BasicBlock *SaveInsertBlock = Builder.GetInsertBlock(); + BasicBlock *PreConvBB = BodyDone->getParent(); + BasicBlock *AfterConvBB = + cast(PreConvBB->getTerminator())->getSuccessor(1); + BasicBlock *CleanupBB = AfterConvBB->getSingleSuccessor(); + BasicBlock *RetBB = Data.CleanupJD.getBlock(); + + AfterConvBB->getTerminator()->eraseFromParent(); + Builder.SetInsertPoint(AfterConvBB); + Builder.CreateCondBr(BodyDone, CleanupBB, RetBB); + Builder.SetInsertPoint(SaveInsertBlock); + } }; } @@ -687,11 +713,11 @@ struct GetReturnObjectManager { } // 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() { + // it can only be initialized after coroutine promise was created. Thus, + // EmitGroActive emits a flag and sets it to false. Later when coroutine + // promise is available we initialize the gro and set the flag indicating that + // the cleanup is now active. + void EmitGroActive() { if (DirectEmit) return; @@ -701,12 +727,23 @@ struct GetReturnObjectManager { 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); + } + + void EmitGroAlloca() { + if (DirectEmit) + return; + + auto *GroDeclStmt = dyn_cast_or_null(S.getResultDecl()); + if (!GroDeclStmt) { + // If get_return_object returns void, no need to do an alloca. + return; + } + + auto *GroVarDecl = cast(GroDeclStmt->getSingleDecl()); GroEmission = CGF.EmitAutoVarAlloca(*GroVarDecl); @@ -768,6 +805,39 @@ struct GetReturnObjectManager { CGF.EmitAutoVarInit(GroEmission); Builder.CreateStore(Builder.getTrue(), GroActiveFlag); } + + void EmitGroConv() { + auto *InsertPt = Builder.GetInsertBlock(); + auto *PreConvBB = CGF.CurCoro.Data->SuspendBB; + auto *AfterConvBB = + CGF.createBasicBlock("after.gro.conv", CGF.CurFn, InsertPt); + Builder.SetInsertPoint(AfterConvBB); + BasicBlock *AfterFinalBB = nullptr; + if (InsertPt) { + AfterFinalBB = InsertPt->getSinglePredecessor(); + InsertPt->replaceAllUsesWith(PreConvBB); + Builder.CreateBr(InsertPt); + } + + auto *ConvBB = CGF.createBasicBlock("gro.conv", CGF.CurFn, AfterConvBB); + Builder.SetInsertPoint(ConvBB); + Builder.CreateBr(AfterConvBB); + + CGF.EmitBlock(PreConvBB); + PreConvBB->moveBefore(ConvBB); + auto *BodyDone = + Builder.CreatePHI(Builder.getInt1Ty(), llvm::pred_size(PreConvBB)); + for (auto *Pred : llvm::predecessors(PreConvBB)) { + auto *V = (Pred == AfterFinalBB) ? Builder.getTrue() : Builder.getFalse(); + BodyDone->addIncoming(V, Pred); + } + CGF.CurCoro.Data->BodyDone = BodyDone; + auto *InRampFn = CGF.CGM.getIntrinsic(llvm::Intrinsic::coro_is_in_ramp); + auto *InRamp = Builder.CreateCall(InRampFn, {}, "InRamp"); + Builder.CreateCondBr(InRamp, ConvBB, AfterConvBB); + + Builder.SetInsertPoint(InsertPt == nullptr ? AfterConvBB : InsertPt); + } }; } // namespace @@ -790,12 +860,13 @@ void CodeGenFunction::EmitCoroutineBody(const CoroutineBodyStmt &S) { auto *InitBB = createBasicBlock("coro.init"); auto *FinalBB = createBasicBlock("coro.final"); auto *RetBB = createBasicBlock("coro.ret"); + auto *PreConvBB = createBasicBlock("pre.gvo.conv"); auto *CoroId = Builder.CreateCall( CGM.getIntrinsic(llvm::Intrinsic::coro_id), {Builder.getInt32(NewAlign), NullPtr, NullPtr, NullPtr}); createCoroData(*this, CurCoro, CoroId); - CurCoro.Data->SuspendBB = RetBB; + CurCoro.Data->SuspendBB = PreConvBB; assert(ShouldEmitLifetimeMarkers && "Must emit lifetime intrinsics for coroutines"); @@ -840,7 +911,6 @@ void CodeGenFunction::EmitCoroutineBody(const CoroutineBodyStmt &S) { CurCoro.Data->CoroBegin = CoroBegin; GetReturnObjectManager GroManager(*this, S); - GroManager.EmitGroAlloca(); CurCoro.Data->CleanupJD = getJumpDestInCurrentScope(RetBB); { @@ -884,6 +954,7 @@ void CodeGenFunction::EmitCoroutineBody(const CoroutineBodyStmt &S) { // not needed. } + GroManager.EmitGroActive(); EmitStmt(S.getPromiseDeclStmt()); Address PromiseAddr = GetAddrOfLocalVar(S.getPromiseDecl()); @@ -895,6 +966,7 @@ void CodeGenFunction::EmitCoroutineBody(const CoroutineBodyStmt &S) { CoroId->setArgOperand(1, PromiseAddrVoidPtr); // Now we have the promise, initialize the GRO + GroManager.EmitGroAlloca(); GroManager.EmitGroInit(); EHStack.pushCleanup(EHCleanup); @@ -950,11 +1022,12 @@ void CodeGenFunction::EmitCoroutineBody(const CoroutineBodyStmt &S) { // We don't need FinalBB. Emit it to make sure the block is deleted. EmitBlock(FinalBB, /*IsFinished=*/true); } + GroManager.EmitGroConv(); } EmitBlock(RetBB); - // Emit coro.end before getReturnStmt (and parameter destructors), since - // resume and destroy parts of the coroutine should not include them. + // Emit coro.end before ret instruction, since resume and destroy parts of the + // coroutine should return void. llvm::Function *CoroEnd = CGM.getIntrinsic(llvm::Intrinsic::coro_end); Builder.CreateCall(CoroEnd, {NullPtr, Builder.getFalse(), @@ -973,8 +1046,13 @@ void CodeGenFunction::EmitCoroutineBody(const CoroutineBodyStmt &S) { // shouldn't change the AST. if (PreviousRetValue) cast(Ret)->setRetValue(PreviousRetValue); + // Send GRO conversion to ConvBB + auto *ConvBB = + cast(PreConvBB->getTerminator())->getSuccessor(0); + auto FromIt = ++RetBB->getFirstInsertionPt(); + auto ToIt = RetBB->getTerminator()->getIterator(); + ConvBB->splice(ConvBB->getFirstNonPHIIt(), RetBB, FromIt, ToIt); } - // LLVM require the frontend to mark the coroutine. CurFn->setPresplitCoroutine(); diff --git a/clang/test/CodeGenCoroutines/coro-dest-slot.cpp b/clang/test/CodeGenCoroutines/coro-dest-slot.cpp index d794c74cd52d6..a770da4d7e321 100644 --- a/clang/test/CodeGenCoroutines/coro-dest-slot.cpp +++ b/clang/test/CodeGenCoroutines/coro-dest-slot.cpp @@ -25,7 +25,7 @@ extern "C" coro f(int) { co_return; } // CHECK: %[[CLEANUP_DEST0:.+]] = phi i32 [ 0, %[[INIT_READY]] ], [ 2, %[[INIT_CLEANUP]] ] // CHECK: %[[FINAL_SUSPEND:.+]] = call i8 @llvm.coro.suspend( -// CHECK-NEXT: switch i8 %{{.*}}, label %coro.ret [ +// CHECK-NEXT: switch i8 %{{.*}}, label %pre.gvo.conv [ // CHECK-NEXT: i8 0, label %[[FINAL_READY:.+]] // CHECK-NEXT: i8 1, label %[[FINAL_CLEANUP:.+]] // CHECK-NEXT: ] diff --git a/clang/test/CodeGenCoroutines/coro-gro.cpp b/clang/test/CodeGenCoroutines/coro-gro.cpp index 037fd03349e76..fb9d0a6d85377 100644 --- a/clang/test/CodeGenCoroutines/coro-gro.cpp +++ b/clang/test/CodeGenCoroutines/coro-gro.cpp @@ -29,47 +29,72 @@ void doSomething() noexcept; // CHECK: define{{.*}} i32 @_Z1fv( int f() { // CHECK: %[[RetVal:.+]] = alloca i32 - // CHECK: %[[GroActive:.+]] = alloca i1 - // CHECK: %[[CoroGro:.+]] = alloca %struct.GroType, {{.*}} !coro.outside.frame ![[OutFrameMetadata:.+]] + // CHECK-NEXT: %[[GroActive:.+]] = alloca i1 + // CHECK-NEXT: %[[Promise:.+]] = alloca %"struct.std::coroutine_traits::promise_type", align 1 + // CHECK-NEXT: %[[CoroGro:.+]] = alloca %struct.GroType, {{.*}} !coro.outside.frame ![[OutFrameMetadata:.+]] // CHECK: %[[Size:.+]] = call i64 @llvm.coro.size.i64() - // CHECK: call noalias noundef nonnull ptr @_Znwm(i64 noundef %[[Size]]) + // CHECK-NEXT: call noalias noundef nonnull ptr @_Znwm(i64 noundef %[[Size]]) + // CHECK: store i1 false, ptr %[[GroActive]] - // CHECK: call void @_ZNSt16coroutine_traitsIiJEE12promise_typeC1Ev( - // CHECK: call void @_ZNSt16coroutine_traitsIiJEE12promise_type17get_return_objectEv({{.*}} %[[CoroGro]] - // CHECK: store i1 true, ptr %[[GroActive]] + // CHECK-NEXT: call void @llvm.lifetime.start.p0(ptr %[[Promise]]) + // CHECK-NEXT: call void @_ZNSt16coroutine_traitsIiJEE12promise_typeC1Ev( + // CHECK-NEXT: call void @llvm.lifetime.start.p0(ptr %[[CoroGro]]) + // CHECK-NEXT: call void @_ZNSt16coroutine_traitsIiJEE12promise_type17get_return_objectEv({{.*}} %[[CoroGro]] + // CHECK-NEXT: store i1 true, ptr %[[GroActive]] Cleanup cleanup; doSomething(); co_return; // CHECK: call void @_Z11doSomethingv( - // CHECK: call void @_ZNSt16coroutine_traitsIiJEE12promise_type11return_voidEv( - // CHECK: call void @_ZN7CleanupD1Ev( - - // Destroy promise and free the memory. - - // CHECK: call void @_ZNSt16coroutine_traitsIiJEE12promise_typeD1Ev( - // CHECK: %[[Mem:.+]] = call ptr @llvm.coro.free( - // CHECK: %[[SIZE:.+]] = call i64 @llvm.coro.size.i64() - // CHECK: call void @_ZdlPvm(ptr noundef %[[Mem]], i64 noundef %[[SIZE]]) + // CHECK-NEXT: call void @_ZNSt16coroutine_traitsIiJEE12promise_type11return_voidEv( + // CHECK-NEXT: call void @_ZN7CleanupD1Ev( // Initialize retval from Gro and destroy Gro // Note this also tests delaying initialization when Gro and function return // types mismatch (see cwg2563). - // 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: pre.gvo.conv: + // CHECK-NEXT: %10 = phi i1 [ true, %cleanup8 ], [ false, %final.suspend ], [ false, %init.suspend ] + // CHECK-NEXT: %InRamp = call i1 @llvm.coro.is_in_ramp() + // CHECK-NEXT: br i1 %InRamp, label %[[GroConv:.+]], label %[[AfterGroConv:.+]] + + // CHECK: [[GroConv]]: + // CHECK-NEXT: %[[Conv:.+]] = call noundef i32 @_ZN7GroTypecviEv( + // CHECK-NEXT: store i32 %[[Conv]], ptr %[[RetVal]] + // CHECK-NEXT: br label %[[AfterGroConv]] + + // CHECK: [[AfterGroConv]]: + // CHECK-NEXT: br i1 %10, label %cleanup.cont10, label %[[CoroRet:.+]] + + // CHECK: cleanup.cont10: + // CHECK-NEXT: br label %[[Cleanup:.+]] + + // CHECK: [[Cleanup]]: + // CHECK-NEXT: %{{.*}} = phi i32 + // CHECK-NEXT: %[[IsActive:.+]] = load i1, ptr %[[GroActive]] + // CHECK-NEXT: br i1 %[[IsActive]], label %[[CleanupGro:.+]], label %[[Done:.+]] // CHECK: [[CleanupGro]]: - // CHECK: call void @_ZN7GroTypeD1Ev( - // CHECK: br label %[[Done]] + // CHECK-NEXT: call void @_ZN7GroTypeD1Ev( + // CHECK-NEXT: br label %[[Done]] + + // Destroy promise and free the memory. // CHECK: [[Done]]: - // CHECK: %[[LoadRet:.+]] = load i32, ptr %[[RetVal]] - // CHECK: ret i32 %[[LoadRet]] + // CHECK-NEXT: call void @llvm.lifetime.end.p0(ptr %[[CoroGro]]) + // CHECK-NEXT: call void @_ZNSt16coroutine_traitsIiJEE12promise_typeD1Ev( + // CHECK-NEXT: call void @llvm.lifetime.end.p0(ptr %[[Promise]]) + // CHECK-NEXT: %[[Mem:.+]] = call ptr @llvm.coro.free( + + // CHECK: %[[SIZE:.+]] = call i64 @llvm.coro.size.i64() + // CHECK-NEXT: call void @_ZdlPvm(ptr noundef %[[Mem]], i64 noundef %[[SIZE]]) + + // CHECK: [[CoroRet]]: + // CHECK-NEXT: call void @llvm.coro.end( + // CHECK-NEXT: %[[LoadRet:.+]] = load i32, ptr %[[RetVal]] + // CHECK-NEXT: ret i32 %[[LoadRet]] } class invoker { diff --git a/clang/test/CodeGenCoroutines/coro-symmetric-transfer-02.cpp b/clang/test/CodeGenCoroutines/coro-symmetric-transfer-02.cpp index f36f89926505f..bfed718016b83 100644 --- a/clang/test/CodeGenCoroutines/coro-symmetric-transfer-02.cpp +++ b/clang/test/CodeGenCoroutines/coro-symmetric-transfer-02.cpp @@ -91,7 +91,7 @@ Task bar() { // CHECK-NEXT: %{{.+}} = call token @llvm.coro.save(ptr null) // CHECK-NEXT: call void @llvm.coro.await.suspend.handle // CHECK-NEXT: %{{.+}} = call i8 @llvm.coro.suspend -// CHECK-NEXT: switch i8 %{{.+}}, label %coro.ret [ +// CHECK-NEXT: switch i8 %{{.+}}, label %pre.gvo.conv [ // CHECK-NEXT: i8 0, label %[[CASE1_AWAIT_READY]] // CHECK-NEXT: i8 1, label %[[CASE1_AWAIT_CLEANUP:.+]] // CHECK-NEXT: ] @@ -105,7 +105,7 @@ Task bar() { // CHECK-NEXT: %{{.+}} = call token @llvm.coro.save(ptr null) // CHECK-NEXT: call void @llvm.coro.await.suspend.handle // CHECK-NEXT: %{{.+}} = call i8 @llvm.coro.suspend -// CHECK-NEXT: switch i8 %{{.+}}, label %coro.ret [ +// CHECK-NEXT: switch i8 %{{.+}}, label %pre.gvo.conv [ // CHECK-NEXT: i8 0, label %[[CASE2_AWAIT_READY]] // CHECK-NEXT: i8 1, label %[[CASE2_AWAIT_CLEANUP:.+]] // CHECK-NEXT: ]