Skip to content

[clang][CodeGen] Promote point of GRO(CWG2563) #151067

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

[clang][CodeGen] Promote point of GRO(CWG2563) #151067

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
335a188
Promote point of GRO
NewSigma Oct 30, 2025
c515650
Simplify pre-cleanup check
NewSigma Oct 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
116 changes: 97 additions & 19 deletions clang/lib/CodeGen/CGCoroutine.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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<llvm::BranchInst>(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);
}
};
}

Expand Down Expand Up @@ -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;

Expand All @@ -701,12 +727,23 @@ struct GetReturnObjectManager {
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);
}

void EmitGroAlloca() {
if (DirectEmit)
return;

auto *GroDeclStmt = dyn_cast_or_null<DeclStmt>(S.getResultDecl());
if (!GroDeclStmt) {
// If get_return_object returns void, no need to do an alloca.
return;
}

auto *GroVarDecl = cast<VarDecl>(GroDeclStmt->getSingleDecl());

GroEmission = CGF.EmitAutoVarAlloca(*GroVarDecl);

Expand Down Expand Up @@ -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

Expand All @@ -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");

Expand Down Expand Up @@ -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);
{
Expand Down Expand Up @@ -884,6 +954,7 @@ void CodeGenFunction::EmitCoroutineBody(const CoroutineBodyStmt &S) {
// not needed.
}

GroManager.EmitGroActive();
EmitStmt(S.getPromiseDeclStmt());

Address PromiseAddr = GetAddrOfLocalVar(S.getPromiseDecl());
Expand All @@ -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<CallCoroEnd>(EHCleanup);
Expand Down Expand Up @@ -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(),
Expand All @@ -973,8 +1046,13 @@ void CodeGenFunction::EmitCoroutineBody(const CoroutineBodyStmt &S) {
// shouldn't change the AST.
if (PreviousRetValue)
cast<ReturnStmt>(Ret)->setRetValue(PreviousRetValue);
// Send GRO conversion to ConvBB
auto *ConvBB =
cast<llvm::BranchInst>(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();

Expand Down
2 changes: 1 addition & 1 deletion clang/test/CodeGenCoroutines/coro-dest-slot.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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: ]
Expand Down
71 changes: 48 additions & 23 deletions clang/test/CodeGenCoroutines/coro-gro.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<int>::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 {
Expand Down
4 changes: 2 additions & 2 deletions clang/test/CodeGenCoroutines/coro-symmetric-transfer-02.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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: ]
Expand All @@ -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: ]
Expand Down
Loading