-
Notifications
You must be signed in to change notification settings - Fork 13.4k
[Coroutines] Replace coro.outside.frame metadata with an intrinsic #129255
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.
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
base: main
Are you sure you want to change the base?
Conversation
Metadata should not be "load bearing", i.e. required for correctness, since optimizations are not required to preserve it. Therefore, turn this into an intrinsic instead. This is follow-up to llvm#127653.
@llvm/pr-subscribers-llvm-ir @llvm/pr-subscribers-clang-codegen Author: Hans Wennborg (zmodem) ChangesMetadata should not be "load bearing", i.e. required for correctness, since optimizations are not required to preserve it. Therefore, turn this into an intrinsic instead. This is a follow-up to #127653. Full diff: https://github.com/llvm/llvm-project/pull/129255.diff 15 Files Affected:
diff --git a/clang/lib/CodeGen/CGCoroutine.cpp b/clang/lib/CodeGen/CGCoroutine.cpp
index a9795c2c0dc8f..7bd3208990572 100644
--- a/clang/lib/CodeGen/CGCoroutine.cpp
+++ b/clang/lib/CodeGen/CGCoroutine.cpp
@@ -709,8 +709,8 @@ struct GetReturnObjectManager {
auto *GroAlloca = dyn_cast_or_null<llvm::AllocaInst>(
GroEmission.getOriginalAllocatedAddress().getPointer());
assert(GroAlloca && "expected alloca to be emitted");
- GroAlloca->setMetadata(llvm::LLVMContext::MD_coro_outside_frame,
- llvm::MDNode::get(CGF.CGM.getLLVMContext(), {}));
+ Builder.CreateCall(
+ CGF.CGM.getIntrinsic(llvm::Intrinsic::coro_outside_frame), {GroAlloca});
// Remember the top of EHStack before emitting the cleanup.
auto old_top = CGF.EHStack.stable_begin();
@@ -863,10 +863,9 @@ void CodeGenFunction::EmitCoroutineBody(const CoroutineBodyStmt &S) {
// in the MSVC C++ ABI, are appropriately destroyed after setting up the
// coroutine.
Address ParmAddr = GetAddrOfLocalVar(Parm);
- if (auto *ParmAlloca =
- dyn_cast<llvm::AllocaInst>(ParmAddr.getBasePointer())) {
- ParmAlloca->setMetadata(llvm::LLVMContext::MD_coro_outside_frame,
- llvm::MDNode::get(CGM.getLLVMContext(), {}));
+ if (auto *PA = dyn_cast<llvm::AllocaInst>(ParmAddr.getBasePointer())) {
+ Builder.CreateCall(
+ CGM.getIntrinsic(llvm::Intrinsic::coro_outside_frame), {PA});
}
}
for (auto *PM : S.getParamMoves()) {
diff --git a/clang/test/CodeGenCoroutines/coro-gro.cpp b/clang/test/CodeGenCoroutines/coro-gro.cpp
index b62134317cef2..bfcfc641dcd72 100644
--- a/clang/test/CodeGenCoroutines/coro-gro.cpp
+++ b/clang/test/CodeGenCoroutines/coro-gro.cpp
@@ -30,11 +30,12 @@ void doSomething() noexcept;
int f() {
// CHECK: %[[RetVal:.+]] = alloca i32
// CHECK: %[[GroActive:.+]] = alloca i1
- // CHECK: %[[CoroGro:.+]] = alloca %struct.GroType, {{.*}} !coro.outside.frame ![[OutFrameMetadata:.+]]
+ // CHECK: %[[CoroGro:.+]] = alloca %struct.GroType
// 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 @llvm.coro.outside.frame(ptr %[[CoroGro]])
// CHECK: call void @_ZNSt16coroutine_traitsIiJEE12promise_typeC1Ev(
// CHECK: call void @_ZNSt16coroutine_traitsIiJEE12promise_type17get_return_objectEv({{.*}} %[[CoroGro]]
// CHECK: store i1 true, ptr %[[GroActive]]
@@ -106,4 +107,3 @@ invoker g() {
// CHECK: call void @_ZN7invoker15invoker_promise17get_return_objectEv({{.*}} %[[AggRes]]
co_return;
}
-// CHECK: ![[OutFrameMetadata]] = !{}
\ No newline at end of file
diff --git a/clang/test/CodeGenCoroutines/coro-params.cpp b/clang/test/CodeGenCoroutines/coro-params.cpp
index 719726cca29c5..792500ce5ac23 100644
--- a/clang/test/CodeGenCoroutines/coro-params.cpp
+++ b/clang/test/CodeGenCoroutines/coro-params.cpp
@@ -72,13 +72,13 @@ void consume(int,int,int,int) noexcept;
// CHECK: define{{.*}} void @_Z1fi8MoveOnly11MoveAndCopy10TrivialABI(i32 noundef %val, ptr noundef %[[MoParam:.+]], ptr noundef %[[McParam:.+]], i32 %[[TrivialParam:.+]]) #0 personality ptr @__gxx_personality_v0
void f(int val, MoveOnly moParam, MoveAndCopy mcParam, TrivialABI trivialParam) {
// CHECK: %[[TrivialAlloca:.+]] = alloca %struct.TrivialABI,
- // CHECK-SAME: !coro.outside.frame
// CHECK: %[[MoCopy:.+]] = alloca %struct.MoveOnly,
// CHECK: %[[McCopy:.+]] = alloca %struct.MoveAndCopy,
// CHECK: %[[TrivialCopy:.+]] = alloca %struct.TrivialABI,
// CHECK: store i32 %val, ptr %[[ValAddr:.+]]
// CHECK: call ptr @llvm.coro.begin(
+ // CHECK: @llvm.coro.outside.frame(ptr %[[TrivialAlloca]])
// 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]]) #
@@ -226,13 +226,12 @@ void consume(int) noexcept;
// may be destroyed before the destructor call.
void msabi(MSParm p) {
// MSABI: define{{.*}} void @"?msabi@@YAXUMSParm@@@Z"(i32 %[[Param:.+]])
-
- // The parameter's local alloca is marked not part of the frame.
// MSABI: %[[ParamAlloca:.+]] = alloca %struct.MSParm
- // MSABI-SAME: !coro.outside.frame
-
// MSABI: %[[ParamCopy:.+]] = alloca %struct.MSParm
+ // The parameter's local alloca is marked not part of the frame.
+ // MSABI: call void @llvm.coro.outside.frame(ptr %[[ParamAlloca]])
+
consume(p.val);
// The parameter's copy is used by the coroutine.
// MSABI: %[[ValPtr:.+]] = getelementptr inbounds nuw %struct.MSParm, ptr %[[ParamCopy]], i32 0, i32 0
diff --git a/llvm/docs/Coroutines.rst b/llvm/docs/Coroutines.rst
index 60e32dc467d27..55bc72026c1ed 100644
--- a/llvm/docs/Coroutines.rst
+++ b/llvm/docs/Coroutines.rst
@@ -1641,6 +1641,37 @@ the function call.
ptr %ctxt, ptr %task, ptr %actor)
unreachable
+
+.. _coro.outside.frame:
+
+'llvm.coro.outside.frame' Intrinsic
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+::
+
+ declare void @llvm.coro.outside.frame(ptr %p)
+
+Overview:
+"""""""""
+
+Signifies that an alloca shouldn't be promoted to the coroutine frame.
+
+Arguments:
+""""""""""
+
+An alloca that should remain outside the coroutine frame.
+
+Semantics:
+""""""""""
+
+Calling `@llvm.coro.outside.frame` with an alloca signifies that the alloca
+should not be promoted to the coroutine frame. This allows the frontend to
+ensure that e.g. callee-destructed parameters are allocated on the stack of the
+ramp function, and thus available until the function returns.
+
+The intrinsic should only be used in presplit coroutines, and is consumed by
+the CoroSplit pass.
+
+
.. _coro.suspend:
.. _suspend points:
@@ -2179,25 +2210,6 @@ or deallocations that may be guarded by `@llvm.coro.alloc` and `@llvm.coro.free`
CoroAnnotationElidePass performs the heap elision when possible. Note that for
recursive or mutually recursive functions this elision is usually not possible.
-Metadata
-========
-
-'``coro.outside.frame``' Metadata
----------------------------------
-
-``coro.outside.frame`` metadata may be attached to an alloca instruction to
-to signify that it shouldn't be promoted to the coroutine frame, useful for
-filtering allocas out by the frontend when emitting internal control mechanisms.
-Additionally, this metadata is only used as a flag, so the associated
-node must be empty.
-
-.. code-block:: text
-
- %__coro_gro = alloca %struct.GroType, align 1, !coro.outside.frame !0
-
- ...
- !0 = !{}
-
Areas Requiring Attention
=========================
#. When coro.suspend returns -1, the coroutine is suspended, and it's possible
diff --git a/llvm/docs/ReleaseNotes.md b/llvm/docs/ReleaseNotes.md
index 12dd09ad41135..24e517f9942f5 100644
--- a/llvm/docs/ReleaseNotes.md
+++ b/llvm/docs/ReleaseNotes.md
@@ -145,6 +145,10 @@ Changes to the CodeGen infrastructure
Changes to the Metadata Info
---------------------------------
+* The `coro.outside.frame` metadata has been replaced by [an intrinsic with the
+ same name](coro.outside.frame). The old metadata is still parsed but has no
+ effect.
+
Changes to the Debug Info
---------------------------------
diff --git a/llvm/include/llvm/IR/FixedMetadataKinds.def b/llvm/include/llvm/IR/FixedMetadataKinds.def
index df572e8791e13..712c139c2d57b 100644
--- a/llvm/include/llvm/IR/FixedMetadataKinds.def
+++ b/llvm/include/llvm/IR/FixedMetadataKinds.def
@@ -50,6 +50,7 @@ LLVM_FIXED_MD_KIND(MD_callsite, "callsite", 35)
LLVM_FIXED_MD_KIND(MD_kcfi_type, "kcfi_type", 36)
LLVM_FIXED_MD_KIND(MD_pcsections, "pcsections", 37)
LLVM_FIXED_MD_KIND(MD_DIAssignID, "DIAssignID", 38)
-LLVM_FIXED_MD_KIND(MD_coro_outside_frame, "coro.outside.frame", 39)
+LLVM_FIXED_MD_KIND(MD_coro_outside_frame_DEPRECATED,
+ "coro.outside.frame", 39) // Preserved for compatibility.
LLVM_FIXED_MD_KIND(MD_mmra, "mmra", 40)
LLVM_FIXED_MD_KIND(MD_noalias_addrspace, "noalias.addrspace", 41)
diff --git a/llvm/include/llvm/IR/Intrinsics.td b/llvm/include/llvm/IR/Intrinsics.td
index 62239ca705b9e..2d90bae3a0b64 100644
--- a/llvm/include/llvm/IR/Intrinsics.td
+++ b/llvm/include/llvm/IR/Intrinsics.td
@@ -1779,6 +1779,7 @@ def int_coro_alloca_alloc : Intrinsic<[llvm_token_ty],
[llvm_anyint_ty, llvm_i32_ty], []>;
def int_coro_alloca_get : Intrinsic<[llvm_ptr_ty], [llvm_token_ty], []>;
def int_coro_alloca_free : Intrinsic<[], [llvm_token_ty], []>;
+def int_coro_outside_frame : Intrinsic<[], [llvm_ptr_ty], [IntrNoMem]>;
// Coroutine Manipulation Intrinsics.
diff --git a/llvm/include/llvm/Transforms/Coroutines/CoroInstr.h b/llvm/include/llvm/Transforms/Coroutines/CoroInstr.h
index fbc76219ead86..f24d7214967a3 100644
--- a/llvm/include/llvm/Transforms/Coroutines/CoroInstr.h
+++ b/llvm/include/llvm/Transforms/Coroutines/CoroInstr.h
@@ -796,6 +796,22 @@ class CoroAllocaFreeInst : public IntrinsicInst {
}
};
+/// This represents the llvm.coro.outside.frame instruction.
+class CoroOutsideFrameInst : public IntrinsicInst {
+ enum { PtrArg };
+
+public:
+ Value *getPtr() const { return getArgOperand(PtrArg); }
+
+ // Methods to support type inquiry through isa, cast, and dyn_cast:
+ static bool classof(const IntrinsicInst *I) {
+ return I->getIntrinsicID() == Intrinsic::coro_outside_frame;
+ }
+ static bool classof(const Value *V) {
+ return isa<IntrinsicInst>(V) && classof(cast<IntrinsicInst>(V));
+ }
+};
+
} // End namespace llvm.
#endif // LLVM_TRANSFORMS_COROUTINES_COROINSTR_H
diff --git a/llvm/include/llvm/Transforms/Coroutines/CoroShape.h b/llvm/include/llvm/Transforms/Coroutines/CoroShape.h
index ea93ced1ce29e..657c2c8bfdc82 100644
--- a/llvm/include/llvm/Transforms/Coroutines/CoroShape.h
+++ b/llvm/include/llvm/Transforms/Coroutines/CoroShape.h
@@ -57,6 +57,7 @@ struct Shape {
SmallVector<AnyCoroSuspendInst *, 4> CoroSuspends;
SmallVector<CoroAwaitSuspendInst *, 4> CoroAwaitSuspends;
SmallVector<CallInst *, 2> SymmetricTransfers;
+ SmallVector<CoroOutsideFrameInst *, 8> OutsideFrames;
// Values invalidated by replaceSwiftErrorOps()
SmallVector<CallInst *, 2> SwiftErrorOps;
@@ -69,6 +70,7 @@ struct Shape {
CoroSuspends.clear();
CoroAwaitSuspends.clear();
SymmetricTransfers.clear();
+ OutsideFrames.clear();
SwiftErrorOps.clear();
diff --git a/llvm/lib/Transforms/Coroutines/CoroSplit.cpp b/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
index e1f767edd6ee1..0a9961d3e06b8 100644
--- a/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
+++ b/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
@@ -2015,7 +2015,13 @@ static void doSplitCoroutine(Function &F, SmallVectorImpl<Function *> &Clones,
simplifySuspendPoints(Shape);
normalizeCoroutine(F, Shape, TTI);
+
ABI.buildCoroutineFrame(OptimizeFrame);
+
+ // @llvm.coro.outside.frame is not needed after the frame has been built.
+ for (Instruction *I : Shape.OutsideFrames)
+ I->eraseFromParent();
+
replaceFrameSizeAndAlignment(Shape);
bool isNoSuspendCoroutine = Shape.CoroSuspends.empty();
diff --git a/llvm/lib/Transforms/Coroutines/Coroutines.cpp b/llvm/lib/Transforms/Coroutines/Coroutines.cpp
index 7b59c39283ded..392f74cd2f52f 100644
--- a/llvm/lib/Transforms/Coroutines/Coroutines.cpp
+++ b/llvm/lib/Transforms/Coroutines/Coroutines.cpp
@@ -286,6 +286,9 @@ void coro::Shape::analyze(Function &F,
}
}
break;
+ case Intrinsic::coro_outside_frame:
+ OutsideFrames.push_back(cast<CoroOutsideFrameInst>(II));
+ break;
}
}
}
diff --git a/llvm/lib/Transforms/Coroutines/SpillUtils.cpp b/llvm/lib/Transforms/Coroutines/SpillUtils.cpp
index 5062ee97a665d..bfe22699f655c 100644
--- a/llvm/lib/Transforms/Coroutines/SpillUtils.cpp
+++ b/llvm/lib/Transforms/Coroutines/SpillUtils.cpp
@@ -417,7 +417,8 @@ struct AllocaUseVisitor : PtrUseVisitor<AllocaUseVisitor> {
static void collectFrameAlloca(AllocaInst *AI, const coro::Shape &Shape,
const SuspendCrossingInfo &Checker,
SmallVectorImpl<AllocaInfo> &Allocas,
- const DominatorTree &DT) {
+ const DominatorTree &DT,
+ const SmallPtrSetImpl<Value*> &OutsideFrameSet) {
if (Shape.CoroSuspends.empty())
return;
@@ -426,9 +427,8 @@ static void collectFrameAlloca(AllocaInst *AI, const coro::Shape &Shape,
if (AI == Shape.SwitchLowering.PromiseAlloca)
return;
- // The __coro_gro alloca should outlive the promise, make sure we
- // keep it outside the frame.
- if (AI->hasMetadata(LLVMContext::MD_coro_outside_frame))
+ // The alloca has been marked as belonging outside the frame.
+ if (OutsideFrameSet.contains(AI))
return;
// The code that uses lifetime.start intrinsic does not work for functions
@@ -464,6 +464,10 @@ void collectSpillsAndAllocasFromInsts(
const SuspendCrossingInfo &Checker, const DominatorTree &DT,
const coro::Shape &Shape) {
+ SmallPtrSet<Value*, 4> OutsideFramePtrs;
+ for (const CoroOutsideFrameInst *I : Shape.OutsideFrames)
+ OutsideFramePtrs.insert(I->getPtr());
+
for (Instruction &I : instructions(F)) {
// Values returned from coroutine structure intrinsics should not be part
// of the Coroutine Frame.
@@ -497,7 +501,7 @@ void collectSpillsAndAllocasFromInsts(
continue;
if (auto *AI = dyn_cast<AllocaInst>(&I)) {
- collectFrameAlloca(AI, Shape, Checker, Allocas, DT);
+ collectFrameAlloca(AI, Shape, Checker, Allocas, DT, OutsideFramePtrs);
continue;
}
diff --git a/llvm/lib/Transforms/Utils/PromoteMemoryToRegister.cpp b/llvm/lib/Transforms/Utils/PromoteMemoryToRegister.cpp
index 05fd989271c32..d6a38a8670c07 100644
--- a/llvm/lib/Transforms/Utils/PromoteMemoryToRegister.cpp
+++ b/llvm/lib/Transforms/Utils/PromoteMemoryToRegister.cpp
@@ -81,7 +81,8 @@ bool llvm::isAllocaPromotable(const AllocaInst *AI) {
return false;
} else if (const IntrinsicInst *II = dyn_cast<IntrinsicInst>(U)) {
if (!II->isLifetimeStartOrEnd() && !II->isDroppable() &&
- II->getIntrinsicID() != Intrinsic::fake_use)
+ II->getIntrinsicID() != Intrinsic::fake_use &&
+ II->getIntrinsicID() != Intrinsic::coro_outside_frame)
return false;
} else if (const BitCastInst *BCI = dyn_cast<BitCastInst>(U)) {
if (!onlyUsedByLifetimeMarkersOrDroppableInsts(BCI))
diff --git a/llvm/test/Transforms/Coroutines/coro-alloca-outside-frame.ll b/llvm/test/Transforms/Coroutines/coro-alloca-outside-frame.ll
index ac6a5752438ce..e227acb082d11 100644
--- a/llvm/test/Transforms/Coroutines/coro-alloca-outside-frame.ll
+++ b/llvm/test/Transforms/Coroutines/coro-alloca-outside-frame.ll
@@ -2,10 +2,29 @@
; RUN: opt < %s -passes='cgscc(coro-split),simplifycfg,early-cse' -S -o %t.ll
; RUN: FileCheck --input-file=%t.ll %s
-define ptr @f(i1 %n) presplitcoroutine {
+; %y and %alias_phi would all go to the frame, but not %x
+; CHECK: %g.Frame = type { ptr, ptr, i64, ptr, i1 }
+
+; CHECK-LABEL: @g(
+; CHECK: %x = alloca i64, align 8
+; CHECK-NOT: %x.reload.addr = getelementptr inbounds %g.Frame, ptr %hdl, i32 0, i32 2
+; CHECK: %y.reload.addr = getelementptr inbounds %g.Frame, ptr %hdl, i32 0, i32 2
+; CHECK: %alias_phi = phi ptr [ %y.reload.addr, %merge.from.flag_false ], [ %x, %entry ]
+
+
+; The deprecated !coro.outside.frame metadata is parsed but doesn't do anything.
+define ptr @f() {
entry:
%x = alloca i64, !coro.outside.frame !{}
+ ret ptr %x
+}
+
+
+define ptr @g(i1 %n) presplitcoroutine {
+entry:
+ %x = alloca i64
%y = alloca i64
+ call void @llvm.coro.outside.frame(ptr %x)
%id = call token @llvm.coro.id(i32 0, ptr null, ptr null, ptr null)
%size = call i32 @llvm.coro.size.i32()
%alloc = call ptr @malloc(i32 %size)
@@ -37,13 +56,6 @@ suspend:
ret ptr %hdl
}
-; %y and %alias_phi would all go to the frame, but not %x
-; CHECK: %f.Frame = type { ptr, ptr, i64, ptr, i1 }
-; CHECK-LABEL: @f(
-; CHECK: %x = alloca i64, align 8, !coro.outside.frame !0
-; CHECK-NOT: %x.reload.addr = getelementptr inbounds %f.Frame, ptr %hdl, i32 0, i32 2
-; CHECK: %y.reload.addr = getelementptr inbounds %f.Frame, ptr %hdl, i32 0, i32 2
-; CHECK: %alias_phi = phi ptr [ %y.reload.addr, %merge.from.flag_false ], [ %x, %entry ]
declare ptr @llvm.coro.free(token, ptr)
declare i32 @llvm.coro.size.i32()
@@ -58,4 +70,4 @@ declare i1 @llvm.coro.end(ptr, i1, token)
declare void @print(ptr)
declare noalias ptr @malloc(i32)
-declare void @free(ptr)
\ No newline at end of file
+declare void @free(ptr)
diff --git a/llvm/test/Transforms/Mem2Reg/ignore-coro-outside-frame.ll b/llvm/test/Transforms/Mem2Reg/ignore-coro-outside-frame.ll
new file mode 100644
index 0000000000000..ec9309503109d
--- /dev/null
+++ b/llvm/test/Transforms/Mem2Reg/ignore-coro-outside-frame.ll
@@ -0,0 +1,13 @@
+; RUN: opt -passes=mem2reg -S -o - < %s | FileCheck %s
+
+declare void @llvm.coro.outside.frame(ptr)
+
+define void @test() {
+; CHECK: test
+; CHECK-NOT: alloca
+; CHECK-NOT: call void @llvm.coro.outside.frame
+ %A = alloca i32
+ call void @llvm.coro.outside.frame(ptr %A)
+ store i32 1, ptr %A
+ ret void
+}
|
@llvm/pr-subscribers-coroutines Author: Hans Wennborg (zmodem) ChangesMetadata should not be "load bearing", i.e. required for correctness, since optimizations are not required to preserve it. Therefore, turn this into an intrinsic instead. This is a follow-up to #127653. Full diff: https://github.com/llvm/llvm-project/pull/129255.diff 15 Files Affected:
diff --git a/clang/lib/CodeGen/CGCoroutine.cpp b/clang/lib/CodeGen/CGCoroutine.cpp
index a9795c2c0dc8f..7bd3208990572 100644
--- a/clang/lib/CodeGen/CGCoroutine.cpp
+++ b/clang/lib/CodeGen/CGCoroutine.cpp
@@ -709,8 +709,8 @@ struct GetReturnObjectManager {
auto *GroAlloca = dyn_cast_or_null<llvm::AllocaInst>(
GroEmission.getOriginalAllocatedAddress().getPointer());
assert(GroAlloca && "expected alloca to be emitted");
- GroAlloca->setMetadata(llvm::LLVMContext::MD_coro_outside_frame,
- llvm::MDNode::get(CGF.CGM.getLLVMContext(), {}));
+ Builder.CreateCall(
+ CGF.CGM.getIntrinsic(llvm::Intrinsic::coro_outside_frame), {GroAlloca});
// Remember the top of EHStack before emitting the cleanup.
auto old_top = CGF.EHStack.stable_begin();
@@ -863,10 +863,9 @@ void CodeGenFunction::EmitCoroutineBody(const CoroutineBodyStmt &S) {
// in the MSVC C++ ABI, are appropriately destroyed after setting up the
// coroutine.
Address ParmAddr = GetAddrOfLocalVar(Parm);
- if (auto *ParmAlloca =
- dyn_cast<llvm::AllocaInst>(ParmAddr.getBasePointer())) {
- ParmAlloca->setMetadata(llvm::LLVMContext::MD_coro_outside_frame,
- llvm::MDNode::get(CGM.getLLVMContext(), {}));
+ if (auto *PA = dyn_cast<llvm::AllocaInst>(ParmAddr.getBasePointer())) {
+ Builder.CreateCall(
+ CGM.getIntrinsic(llvm::Intrinsic::coro_outside_frame), {PA});
}
}
for (auto *PM : S.getParamMoves()) {
diff --git a/clang/test/CodeGenCoroutines/coro-gro.cpp b/clang/test/CodeGenCoroutines/coro-gro.cpp
index b62134317cef2..bfcfc641dcd72 100644
--- a/clang/test/CodeGenCoroutines/coro-gro.cpp
+++ b/clang/test/CodeGenCoroutines/coro-gro.cpp
@@ -30,11 +30,12 @@ void doSomething() noexcept;
int f() {
// CHECK: %[[RetVal:.+]] = alloca i32
// CHECK: %[[GroActive:.+]] = alloca i1
- // CHECK: %[[CoroGro:.+]] = alloca %struct.GroType, {{.*}} !coro.outside.frame ![[OutFrameMetadata:.+]]
+ // CHECK: %[[CoroGro:.+]] = alloca %struct.GroType
// 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 @llvm.coro.outside.frame(ptr %[[CoroGro]])
// CHECK: call void @_ZNSt16coroutine_traitsIiJEE12promise_typeC1Ev(
// CHECK: call void @_ZNSt16coroutine_traitsIiJEE12promise_type17get_return_objectEv({{.*}} %[[CoroGro]]
// CHECK: store i1 true, ptr %[[GroActive]]
@@ -106,4 +107,3 @@ invoker g() {
// CHECK: call void @_ZN7invoker15invoker_promise17get_return_objectEv({{.*}} %[[AggRes]]
co_return;
}
-// CHECK: ![[OutFrameMetadata]] = !{}
\ No newline at end of file
diff --git a/clang/test/CodeGenCoroutines/coro-params.cpp b/clang/test/CodeGenCoroutines/coro-params.cpp
index 719726cca29c5..792500ce5ac23 100644
--- a/clang/test/CodeGenCoroutines/coro-params.cpp
+++ b/clang/test/CodeGenCoroutines/coro-params.cpp
@@ -72,13 +72,13 @@ void consume(int,int,int,int) noexcept;
// CHECK: define{{.*}} void @_Z1fi8MoveOnly11MoveAndCopy10TrivialABI(i32 noundef %val, ptr noundef %[[MoParam:.+]], ptr noundef %[[McParam:.+]], i32 %[[TrivialParam:.+]]) #0 personality ptr @__gxx_personality_v0
void f(int val, MoveOnly moParam, MoveAndCopy mcParam, TrivialABI trivialParam) {
// CHECK: %[[TrivialAlloca:.+]] = alloca %struct.TrivialABI,
- // CHECK-SAME: !coro.outside.frame
// CHECK: %[[MoCopy:.+]] = alloca %struct.MoveOnly,
// CHECK: %[[McCopy:.+]] = alloca %struct.MoveAndCopy,
// CHECK: %[[TrivialCopy:.+]] = alloca %struct.TrivialABI,
// CHECK: store i32 %val, ptr %[[ValAddr:.+]]
// CHECK: call ptr @llvm.coro.begin(
+ // CHECK: @llvm.coro.outside.frame(ptr %[[TrivialAlloca]])
// 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]]) #
@@ -226,13 +226,12 @@ void consume(int) noexcept;
// may be destroyed before the destructor call.
void msabi(MSParm p) {
// MSABI: define{{.*}} void @"?msabi@@YAXUMSParm@@@Z"(i32 %[[Param:.+]])
-
- // The parameter's local alloca is marked not part of the frame.
// MSABI: %[[ParamAlloca:.+]] = alloca %struct.MSParm
- // MSABI-SAME: !coro.outside.frame
-
// MSABI: %[[ParamCopy:.+]] = alloca %struct.MSParm
+ // The parameter's local alloca is marked not part of the frame.
+ // MSABI: call void @llvm.coro.outside.frame(ptr %[[ParamAlloca]])
+
consume(p.val);
// The parameter's copy is used by the coroutine.
// MSABI: %[[ValPtr:.+]] = getelementptr inbounds nuw %struct.MSParm, ptr %[[ParamCopy]], i32 0, i32 0
diff --git a/llvm/docs/Coroutines.rst b/llvm/docs/Coroutines.rst
index 60e32dc467d27..55bc72026c1ed 100644
--- a/llvm/docs/Coroutines.rst
+++ b/llvm/docs/Coroutines.rst
@@ -1641,6 +1641,37 @@ the function call.
ptr %ctxt, ptr %task, ptr %actor)
unreachable
+
+.. _coro.outside.frame:
+
+'llvm.coro.outside.frame' Intrinsic
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+::
+
+ declare void @llvm.coro.outside.frame(ptr %p)
+
+Overview:
+"""""""""
+
+Signifies that an alloca shouldn't be promoted to the coroutine frame.
+
+Arguments:
+""""""""""
+
+An alloca that should remain outside the coroutine frame.
+
+Semantics:
+""""""""""
+
+Calling `@llvm.coro.outside.frame` with an alloca signifies that the alloca
+should not be promoted to the coroutine frame. This allows the frontend to
+ensure that e.g. callee-destructed parameters are allocated on the stack of the
+ramp function, and thus available until the function returns.
+
+The intrinsic should only be used in presplit coroutines, and is consumed by
+the CoroSplit pass.
+
+
.. _coro.suspend:
.. _suspend points:
@@ -2179,25 +2210,6 @@ or deallocations that may be guarded by `@llvm.coro.alloc` and `@llvm.coro.free`
CoroAnnotationElidePass performs the heap elision when possible. Note that for
recursive or mutually recursive functions this elision is usually not possible.
-Metadata
-========
-
-'``coro.outside.frame``' Metadata
----------------------------------
-
-``coro.outside.frame`` metadata may be attached to an alloca instruction to
-to signify that it shouldn't be promoted to the coroutine frame, useful for
-filtering allocas out by the frontend when emitting internal control mechanisms.
-Additionally, this metadata is only used as a flag, so the associated
-node must be empty.
-
-.. code-block:: text
-
- %__coro_gro = alloca %struct.GroType, align 1, !coro.outside.frame !0
-
- ...
- !0 = !{}
-
Areas Requiring Attention
=========================
#. When coro.suspend returns -1, the coroutine is suspended, and it's possible
diff --git a/llvm/docs/ReleaseNotes.md b/llvm/docs/ReleaseNotes.md
index 12dd09ad41135..24e517f9942f5 100644
--- a/llvm/docs/ReleaseNotes.md
+++ b/llvm/docs/ReleaseNotes.md
@@ -145,6 +145,10 @@ Changes to the CodeGen infrastructure
Changes to the Metadata Info
---------------------------------
+* The `coro.outside.frame` metadata has been replaced by [an intrinsic with the
+ same name](coro.outside.frame). The old metadata is still parsed but has no
+ effect.
+
Changes to the Debug Info
---------------------------------
diff --git a/llvm/include/llvm/IR/FixedMetadataKinds.def b/llvm/include/llvm/IR/FixedMetadataKinds.def
index df572e8791e13..712c139c2d57b 100644
--- a/llvm/include/llvm/IR/FixedMetadataKinds.def
+++ b/llvm/include/llvm/IR/FixedMetadataKinds.def
@@ -50,6 +50,7 @@ LLVM_FIXED_MD_KIND(MD_callsite, "callsite", 35)
LLVM_FIXED_MD_KIND(MD_kcfi_type, "kcfi_type", 36)
LLVM_FIXED_MD_KIND(MD_pcsections, "pcsections", 37)
LLVM_FIXED_MD_KIND(MD_DIAssignID, "DIAssignID", 38)
-LLVM_FIXED_MD_KIND(MD_coro_outside_frame, "coro.outside.frame", 39)
+LLVM_FIXED_MD_KIND(MD_coro_outside_frame_DEPRECATED,
+ "coro.outside.frame", 39) // Preserved for compatibility.
LLVM_FIXED_MD_KIND(MD_mmra, "mmra", 40)
LLVM_FIXED_MD_KIND(MD_noalias_addrspace, "noalias.addrspace", 41)
diff --git a/llvm/include/llvm/IR/Intrinsics.td b/llvm/include/llvm/IR/Intrinsics.td
index 62239ca705b9e..2d90bae3a0b64 100644
--- a/llvm/include/llvm/IR/Intrinsics.td
+++ b/llvm/include/llvm/IR/Intrinsics.td
@@ -1779,6 +1779,7 @@ def int_coro_alloca_alloc : Intrinsic<[llvm_token_ty],
[llvm_anyint_ty, llvm_i32_ty], []>;
def int_coro_alloca_get : Intrinsic<[llvm_ptr_ty], [llvm_token_ty], []>;
def int_coro_alloca_free : Intrinsic<[], [llvm_token_ty], []>;
+def int_coro_outside_frame : Intrinsic<[], [llvm_ptr_ty], [IntrNoMem]>;
// Coroutine Manipulation Intrinsics.
diff --git a/llvm/include/llvm/Transforms/Coroutines/CoroInstr.h b/llvm/include/llvm/Transforms/Coroutines/CoroInstr.h
index fbc76219ead86..f24d7214967a3 100644
--- a/llvm/include/llvm/Transforms/Coroutines/CoroInstr.h
+++ b/llvm/include/llvm/Transforms/Coroutines/CoroInstr.h
@@ -796,6 +796,22 @@ class CoroAllocaFreeInst : public IntrinsicInst {
}
};
+/// This represents the llvm.coro.outside.frame instruction.
+class CoroOutsideFrameInst : public IntrinsicInst {
+ enum { PtrArg };
+
+public:
+ Value *getPtr() const { return getArgOperand(PtrArg); }
+
+ // Methods to support type inquiry through isa, cast, and dyn_cast:
+ static bool classof(const IntrinsicInst *I) {
+ return I->getIntrinsicID() == Intrinsic::coro_outside_frame;
+ }
+ static bool classof(const Value *V) {
+ return isa<IntrinsicInst>(V) && classof(cast<IntrinsicInst>(V));
+ }
+};
+
} // End namespace llvm.
#endif // LLVM_TRANSFORMS_COROUTINES_COROINSTR_H
diff --git a/llvm/include/llvm/Transforms/Coroutines/CoroShape.h b/llvm/include/llvm/Transforms/Coroutines/CoroShape.h
index ea93ced1ce29e..657c2c8bfdc82 100644
--- a/llvm/include/llvm/Transforms/Coroutines/CoroShape.h
+++ b/llvm/include/llvm/Transforms/Coroutines/CoroShape.h
@@ -57,6 +57,7 @@ struct Shape {
SmallVector<AnyCoroSuspendInst *, 4> CoroSuspends;
SmallVector<CoroAwaitSuspendInst *, 4> CoroAwaitSuspends;
SmallVector<CallInst *, 2> SymmetricTransfers;
+ SmallVector<CoroOutsideFrameInst *, 8> OutsideFrames;
// Values invalidated by replaceSwiftErrorOps()
SmallVector<CallInst *, 2> SwiftErrorOps;
@@ -69,6 +70,7 @@ struct Shape {
CoroSuspends.clear();
CoroAwaitSuspends.clear();
SymmetricTransfers.clear();
+ OutsideFrames.clear();
SwiftErrorOps.clear();
diff --git a/llvm/lib/Transforms/Coroutines/CoroSplit.cpp b/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
index e1f767edd6ee1..0a9961d3e06b8 100644
--- a/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
+++ b/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
@@ -2015,7 +2015,13 @@ static void doSplitCoroutine(Function &F, SmallVectorImpl<Function *> &Clones,
simplifySuspendPoints(Shape);
normalizeCoroutine(F, Shape, TTI);
+
ABI.buildCoroutineFrame(OptimizeFrame);
+
+ // @llvm.coro.outside.frame is not needed after the frame has been built.
+ for (Instruction *I : Shape.OutsideFrames)
+ I->eraseFromParent();
+
replaceFrameSizeAndAlignment(Shape);
bool isNoSuspendCoroutine = Shape.CoroSuspends.empty();
diff --git a/llvm/lib/Transforms/Coroutines/Coroutines.cpp b/llvm/lib/Transforms/Coroutines/Coroutines.cpp
index 7b59c39283ded..392f74cd2f52f 100644
--- a/llvm/lib/Transforms/Coroutines/Coroutines.cpp
+++ b/llvm/lib/Transforms/Coroutines/Coroutines.cpp
@@ -286,6 +286,9 @@ void coro::Shape::analyze(Function &F,
}
}
break;
+ case Intrinsic::coro_outside_frame:
+ OutsideFrames.push_back(cast<CoroOutsideFrameInst>(II));
+ break;
}
}
}
diff --git a/llvm/lib/Transforms/Coroutines/SpillUtils.cpp b/llvm/lib/Transforms/Coroutines/SpillUtils.cpp
index 5062ee97a665d..bfe22699f655c 100644
--- a/llvm/lib/Transforms/Coroutines/SpillUtils.cpp
+++ b/llvm/lib/Transforms/Coroutines/SpillUtils.cpp
@@ -417,7 +417,8 @@ struct AllocaUseVisitor : PtrUseVisitor<AllocaUseVisitor> {
static void collectFrameAlloca(AllocaInst *AI, const coro::Shape &Shape,
const SuspendCrossingInfo &Checker,
SmallVectorImpl<AllocaInfo> &Allocas,
- const DominatorTree &DT) {
+ const DominatorTree &DT,
+ const SmallPtrSetImpl<Value*> &OutsideFrameSet) {
if (Shape.CoroSuspends.empty())
return;
@@ -426,9 +427,8 @@ static void collectFrameAlloca(AllocaInst *AI, const coro::Shape &Shape,
if (AI == Shape.SwitchLowering.PromiseAlloca)
return;
- // The __coro_gro alloca should outlive the promise, make sure we
- // keep it outside the frame.
- if (AI->hasMetadata(LLVMContext::MD_coro_outside_frame))
+ // The alloca has been marked as belonging outside the frame.
+ if (OutsideFrameSet.contains(AI))
return;
// The code that uses lifetime.start intrinsic does not work for functions
@@ -464,6 +464,10 @@ void collectSpillsAndAllocasFromInsts(
const SuspendCrossingInfo &Checker, const DominatorTree &DT,
const coro::Shape &Shape) {
+ SmallPtrSet<Value*, 4> OutsideFramePtrs;
+ for (const CoroOutsideFrameInst *I : Shape.OutsideFrames)
+ OutsideFramePtrs.insert(I->getPtr());
+
for (Instruction &I : instructions(F)) {
// Values returned from coroutine structure intrinsics should not be part
// of the Coroutine Frame.
@@ -497,7 +501,7 @@ void collectSpillsAndAllocasFromInsts(
continue;
if (auto *AI = dyn_cast<AllocaInst>(&I)) {
- collectFrameAlloca(AI, Shape, Checker, Allocas, DT);
+ collectFrameAlloca(AI, Shape, Checker, Allocas, DT, OutsideFramePtrs);
continue;
}
diff --git a/llvm/lib/Transforms/Utils/PromoteMemoryToRegister.cpp b/llvm/lib/Transforms/Utils/PromoteMemoryToRegister.cpp
index 05fd989271c32..d6a38a8670c07 100644
--- a/llvm/lib/Transforms/Utils/PromoteMemoryToRegister.cpp
+++ b/llvm/lib/Transforms/Utils/PromoteMemoryToRegister.cpp
@@ -81,7 +81,8 @@ bool llvm::isAllocaPromotable(const AllocaInst *AI) {
return false;
} else if (const IntrinsicInst *II = dyn_cast<IntrinsicInst>(U)) {
if (!II->isLifetimeStartOrEnd() && !II->isDroppable() &&
- II->getIntrinsicID() != Intrinsic::fake_use)
+ II->getIntrinsicID() != Intrinsic::fake_use &&
+ II->getIntrinsicID() != Intrinsic::coro_outside_frame)
return false;
} else if (const BitCastInst *BCI = dyn_cast<BitCastInst>(U)) {
if (!onlyUsedByLifetimeMarkersOrDroppableInsts(BCI))
diff --git a/llvm/test/Transforms/Coroutines/coro-alloca-outside-frame.ll b/llvm/test/Transforms/Coroutines/coro-alloca-outside-frame.ll
index ac6a5752438ce..e227acb082d11 100644
--- a/llvm/test/Transforms/Coroutines/coro-alloca-outside-frame.ll
+++ b/llvm/test/Transforms/Coroutines/coro-alloca-outside-frame.ll
@@ -2,10 +2,29 @@
; RUN: opt < %s -passes='cgscc(coro-split),simplifycfg,early-cse' -S -o %t.ll
; RUN: FileCheck --input-file=%t.ll %s
-define ptr @f(i1 %n) presplitcoroutine {
+; %y and %alias_phi would all go to the frame, but not %x
+; CHECK: %g.Frame = type { ptr, ptr, i64, ptr, i1 }
+
+; CHECK-LABEL: @g(
+; CHECK: %x = alloca i64, align 8
+; CHECK-NOT: %x.reload.addr = getelementptr inbounds %g.Frame, ptr %hdl, i32 0, i32 2
+; CHECK: %y.reload.addr = getelementptr inbounds %g.Frame, ptr %hdl, i32 0, i32 2
+; CHECK: %alias_phi = phi ptr [ %y.reload.addr, %merge.from.flag_false ], [ %x, %entry ]
+
+
+; The deprecated !coro.outside.frame metadata is parsed but doesn't do anything.
+define ptr @f() {
entry:
%x = alloca i64, !coro.outside.frame !{}
+ ret ptr %x
+}
+
+
+define ptr @g(i1 %n) presplitcoroutine {
+entry:
+ %x = alloca i64
%y = alloca i64
+ call void @llvm.coro.outside.frame(ptr %x)
%id = call token @llvm.coro.id(i32 0, ptr null, ptr null, ptr null)
%size = call i32 @llvm.coro.size.i32()
%alloc = call ptr @malloc(i32 %size)
@@ -37,13 +56,6 @@ suspend:
ret ptr %hdl
}
-; %y and %alias_phi would all go to the frame, but not %x
-; CHECK: %f.Frame = type { ptr, ptr, i64, ptr, i1 }
-; CHECK-LABEL: @f(
-; CHECK: %x = alloca i64, align 8, !coro.outside.frame !0
-; CHECK-NOT: %x.reload.addr = getelementptr inbounds %f.Frame, ptr %hdl, i32 0, i32 2
-; CHECK: %y.reload.addr = getelementptr inbounds %f.Frame, ptr %hdl, i32 0, i32 2
-; CHECK: %alias_phi = phi ptr [ %y.reload.addr, %merge.from.flag_false ], [ %x, %entry ]
declare ptr @llvm.coro.free(token, ptr)
declare i32 @llvm.coro.size.i32()
@@ -58,4 +70,4 @@ declare i1 @llvm.coro.end(ptr, i1, token)
declare void @print(ptr)
declare noalias ptr @malloc(i32)
-declare void @free(ptr)
\ No newline at end of file
+declare void @free(ptr)
diff --git a/llvm/test/Transforms/Mem2Reg/ignore-coro-outside-frame.ll b/llvm/test/Transforms/Mem2Reg/ignore-coro-outside-frame.ll
new file mode 100644
index 0000000000000..ec9309503109d
--- /dev/null
+++ b/llvm/test/Transforms/Mem2Reg/ignore-coro-outside-frame.ll
@@ -0,0 +1,13 @@
+; RUN: opt -passes=mem2reg -S -o - < %s | FileCheck %s
+
+declare void @llvm.coro.outside.frame(ptr)
+
+define void @test() {
+; CHECK: test
+; CHECK-NOT: alloca
+; CHECK-NOT: call void @llvm.coro.outside.frame
+ %A = alloca i32
+ call void @llvm.coro.outside.frame(ptr %A)
+ store i32 1, ptr %A
+ ret void
+}
|
@llvm/pr-subscribers-clang Author: Hans Wennborg (zmodem) ChangesMetadata should not be "load bearing", i.e. required for correctness, since optimizations are not required to preserve it. Therefore, turn this into an intrinsic instead. This is a follow-up to #127653. Full diff: https://github.com/llvm/llvm-project/pull/129255.diff 15 Files Affected:
diff --git a/clang/lib/CodeGen/CGCoroutine.cpp b/clang/lib/CodeGen/CGCoroutine.cpp
index a9795c2c0dc8f..7bd3208990572 100644
--- a/clang/lib/CodeGen/CGCoroutine.cpp
+++ b/clang/lib/CodeGen/CGCoroutine.cpp
@@ -709,8 +709,8 @@ struct GetReturnObjectManager {
auto *GroAlloca = dyn_cast_or_null<llvm::AllocaInst>(
GroEmission.getOriginalAllocatedAddress().getPointer());
assert(GroAlloca && "expected alloca to be emitted");
- GroAlloca->setMetadata(llvm::LLVMContext::MD_coro_outside_frame,
- llvm::MDNode::get(CGF.CGM.getLLVMContext(), {}));
+ Builder.CreateCall(
+ CGF.CGM.getIntrinsic(llvm::Intrinsic::coro_outside_frame), {GroAlloca});
// Remember the top of EHStack before emitting the cleanup.
auto old_top = CGF.EHStack.stable_begin();
@@ -863,10 +863,9 @@ void CodeGenFunction::EmitCoroutineBody(const CoroutineBodyStmt &S) {
// in the MSVC C++ ABI, are appropriately destroyed after setting up the
// coroutine.
Address ParmAddr = GetAddrOfLocalVar(Parm);
- if (auto *ParmAlloca =
- dyn_cast<llvm::AllocaInst>(ParmAddr.getBasePointer())) {
- ParmAlloca->setMetadata(llvm::LLVMContext::MD_coro_outside_frame,
- llvm::MDNode::get(CGM.getLLVMContext(), {}));
+ if (auto *PA = dyn_cast<llvm::AllocaInst>(ParmAddr.getBasePointer())) {
+ Builder.CreateCall(
+ CGM.getIntrinsic(llvm::Intrinsic::coro_outside_frame), {PA});
}
}
for (auto *PM : S.getParamMoves()) {
diff --git a/clang/test/CodeGenCoroutines/coro-gro.cpp b/clang/test/CodeGenCoroutines/coro-gro.cpp
index b62134317cef2..bfcfc641dcd72 100644
--- a/clang/test/CodeGenCoroutines/coro-gro.cpp
+++ b/clang/test/CodeGenCoroutines/coro-gro.cpp
@@ -30,11 +30,12 @@ void doSomething() noexcept;
int f() {
// CHECK: %[[RetVal:.+]] = alloca i32
// CHECK: %[[GroActive:.+]] = alloca i1
- // CHECK: %[[CoroGro:.+]] = alloca %struct.GroType, {{.*}} !coro.outside.frame ![[OutFrameMetadata:.+]]
+ // CHECK: %[[CoroGro:.+]] = alloca %struct.GroType
// 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 @llvm.coro.outside.frame(ptr %[[CoroGro]])
// CHECK: call void @_ZNSt16coroutine_traitsIiJEE12promise_typeC1Ev(
// CHECK: call void @_ZNSt16coroutine_traitsIiJEE12promise_type17get_return_objectEv({{.*}} %[[CoroGro]]
// CHECK: store i1 true, ptr %[[GroActive]]
@@ -106,4 +107,3 @@ invoker g() {
// CHECK: call void @_ZN7invoker15invoker_promise17get_return_objectEv({{.*}} %[[AggRes]]
co_return;
}
-// CHECK: ![[OutFrameMetadata]] = !{}
\ No newline at end of file
diff --git a/clang/test/CodeGenCoroutines/coro-params.cpp b/clang/test/CodeGenCoroutines/coro-params.cpp
index 719726cca29c5..792500ce5ac23 100644
--- a/clang/test/CodeGenCoroutines/coro-params.cpp
+++ b/clang/test/CodeGenCoroutines/coro-params.cpp
@@ -72,13 +72,13 @@ void consume(int,int,int,int) noexcept;
// CHECK: define{{.*}} void @_Z1fi8MoveOnly11MoveAndCopy10TrivialABI(i32 noundef %val, ptr noundef %[[MoParam:.+]], ptr noundef %[[McParam:.+]], i32 %[[TrivialParam:.+]]) #0 personality ptr @__gxx_personality_v0
void f(int val, MoveOnly moParam, MoveAndCopy mcParam, TrivialABI trivialParam) {
// CHECK: %[[TrivialAlloca:.+]] = alloca %struct.TrivialABI,
- // CHECK-SAME: !coro.outside.frame
// CHECK: %[[MoCopy:.+]] = alloca %struct.MoveOnly,
// CHECK: %[[McCopy:.+]] = alloca %struct.MoveAndCopy,
// CHECK: %[[TrivialCopy:.+]] = alloca %struct.TrivialABI,
// CHECK: store i32 %val, ptr %[[ValAddr:.+]]
// CHECK: call ptr @llvm.coro.begin(
+ // CHECK: @llvm.coro.outside.frame(ptr %[[TrivialAlloca]])
// 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]]) #
@@ -226,13 +226,12 @@ void consume(int) noexcept;
// may be destroyed before the destructor call.
void msabi(MSParm p) {
// MSABI: define{{.*}} void @"?msabi@@YAXUMSParm@@@Z"(i32 %[[Param:.+]])
-
- // The parameter's local alloca is marked not part of the frame.
// MSABI: %[[ParamAlloca:.+]] = alloca %struct.MSParm
- // MSABI-SAME: !coro.outside.frame
-
// MSABI: %[[ParamCopy:.+]] = alloca %struct.MSParm
+ // The parameter's local alloca is marked not part of the frame.
+ // MSABI: call void @llvm.coro.outside.frame(ptr %[[ParamAlloca]])
+
consume(p.val);
// The parameter's copy is used by the coroutine.
// MSABI: %[[ValPtr:.+]] = getelementptr inbounds nuw %struct.MSParm, ptr %[[ParamCopy]], i32 0, i32 0
diff --git a/llvm/docs/Coroutines.rst b/llvm/docs/Coroutines.rst
index 60e32dc467d27..55bc72026c1ed 100644
--- a/llvm/docs/Coroutines.rst
+++ b/llvm/docs/Coroutines.rst
@@ -1641,6 +1641,37 @@ the function call.
ptr %ctxt, ptr %task, ptr %actor)
unreachable
+
+.. _coro.outside.frame:
+
+'llvm.coro.outside.frame' Intrinsic
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+::
+
+ declare void @llvm.coro.outside.frame(ptr %p)
+
+Overview:
+"""""""""
+
+Signifies that an alloca shouldn't be promoted to the coroutine frame.
+
+Arguments:
+""""""""""
+
+An alloca that should remain outside the coroutine frame.
+
+Semantics:
+""""""""""
+
+Calling `@llvm.coro.outside.frame` with an alloca signifies that the alloca
+should not be promoted to the coroutine frame. This allows the frontend to
+ensure that e.g. callee-destructed parameters are allocated on the stack of the
+ramp function, and thus available until the function returns.
+
+The intrinsic should only be used in presplit coroutines, and is consumed by
+the CoroSplit pass.
+
+
.. _coro.suspend:
.. _suspend points:
@@ -2179,25 +2210,6 @@ or deallocations that may be guarded by `@llvm.coro.alloc` and `@llvm.coro.free`
CoroAnnotationElidePass performs the heap elision when possible. Note that for
recursive or mutually recursive functions this elision is usually not possible.
-Metadata
-========
-
-'``coro.outside.frame``' Metadata
----------------------------------
-
-``coro.outside.frame`` metadata may be attached to an alloca instruction to
-to signify that it shouldn't be promoted to the coroutine frame, useful for
-filtering allocas out by the frontend when emitting internal control mechanisms.
-Additionally, this metadata is only used as a flag, so the associated
-node must be empty.
-
-.. code-block:: text
-
- %__coro_gro = alloca %struct.GroType, align 1, !coro.outside.frame !0
-
- ...
- !0 = !{}
-
Areas Requiring Attention
=========================
#. When coro.suspend returns -1, the coroutine is suspended, and it's possible
diff --git a/llvm/docs/ReleaseNotes.md b/llvm/docs/ReleaseNotes.md
index 12dd09ad41135..24e517f9942f5 100644
--- a/llvm/docs/ReleaseNotes.md
+++ b/llvm/docs/ReleaseNotes.md
@@ -145,6 +145,10 @@ Changes to the CodeGen infrastructure
Changes to the Metadata Info
---------------------------------
+* The `coro.outside.frame` metadata has been replaced by [an intrinsic with the
+ same name](coro.outside.frame). The old metadata is still parsed but has no
+ effect.
+
Changes to the Debug Info
---------------------------------
diff --git a/llvm/include/llvm/IR/FixedMetadataKinds.def b/llvm/include/llvm/IR/FixedMetadataKinds.def
index df572e8791e13..712c139c2d57b 100644
--- a/llvm/include/llvm/IR/FixedMetadataKinds.def
+++ b/llvm/include/llvm/IR/FixedMetadataKinds.def
@@ -50,6 +50,7 @@ LLVM_FIXED_MD_KIND(MD_callsite, "callsite", 35)
LLVM_FIXED_MD_KIND(MD_kcfi_type, "kcfi_type", 36)
LLVM_FIXED_MD_KIND(MD_pcsections, "pcsections", 37)
LLVM_FIXED_MD_KIND(MD_DIAssignID, "DIAssignID", 38)
-LLVM_FIXED_MD_KIND(MD_coro_outside_frame, "coro.outside.frame", 39)
+LLVM_FIXED_MD_KIND(MD_coro_outside_frame_DEPRECATED,
+ "coro.outside.frame", 39) // Preserved for compatibility.
LLVM_FIXED_MD_KIND(MD_mmra, "mmra", 40)
LLVM_FIXED_MD_KIND(MD_noalias_addrspace, "noalias.addrspace", 41)
diff --git a/llvm/include/llvm/IR/Intrinsics.td b/llvm/include/llvm/IR/Intrinsics.td
index 62239ca705b9e..2d90bae3a0b64 100644
--- a/llvm/include/llvm/IR/Intrinsics.td
+++ b/llvm/include/llvm/IR/Intrinsics.td
@@ -1779,6 +1779,7 @@ def int_coro_alloca_alloc : Intrinsic<[llvm_token_ty],
[llvm_anyint_ty, llvm_i32_ty], []>;
def int_coro_alloca_get : Intrinsic<[llvm_ptr_ty], [llvm_token_ty], []>;
def int_coro_alloca_free : Intrinsic<[], [llvm_token_ty], []>;
+def int_coro_outside_frame : Intrinsic<[], [llvm_ptr_ty], [IntrNoMem]>;
// Coroutine Manipulation Intrinsics.
diff --git a/llvm/include/llvm/Transforms/Coroutines/CoroInstr.h b/llvm/include/llvm/Transforms/Coroutines/CoroInstr.h
index fbc76219ead86..f24d7214967a3 100644
--- a/llvm/include/llvm/Transforms/Coroutines/CoroInstr.h
+++ b/llvm/include/llvm/Transforms/Coroutines/CoroInstr.h
@@ -796,6 +796,22 @@ class CoroAllocaFreeInst : public IntrinsicInst {
}
};
+/// This represents the llvm.coro.outside.frame instruction.
+class CoroOutsideFrameInst : public IntrinsicInst {
+ enum { PtrArg };
+
+public:
+ Value *getPtr() const { return getArgOperand(PtrArg); }
+
+ // Methods to support type inquiry through isa, cast, and dyn_cast:
+ static bool classof(const IntrinsicInst *I) {
+ return I->getIntrinsicID() == Intrinsic::coro_outside_frame;
+ }
+ static bool classof(const Value *V) {
+ return isa<IntrinsicInst>(V) && classof(cast<IntrinsicInst>(V));
+ }
+};
+
} // End namespace llvm.
#endif // LLVM_TRANSFORMS_COROUTINES_COROINSTR_H
diff --git a/llvm/include/llvm/Transforms/Coroutines/CoroShape.h b/llvm/include/llvm/Transforms/Coroutines/CoroShape.h
index ea93ced1ce29e..657c2c8bfdc82 100644
--- a/llvm/include/llvm/Transforms/Coroutines/CoroShape.h
+++ b/llvm/include/llvm/Transforms/Coroutines/CoroShape.h
@@ -57,6 +57,7 @@ struct Shape {
SmallVector<AnyCoroSuspendInst *, 4> CoroSuspends;
SmallVector<CoroAwaitSuspendInst *, 4> CoroAwaitSuspends;
SmallVector<CallInst *, 2> SymmetricTransfers;
+ SmallVector<CoroOutsideFrameInst *, 8> OutsideFrames;
// Values invalidated by replaceSwiftErrorOps()
SmallVector<CallInst *, 2> SwiftErrorOps;
@@ -69,6 +70,7 @@ struct Shape {
CoroSuspends.clear();
CoroAwaitSuspends.clear();
SymmetricTransfers.clear();
+ OutsideFrames.clear();
SwiftErrorOps.clear();
diff --git a/llvm/lib/Transforms/Coroutines/CoroSplit.cpp b/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
index e1f767edd6ee1..0a9961d3e06b8 100644
--- a/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
+++ b/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
@@ -2015,7 +2015,13 @@ static void doSplitCoroutine(Function &F, SmallVectorImpl<Function *> &Clones,
simplifySuspendPoints(Shape);
normalizeCoroutine(F, Shape, TTI);
+
ABI.buildCoroutineFrame(OptimizeFrame);
+
+ // @llvm.coro.outside.frame is not needed after the frame has been built.
+ for (Instruction *I : Shape.OutsideFrames)
+ I->eraseFromParent();
+
replaceFrameSizeAndAlignment(Shape);
bool isNoSuspendCoroutine = Shape.CoroSuspends.empty();
diff --git a/llvm/lib/Transforms/Coroutines/Coroutines.cpp b/llvm/lib/Transforms/Coroutines/Coroutines.cpp
index 7b59c39283ded..392f74cd2f52f 100644
--- a/llvm/lib/Transforms/Coroutines/Coroutines.cpp
+++ b/llvm/lib/Transforms/Coroutines/Coroutines.cpp
@@ -286,6 +286,9 @@ void coro::Shape::analyze(Function &F,
}
}
break;
+ case Intrinsic::coro_outside_frame:
+ OutsideFrames.push_back(cast<CoroOutsideFrameInst>(II));
+ break;
}
}
}
diff --git a/llvm/lib/Transforms/Coroutines/SpillUtils.cpp b/llvm/lib/Transforms/Coroutines/SpillUtils.cpp
index 5062ee97a665d..bfe22699f655c 100644
--- a/llvm/lib/Transforms/Coroutines/SpillUtils.cpp
+++ b/llvm/lib/Transforms/Coroutines/SpillUtils.cpp
@@ -417,7 +417,8 @@ struct AllocaUseVisitor : PtrUseVisitor<AllocaUseVisitor> {
static void collectFrameAlloca(AllocaInst *AI, const coro::Shape &Shape,
const SuspendCrossingInfo &Checker,
SmallVectorImpl<AllocaInfo> &Allocas,
- const DominatorTree &DT) {
+ const DominatorTree &DT,
+ const SmallPtrSetImpl<Value*> &OutsideFrameSet) {
if (Shape.CoroSuspends.empty())
return;
@@ -426,9 +427,8 @@ static void collectFrameAlloca(AllocaInst *AI, const coro::Shape &Shape,
if (AI == Shape.SwitchLowering.PromiseAlloca)
return;
- // The __coro_gro alloca should outlive the promise, make sure we
- // keep it outside the frame.
- if (AI->hasMetadata(LLVMContext::MD_coro_outside_frame))
+ // The alloca has been marked as belonging outside the frame.
+ if (OutsideFrameSet.contains(AI))
return;
// The code that uses lifetime.start intrinsic does not work for functions
@@ -464,6 +464,10 @@ void collectSpillsAndAllocasFromInsts(
const SuspendCrossingInfo &Checker, const DominatorTree &DT,
const coro::Shape &Shape) {
+ SmallPtrSet<Value*, 4> OutsideFramePtrs;
+ for (const CoroOutsideFrameInst *I : Shape.OutsideFrames)
+ OutsideFramePtrs.insert(I->getPtr());
+
for (Instruction &I : instructions(F)) {
// Values returned from coroutine structure intrinsics should not be part
// of the Coroutine Frame.
@@ -497,7 +501,7 @@ void collectSpillsAndAllocasFromInsts(
continue;
if (auto *AI = dyn_cast<AllocaInst>(&I)) {
- collectFrameAlloca(AI, Shape, Checker, Allocas, DT);
+ collectFrameAlloca(AI, Shape, Checker, Allocas, DT, OutsideFramePtrs);
continue;
}
diff --git a/llvm/lib/Transforms/Utils/PromoteMemoryToRegister.cpp b/llvm/lib/Transforms/Utils/PromoteMemoryToRegister.cpp
index 05fd989271c32..d6a38a8670c07 100644
--- a/llvm/lib/Transforms/Utils/PromoteMemoryToRegister.cpp
+++ b/llvm/lib/Transforms/Utils/PromoteMemoryToRegister.cpp
@@ -81,7 +81,8 @@ bool llvm::isAllocaPromotable(const AllocaInst *AI) {
return false;
} else if (const IntrinsicInst *II = dyn_cast<IntrinsicInst>(U)) {
if (!II->isLifetimeStartOrEnd() && !II->isDroppable() &&
- II->getIntrinsicID() != Intrinsic::fake_use)
+ II->getIntrinsicID() != Intrinsic::fake_use &&
+ II->getIntrinsicID() != Intrinsic::coro_outside_frame)
return false;
} else if (const BitCastInst *BCI = dyn_cast<BitCastInst>(U)) {
if (!onlyUsedByLifetimeMarkersOrDroppableInsts(BCI))
diff --git a/llvm/test/Transforms/Coroutines/coro-alloca-outside-frame.ll b/llvm/test/Transforms/Coroutines/coro-alloca-outside-frame.ll
index ac6a5752438ce..e227acb082d11 100644
--- a/llvm/test/Transforms/Coroutines/coro-alloca-outside-frame.ll
+++ b/llvm/test/Transforms/Coroutines/coro-alloca-outside-frame.ll
@@ -2,10 +2,29 @@
; RUN: opt < %s -passes='cgscc(coro-split),simplifycfg,early-cse' -S -o %t.ll
; RUN: FileCheck --input-file=%t.ll %s
-define ptr @f(i1 %n) presplitcoroutine {
+; %y and %alias_phi would all go to the frame, but not %x
+; CHECK: %g.Frame = type { ptr, ptr, i64, ptr, i1 }
+
+; CHECK-LABEL: @g(
+; CHECK: %x = alloca i64, align 8
+; CHECK-NOT: %x.reload.addr = getelementptr inbounds %g.Frame, ptr %hdl, i32 0, i32 2
+; CHECK: %y.reload.addr = getelementptr inbounds %g.Frame, ptr %hdl, i32 0, i32 2
+; CHECK: %alias_phi = phi ptr [ %y.reload.addr, %merge.from.flag_false ], [ %x, %entry ]
+
+
+; The deprecated !coro.outside.frame metadata is parsed but doesn't do anything.
+define ptr @f() {
entry:
%x = alloca i64, !coro.outside.frame !{}
+ ret ptr %x
+}
+
+
+define ptr @g(i1 %n) presplitcoroutine {
+entry:
+ %x = alloca i64
%y = alloca i64
+ call void @llvm.coro.outside.frame(ptr %x)
%id = call token @llvm.coro.id(i32 0, ptr null, ptr null, ptr null)
%size = call i32 @llvm.coro.size.i32()
%alloc = call ptr @malloc(i32 %size)
@@ -37,13 +56,6 @@ suspend:
ret ptr %hdl
}
-; %y and %alias_phi would all go to the frame, but not %x
-; CHECK: %f.Frame = type { ptr, ptr, i64, ptr, i1 }
-; CHECK-LABEL: @f(
-; CHECK: %x = alloca i64, align 8, !coro.outside.frame !0
-; CHECK-NOT: %x.reload.addr = getelementptr inbounds %f.Frame, ptr %hdl, i32 0, i32 2
-; CHECK: %y.reload.addr = getelementptr inbounds %f.Frame, ptr %hdl, i32 0, i32 2
-; CHECK: %alias_phi = phi ptr [ %y.reload.addr, %merge.from.flag_false ], [ %x, %entry ]
declare ptr @llvm.coro.free(token, ptr)
declare i32 @llvm.coro.size.i32()
@@ -58,4 +70,4 @@ declare i1 @llvm.coro.end(ptr, i1, token)
declare void @print(ptr)
declare noalias ptr @malloc(i32)
-declare void @free(ptr)
\ No newline at end of file
+declare void @free(ptr)
diff --git a/llvm/test/Transforms/Mem2Reg/ignore-coro-outside-frame.ll b/llvm/test/Transforms/Mem2Reg/ignore-coro-outside-frame.ll
new file mode 100644
index 0000000000000..ec9309503109d
--- /dev/null
+++ b/llvm/test/Transforms/Mem2Reg/ignore-coro-outside-frame.ll
@@ -0,0 +1,13 @@
+; RUN: opt -passes=mem2reg -S -o - < %s | FileCheck %s
+
+declare void @llvm.coro.outside.frame(ptr)
+
+define void @test() {
+; CHECK: test
+; CHECK-NOT: alloca
+; CHECK-NOT: call void @llvm.coro.outside.frame
+ %A = alloca i32
+ call void @llvm.coro.outside.frame(ptr %A)
+ store i32 1, ptr %A
+ ret void
+}
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
This is a follow-up to the discussion on the previous PR:
|
(sorry, wrong button) |
Ping? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am still concerning about the potential performance loss on using the intrinsic on parameters. Did you have any insight?
Metadata should not be "load bearing", i.e. required for correctness, since optimizations are not required to preserve it. Therefore, turn this into an intrinsic instead.
This is a follow-up to #127653.