[clang] [llvm] [Coroutines] Replace coro.outside.frame metadata with an intrinsic (PR #129255)
via llvm-commits
llvm-commits at lists.llvm.org
Fri Feb 28 06:46:26 PST 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: Hans Wennborg (zmodem)
<details>
<summary>Changes</summary>
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.
---
Full diff: https://github.com/llvm/llvm-project/pull/129255.diff
15 Files Affected:
- (modified) clang/lib/CodeGen/CGCoroutine.cpp (+5-6)
- (modified) clang/test/CodeGenCoroutines/coro-gro.cpp (+2-2)
- (modified) clang/test/CodeGenCoroutines/coro-params.cpp (+4-5)
- (modified) llvm/docs/Coroutines.rst (+31-19)
- (modified) llvm/docs/ReleaseNotes.md (+4)
- (modified) llvm/include/llvm/IR/FixedMetadataKinds.def (+2-1)
- (modified) llvm/include/llvm/IR/Intrinsics.td (+1)
- (modified) llvm/include/llvm/Transforms/Coroutines/CoroInstr.h (+16)
- (modified) llvm/include/llvm/Transforms/Coroutines/CoroShape.h (+2)
- (modified) llvm/lib/Transforms/Coroutines/CoroSplit.cpp (+6)
- (modified) llvm/lib/Transforms/Coroutines/Coroutines.cpp (+3)
- (modified) llvm/lib/Transforms/Coroutines/SpillUtils.cpp (+9-5)
- (modified) llvm/lib/Transforms/Utils/PromoteMemoryToRegister.cpp (+2-1)
- (modified) llvm/test/Transforms/Coroutines/coro-alloca-outside-frame.ll (+21-9)
- (added) llvm/test/Transforms/Mem2Reg/ignore-coro-outside-frame.ll (+13)
``````````diff
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
+}
``````````
</details>
https://github.com/llvm/llvm-project/pull/129255
More information about the llvm-commits
mailing list