[clang] [clang][CodeGen] Promote point of GRO(CWG2563) (PR #151067)
Weibo He via cfe-commits
cfe-commits at lists.llvm.org
Sun Dec 14 18:21:05 PST 2025
https://github.com/NewSigma updated https://github.com/llvm/llvm-project/pull/151067
>From 01176d70b52235cc3ceb00bc33a285bcbd11bc58 Mon Sep 17 00:00:00 2001
From: NewSigma <NewSigma at 163.com>
Date: Sat, 13 Dec 2025 20:57:43 +0800
Subject: [PATCH] Resolve review comments
---
clang/lib/CodeGen/CGCoroutine.cpp | 134 +++++++++++++++++++---
clang/test/CodeGenCoroutines/coro-gro.cpp | 71 ++++++++----
2 files changed, 165 insertions(+), 40 deletions(-)
diff --git a/clang/lib/CodeGen/CGCoroutine.cpp b/clang/lib/CodeGen/CGCoroutine.cpp
index b76450152203d..d6fa1727cc841 100644
--- a/clang/lib/CodeGen/CGCoroutine.cpp
+++ b/clang/lib/CodeGen/CGCoroutine.cpp
@@ -656,6 +656,8 @@ struct GetReturnObjectManager {
Address GroActiveFlag;
CodeGenFunction::AutoVarEmission GroEmission;
+ llvm::BasicBlock *ConvBB = nullptr;
+
GetReturnObjectManager(CodeGenFunction &CGF, const CoroutineBodyStmt &S)
: CGF(CGF), Builder(CGF.Builder), S(S), GroActiveFlag(Address::invalid()),
GroEmission(CodeGenFunction::AutoVarEmission::invalid()) {
@@ -687,11 +689,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 +703,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);
@@ -768,6 +781,76 @@ struct GetReturnObjectManager {
CGF.EmitAutoVarInit(GroEmission);
Builder.CreateStore(Builder.getTrue(), GroActiveFlag);
}
+ // The GRO returns either when it is first suspended or when it completes
+ // without ever being suspended. The EmitGroConv function evaluates these
+ // conditions and executes the conversion if true.
+ //
+ // Before EmitGroConv():
+ // final.exit:
+ // switch i32 %cleanup.dest, label %destroy [
+ // i32 0, label %InsertPt
+ // ]
+ //
+ // InsertPt:
+ // ; (empty)
+ //
+ // After EmitGroConv():
+ // final.exit:
+ // switch i32 %cleanup.dest, label %destroy [
+ // i32 0, label %pre.gro.conv
+ // ]
+ //
+ // pre.gro.conv:
+ // %IsFinalExit = phi i1 [ false, %any.suspend ], [ true, %final.exit ]
+ // %InRamp = call i1 @llvm.coro.is_in_ramp()
+ // br i1 %InRamp, label %gro.conv, label %after.gro.conv
+ //
+ // gro.conv:
+ // ; Empty. We will collect GRO conversion later
+ // br label %after.gro.conv
+ //
+ // after.gro.conv:
+ // br i1 %IsFinalExit, label %InsertPt, label %coro.ret
+ void EmitGroConv(BasicBlock *RetBB) {
+ auto *InsertPt = Builder.GetInsertBlock();
+ Builder.ClearInsertionPoint();
+
+ auto *PreConvBB = CGF.CurCoro.Data->SuspendBB;
+ CGF.EmitBlock(PreConvBB);
+ // If final suspend exists, redirect final.exit to PreConvBB
+ bool HasFinalSuspend = CGF.CurCoro.Data->CurrentAwaitKind == AwaitKind::Final;
+ llvm::PHINode *IsFinalExit = nullptr;
+ if (HasFinalSuspend) {
+ assert(InsertPt && InsertPt->getSinglePredecessor() &&
+ "Expect fallthrough from final.exit block");
+ BasicBlock *FinalExit = InsertPt->getSinglePredecessor();
+ InsertPt->replaceAllUsesWith(PreConvBB);
+ PreConvBB->moveBefore(InsertPt);
+
+ // If true, coroutine completes and should be destroyed after conversion
+ IsFinalExit =
+ Builder.CreatePHI(Builder.getInt1Ty(), llvm::pred_size(PreConvBB));
+ for (auto *Pred : llvm::predecessors(PreConvBB)) {
+ auto *V = (Pred == FinalExit) ? Builder.getTrue() : Builder.getFalse();
+ IsFinalExit->addIncoming(V, Pred);
+ }
+ }
+ auto *InRampFn = CGF.CGM.getIntrinsic(llvm::Intrinsic::coro_is_in_ramp);
+ auto *InRamp = Builder.CreateCall(InRampFn, {}, "InRamp");
+ auto *AfterConvBB = CGF.createBasicBlock("after.gro.conv");
+ ConvBB = CGF.createBasicBlock("gro.conv");
+ Builder.CreateCondBr(InRamp, ConvBB, AfterConvBB);
+
+ CGF.EmitBlock(ConvBB);
+ Builder.CreateBr(AfterConvBB);
+
+ CGF.EmitBlock(AfterConvBB);
+ if (HasFinalSuspend)
+ Builder.CreateCondBr(IsFinalExit, InsertPt, RetBB);
+ else
+ Builder.CreateBr(RetBB);
+ Builder.SetInsertPoint(InsertPt);
+ }
};
} // namespace
@@ -795,7 +878,10 @@ void CodeGenFunction::EmitCoroutineBody(const CoroutineBodyStmt &S) {
CGM.getIntrinsic(llvm::Intrinsic::coro_id),
{Builder.getInt32(NewAlign), NullPtr, NullPtr, NullPtr});
createCoroData(*this, CurCoro, CoroId);
- CurCoro.Data->SuspendBB = RetBB;
+
+ GetReturnObjectManager GroManager(*this, S);
+ CurCoro.Data->SuspendBB =
+ GroManager.DirectEmit ? RetBB : createBasicBlock("pre.gvo.conv");
assert(ShouldEmitLifetimeMarkers &&
"Must emit lifetime intrinsics for coroutines");
@@ -839,9 +925,6 @@ void CodeGenFunction::EmitCoroutineBody(const CoroutineBodyStmt &S) {
CGM.getIntrinsic(llvm::Intrinsic::coro_begin), {CoroId, Phi});
CurCoro.Data->CoroBegin = CoroBegin;
- GetReturnObjectManager GroManager(*this, S);
- GroManager.EmitGroAlloca();
-
CurCoro.Data->CleanupJD = getJumpDestInCurrentScope(RetBB);
{
CGDebugInfo *DI = getDebugInfo();
@@ -884,6 +967,7 @@ void CodeGenFunction::EmitCoroutineBody(const CoroutineBodyStmt &S) {
// not needed.
}
+ GroManager.EmitGroActive();
EmitStmt(S.getPromiseDeclStmt());
Address PromiseAddr = GetAddrOfLocalVar(S.getPromiseDecl());
@@ -895,6 +979,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);
@@ -950,15 +1035,20 @@ void CodeGenFunction::EmitCoroutineBody(const CoroutineBodyStmt &S) {
// We don't need FinalBB. Emit it to make sure the block is deleted.
EmitBlock(FinalBB, /*IsFinished=*/true);
}
+
+ // GRO conversion is unnecessary when get_return_object's type matches the
+ // coroutine return type.
+ if (!GroManager.DirectEmit)
+ GroManager.EmitGroConv(RetBB);
}
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(),
- llvm::ConstantTokenNone::get(CoroEnd->getContext())});
+ auto *EndCall = Builder.CreateCall(
+ CoroEnd, {NullPtr, Builder.getFalse(),
+ llvm::ConstantTokenNone::get(CoroEnd->getContext())});
if (Stmt *Ret = S.getReturnStmt()) {
// Since we already emitted the return value above, so we shouldn't
@@ -973,8 +1063,18 @@ void CodeGenFunction::EmitCoroutineBody(const CoroutineBodyStmt &S) {
// shouldn't change the AST.
if (PreviousRetValue)
cast<ReturnStmt>(Ret)->setRetValue(PreviousRetValue);
- }
+ if (!GroManager.DirectEmit) {
+ EndCall->moveBefore(RetBB->getTerminator()->getIterator());
+ // Send GRO conversion to ConvBB
+ BasicBlock *ConvBB = GroManager.ConvBB;
+ auto *GroConv = RetBB->splitBasicBlockBefore(EndCall);
+ GroConv->replaceAllUsesWith(RetBB);
+ GroConv->getTerminator()->eraseFromParent();
+ ConvBB->splice(ConvBB->begin(), GroConv);
+ GroConv->eraseFromParent();
+ }
+ }
// LLVM require the frontend to mark the coroutine.
CurFn->setPresplitCoroutine();
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<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 {
More information about the cfe-commits
mailing list