[clang] [Clang] CGCoroutine: Skip moving parameters if the allocation decision is false (PR #81195)
Yuxuan Chen via cfe-commits
cfe-commits at lists.llvm.org
Fri Feb 9 09:49:27 PST 2024
https://github.com/yuxuanchen1997 updated https://github.com/llvm/llvm-project/pull/81195
>From b08a0fed9ae7d0ea5a130875be4d2f5ef3a38b3b Mon Sep 17 00:00:00 2001
From: Yuxuan Chen <ych at meta.com>
Date: Wed, 7 Feb 2024 16:05:42 -0800
Subject: [PATCH] Skip moving parameters if the allocation decision is false
---
clang/lib/CodeGen/CGCoroutine.cpp | 120 ++++++++++++++-----
clang/test/CodeGenCoroutines/coro-gro.cpp | 6 +-
clang/test/CodeGenCoroutines/coro-params.cpp | 48 +++++---
3 files changed, 130 insertions(+), 44 deletions(-)
diff --git a/clang/lib/CodeGen/CGCoroutine.cpp b/clang/lib/CodeGen/CGCoroutine.cpp
index 888d30bfb3e1d6..b2933650367c12 100644
--- a/clang/lib/CodeGen/CGCoroutine.cpp
+++ b/clang/lib/CodeGen/CGCoroutine.cpp
@@ -389,25 +389,12 @@ namespace {
ParamReferenceReplacerRAII(CodeGenFunction::DeclMapTy &LocalDeclMap)
: LocalDeclMap(LocalDeclMap) {}
- void addCopy(DeclStmt const *PM) {
- // Figure out what param it refers to.
-
- assert(PM->isSingleDecl());
- VarDecl const*VD = static_cast<VarDecl const*>(PM->getSingleDecl());
- Expr const *InitExpr = VD->getInit();
- GetParamRef Visitor;
- Visitor.Visit(const_cast<Expr*>(InitExpr));
- assert(Visitor.Expr);
- DeclRefExpr *DREOrig = Visitor.Expr;
- auto *PD = DREOrig->getDecl();
-
- auto it = LocalDeclMap.find(PD);
- assert(it != LocalDeclMap.end() && "parameter is not found");
- SavedLocals.insert({ PD, it->second });
-
- auto copyIt = LocalDeclMap.find(VD);
- assert(copyIt != LocalDeclMap.end() && "parameter copy is not found");
- it->second = copyIt->getSecond();
+ void substAddress(ValueDecl *D, Address Addr) {
+ auto it = LocalDeclMap.find(D);
+ assert(it != LocalDeclMap.end() && "original decl is not found");
+ SavedLocals.insert({D, it->second});
+
+ it->second = Addr;
}
~ParamReferenceReplacerRAII() {
@@ -629,6 +616,63 @@ struct GetReturnObjectManager {
Builder.CreateStore(Builder.getTrue(), GroActiveFlag);
}
};
+
+static ValueDecl *getOriginalParamDeclForParamMove(VarDecl const *VD) {
+ Expr const *InitExpr = VD->getInit();
+ GetParamRef Visitor;
+ Visitor.Visit(const_cast<Expr *>(InitExpr));
+ assert(Visitor.Expr);
+ return Visitor.Expr->getDecl();
+}
+
+struct ParamMoveManager {
+ ParamMoveManager(CodeGenFunction &CGF,
+ llvm::ArrayRef<const Stmt *> ParamMoves)
+ : CGF(CGF), ParamMovesVarDecls() {
+ ParamMovesVarDecls.reserve(ParamMoves.size());
+ for (auto *S : ParamMoves) {
+ auto *PMStmt = cast<DeclStmt>(S);
+ assert(PMStmt->isSingleDecl());
+ auto *ParamMoveVD = static_cast<VarDecl const *>(PMStmt->getSingleDecl());
+ ParamMovesVarDecls.push_back(ParamMoveVD);
+ }
+ }
+
+ // Because we wrap param moves in the coro.alloc block. It's not always
+ // necessary to run the corresponding cleanups in the branches.
+ // We would need to know when to (conditionally) clean them up.
+ void EmitMovesWithCleanup(Address PMCleanupActiveFlag) {
+ // Create parameter copies. We do it before creating a promise, since an
+ // evolution of coroutine TS may allow promise constructor to observe
+ // parameter copies.
+ for (auto *VD : ParamMovesVarDecls) {
+ auto Emission = CGF.EmitAutoVarAlloca(*VD);
+ CGF.EmitAutoVarInit(Emission);
+ auto OldTop = CGF.EHStack.stable_begin();
+ CGF.EmitAutoVarCleanups(Emission);
+ auto Top = CGF.EHStack.stable_begin();
+
+ for (auto I = CGF.EHStack.find(Top), E = CGF.EHStack.find(OldTop); I != E;
+ I++) {
+ if (auto *Cleanup = dyn_cast<EHCleanupScope>(&*I)) {
+ assert(!Cleanup->hasActiveFlag() &&
+ "cleanup already has active flag?");
+ Cleanup->setActiveFlag(PMCleanupActiveFlag);
+ Cleanup->setTestFlagInEHCleanup();
+ Cleanup->setTestFlagInNormalCleanup();
+ }
+ }
+ }
+ }
+
+ llvm::ArrayRef<const VarDecl *> GetParamMovesVarDecls() {
+ return ParamMovesVarDecls;
+ }
+
+private:
+ CodeGenFunction &CGF;
+ SmallVector<const VarDecl *> ParamMovesVarDecls;
+};
} // namespace
static void emitBodyAndFallthrough(CodeGenFunction &CGF,
@@ -648,6 +692,8 @@ void CodeGenFunction::EmitCoroutineBody(const CoroutineBodyStmt &S) {
auto *EntryBB = Builder.GetInsertBlock();
auto *AllocBB = createBasicBlock("coro.alloc");
auto *InitBB = createBasicBlock("coro.init");
+ auto *ParamMoveBB = createBasicBlock("coro.param.move");
+ auto *AfterParamMoveBB = createBasicBlock("coro.after.param.move");
auto *FinalBB = createBasicBlock("coro.final");
auto *RetBB = createBasicBlock("coro.ret");
@@ -664,6 +710,9 @@ void CodeGenFunction::EmitCoroutineBody(const CoroutineBodyStmt &S) {
auto *CoroAlloc = Builder.CreateCall(
CGM.getIntrinsic(llvm::Intrinsic::coro_alloc), {CoroId});
+ auto PMCleanupActiveFlag = CreateTempAlloca(
+ Builder.getInt1Ty(), CharUnits::One(), "param.move.cleanup.active");
+ Builder.CreateStore(CoroAlloc, PMCleanupActiveFlag);
Builder.CreateCondBr(CoroAlloc, AllocBB, InitBB);
EmitBlock(AllocBB);
@@ -695,6 +744,7 @@ void CodeGenFunction::EmitCoroutineBody(const CoroutineBodyStmt &S) {
auto *Phi = Builder.CreatePHI(VoidPtrTy, 2);
Phi->addIncoming(NullPtr, EntryBB);
Phi->addIncoming(AllocateCall, AllocOrInvokeContBB);
+
auto *CoroBegin = Builder.CreateCall(
CGM.getIntrinsic(llvm::Intrinsic::coro_begin), {CoroId, Phi});
CurCoro.Data->CoroBegin = CoroBegin;
@@ -719,15 +769,29 @@ void CodeGenFunction::EmitCoroutineBody(const CoroutineBodyStmt &S) {
DI->getCoroutineParameterMappings().insert(
{std::get<0>(Pair), std::get<1>(Pair)});
- // Create parameter copies. We do it before creating a promise, since an
- // evolution of coroutine TS may allow promise constructor to observe
- // parameter copies.
- for (auto *PM : S.getParamMoves()) {
- EmitStmt(PM);
- ParamReplacer.addCopy(cast<DeclStmt>(PM));
- // TODO: if(CoroParam(...)) need to surround ctor and dtor
- // for the copy, so that llvm can elide it if the copy is
- // not needed.
+ Builder.CreateCondBr(CoroAlloc, ParamMoveBB, AfterParamMoveBB);
+
+ EmitBlock(ParamMoveBB);
+
+ ParamMoveManager Mover{*this, ParamMoves};
+ Mover.EmitMovesWithCleanup(PMCleanupActiveFlag);
+
+ Builder.CreateBr(AfterParamMoveBB);
+
+ EmitBlock(AfterParamMoveBB);
+
+ for (auto *VD : Mover.GetParamMovesVarDecls()) {
+ auto NewAddr = LocalDeclMap.find(VD)->getSecond();
+ auto *OrigVD = getOriginalParamDeclForParamMove(VD);
+ auto OldAddr = LocalDeclMap.find(OrigVD)->getSecond();
+
+ auto *ParamPhi = Builder.CreatePHI(VoidPtrTy, 2);
+ ParamPhi->addIncoming(NewAddr.getPointer(), ParamMoveBB);
+ ParamPhi->addIncoming(OldAddr.getPointer(), InitBB);
+
+ ParamReplacer.substAddress(
+ OrigVD,
+ Address(ParamPhi, OldAddr.getElementType(), OldAddr.getAlignment()));
}
EmitStmt(S.getPromiseDeclStmt());
diff --git a/clang/test/CodeGenCoroutines/coro-gro.cpp b/clang/test/CodeGenCoroutines/coro-gro.cpp
index d4c3ff589e340a..fb42c7e089b363 100644
--- a/clang/test/CodeGenCoroutines/coro-gro.cpp
+++ b/clang/test/CodeGenCoroutines/coro-gro.cpp
@@ -29,8 +29,11 @@ void doSomething() noexcept;
// CHECK: define{{.*}} i32 @_Z1fv(
int f() {
// CHECK: %[[RetVal:.+]] = alloca i32
+ // CHECK: %[[ParamMoveCleanupActive:.+]] = alloca i1
// CHECK: %[[GroActive:.+]] = alloca i1
// CHECK: %[[CoroGro:.+]] = alloca %struct.GroType, {{.*}} !coro.outside.frame ![[OutFrameMetadata:.+]]
+ // CHECK: %[[CoroAlloc:.+]] = call i1 @llvm.coro.alloc
+ // CHECK-NEXT: store i1 %[[CoroAlloc:.+]], ptr %[[ParamMoveCleanupActive:.+]]
// CHECK: %[[Size:.+]] = call i64 @llvm.coro.size.i64()
// CHECK: call noalias noundef nonnull ptr @_Znwm(i64 noundef %[[Size]])
@@ -94,6 +97,7 @@ class invoker {
// CHECK: define{{.*}} void @_Z1gv({{.*}} %[[AggRes:.+]])
invoker g() {
// CHECK: %[[ResultPtr:.+]] = alloca ptr
+ // CHECK-NEXT: %[[ParamMoveCleanupActive:.+]] = alloca i1
// CHECK-NEXT: %[[Promise:.+]] = alloca %"class.invoker::invoker_promise"
// CHECK: store ptr %[[AggRes]], ptr %[[ResultPtr]]
@@ -105,4 +109,4 @@ invoker g() {
// CHECK: call void @_ZN7invoker15invoker_promise17get_return_objectEv({{.*}} %[[AggRes]]
co_return;
}
-// CHECK: ![[OutFrameMetadata]] = !{}
\ No newline at end of file
+// CHECK: ![[OutFrameMetadata]] = !{}
diff --git a/clang/test/CodeGenCoroutines/coro-params.cpp b/clang/test/CodeGenCoroutines/coro-params.cpp
index c5a61a53cb46ed..873936ac28aba4 100644
--- a/clang/test/CodeGenCoroutines/coro-params.cpp
+++ b/clang/test/CodeGenCoroutines/coro-params.cpp
@@ -64,22 +64,36 @@ void consume(int,int,int) noexcept;
// TODO: Add support for CopyOnly params
// CHECK: define{{.*}} void @_Z1fi8MoveOnly11MoveAndCopy(i32 noundef %val, ptr noundef %[[MoParam:.+]], ptr noundef %[[McParam:.+]]) #0 personality ptr @__gxx_personality_v0
void f(int val, MoveOnly moParam, MoveAndCopy mcParam) {
+ // CHECK: %[[ValCopy:.+]] = alloca i32,
// CHECK: %[[MoCopy:.+]] = alloca %struct.MoveOnly,
// CHECK: %[[McCopy:.+]] = alloca %struct.MoveAndCopy,
// CHECK: store i32 %val, ptr %[[ValAddr:.+]]
+ // CHECK: %[[AllocFlag:.+]] = call i1 @llvm.coro.alloc
+ // CHECK-NEXT: store i1 %[[AllocFlag:.+]] %[[CleanupActiveMem:.+]]
// CHECK: call ptr @llvm.coro.begin(
- // CHECK: call void @_ZN8MoveOnlyC1EOS_(ptr {{[^,]*}} %[[MoCopy]], ptr noundef nonnull align 4 dereferenceable(4) %[[MoParam]])
- // CHECK-NEXT: call void @llvm.lifetime.start.p0(
- // CHECK-NEXT: call void @_ZN11MoveAndCopyC1EOS_(ptr {{[^,]*}} %[[McCopy]], ptr noundef nonnull align 4 dereferenceable(4) %[[McParam]]) #
- // CHECK-NEXT: call void @llvm.lifetime.start.p0(
- // CHECK-NEXT: invoke void @_ZNSt16coroutine_traitsIJvi8MoveOnly11MoveAndCopyEE12promise_typeC1Ev(
-
+ // CHECK: br i1 %[[CoroAlloc:.+]], label %coro.param.move, label %coro.after.param.move
+ // CHECK: coro.param.move:
+ // CHECK: call void @llvm.lifetime.start.p0(i64 4, ptr %[[ValCopy:.+]])
+ // CHECK-NEXT: %[[Temp:.+]] = load i32, ptr %val.addr, align 4
+ // CHECK-NEXT: store i32 %[[Temp:.+]], ptr %val1, align 4
+ // CHECK-NEXT: call void @llvm.lifetime.start.p0(i64 4, ptr %[[MoCopy:.+]]) #2
+ // CHECK-NEXT: call void @_ZN8MoveOnlyC1EOS_(ptr noundef nonnull align 4 dereferenceable(4) %[[MoCopy:.+]], ptr noundef nonnull align 4 dereferenceable(4) %[[MoParam:.+]]) #2
+ // CHECK-NEXT: call void @llvm.lifetime.start.p0(i64 4, ptr %[[McCopy:.+]]) #2
+ // CHECK-NEXT: call void @_ZN11MoveAndCopyC1EOS_(ptr noundef nonnull align 4 dereferenceable(4) %[[McCopy:.+]], ptr noundef nonnull align 4 dereferenceable(4) %[[McParam:.+]]) #2
+ // CHECK-NEXT: br label %coro.after.param.move
+ //
+ // CHECK: %[[ValPhi:.+]] = phi ptr [ %[[ValCopy:.+]], %coro.param.move ], [ %[[ValAddr:.+]], %coro.init ]
+ // CHECK-NEXT: %[[MoPhi:.+]] = phi ptr [ %[[MoCopy:.+]], %coro.param.move ], [ %[[MoParam:.+]], %coro.init ]
+ // CHECK-NEXT: %[[McPhi:.+]] = phi ptr [ %[[McCopy:.+]], %coro.param.move ], [ %[[McParam:.+]], %coro.init ]
+ // CHECK: invoke void @_ZNSt16coroutine_traitsIJvi8MoveOnly11MoveAndCopyEE12promise_typeC1Ev(
+
+ // CHECK: init.ready
// CHECK: call void @_ZN14suspend_always12await_resumeEv(
// CHECK: %[[IntParam:.+]] = load i32, ptr %{{.*}}
- // CHECK: %[[MoGep:.+]] = getelementptr inbounds %struct.MoveOnly, ptr %[[MoCopy]], i32 0, i32 0
+ // CHECK: %[[MoGep:.+]] = getelementptr inbounds %struct.MoveOnly, ptr %[[MoPhi]], i32 0, i32 0
// CHECK: %[[MoVal:.+]] = load i32, ptr %[[MoGep]]
- // CHECK: %[[McGep:.+]] = getelementptr inbounds %struct.MoveAndCopy, ptr %[[McCopy]], i32 0, i32 0
+ // CHECK: %[[McGep:.+]] = getelementptr inbounds %struct.MoveAndCopy, ptr %[[McPhi]], i32 0, i32 0
// CHECK: %[[McVal:.+]] = load i32, ptr %[[McGep]]
// CHECK: call void @_Z7consumeiii(i32 noundef %[[IntParam]], i32 noundef %[[MoVal]], i32 noundef %[[McVal]])
@@ -90,13 +104,17 @@ void f(int val, MoveOnly moParam, MoveAndCopy mcParam) {
// CHECK: call void @_ZNSt16coroutine_traitsIJvi8MoveOnly11MoveAndCopyEE12promise_type13final_suspendEv(
// CHECK: call void @_ZN14suspend_always12await_resumeEv(
- // Destroy promise, then parameter copies:
+ // Destroy promise, then test the cleanup flag for parameter copies (if exist):
// CHECK: call void @_ZNSt16coroutine_traitsIJvi8MoveOnly11MoveAndCopyEE12promise_typeD1Ev(ptr {{[^,]*}} %__promise)
// CHECK-NEXT: call void @llvm.lifetime.end.p0(
- // CHECK-NEXT: call void @_ZN11MoveAndCopyD1Ev(ptr {{[^,]*}} %[[McCopy]])
- // CHECK-NEXT: call void @llvm.lifetime.end.p0(
- // CHECK-NEXT: call void @_ZN8MoveOnlyD1Ev(ptr {{[^,]*}} %[[MoCopy]]
- // CHECK-NEXT: call void @llvm.lifetime.end.p0(
+ // CHECK-NEXT: %[[TempCleanupActive:.+]] = load i1, ptr %[[CleanupActiveMem:.+]]
+ // CHECK-NEXT: br i1 %[[TempCleanupActive]]
+
+ // The next two may be in different cleanup blocks:
+ // CHECK: call void @_ZN11MoveAndCopyD1Ev(ptr {{[^,]*}} %[[McCopy]])
+ // CHECK: call void @_ZN8MoveOnlyD1Ev(ptr {{[^,]*}} %[[MoCopy]]
+
+ // CHECK: call void @llvm.lifetime.end.p0(
// CHECK-NEXT: call void @llvm.lifetime.end.p0(
// CHECK-NEXT: call ptr @llvm.coro.free(
}
@@ -109,13 +127,13 @@ void dependent_params(T x, U, U y) {
// CHECK-NEXT: %[[y_copy:.+]] = alloca %struct.B
// CHECK: call ptr @llvm.coro.begin
- // CHECK-NEXT: call void @llvm.lifetime.start.p0(
+ // CHECK: call void @llvm.lifetime.start.p0(
// CHECK-NEXT: call void @_ZN1AC1EOS_(ptr {{[^,]*}} %[[x_copy]], ptr noundef nonnull align 4 dereferenceable(512) %x)
// CHECK-NEXT: call void @llvm.lifetime.start.p0(
// CHECK-NEXT: call void @_ZN1BC1EOS_(ptr {{[^,]*}} %[[unnamed_copy]], ptr noundef nonnull align 4 dereferenceable(512) %0)
// CHECK-NEXT: call void @llvm.lifetime.start.p0(
// CHECK-NEXT: call void @_ZN1BC1EOS_(ptr {{[^,]*}} %[[y_copy]], ptr noundef nonnull align 4 dereferenceable(512) %y)
- // CHECK-NEXT: call void @llvm.lifetime.start.p0(
+ // CHECK: call void @llvm.lifetime.start.p0(
// CHECK-NEXT: invoke void @_ZNSt16coroutine_traitsIJv1A1BS1_EE12promise_typeC1Ev(
co_return;
More information about the cfe-commits
mailing list