Skip to content

[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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 5 additions & 6 deletions clang/lib/CodeGen/CGCoroutine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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()) {
Expand Down
4 changes: 2 additions & 2 deletions clang/test/CodeGenCoroutines/coro-gro.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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]]
Expand Down Expand Up @@ -106,4 +107,3 @@ invoker g() {
// CHECK: call void @_ZN7invoker15invoker_promise17get_return_objectEv({{.*}} %[[AggRes]]
co_return;
}
// CHECK: ![[OutFrameMetadata]] = !{}
9 changes: 4 additions & 5 deletions clang/test/CodeGenCoroutines/coro-params.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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]]) #
Expand Down Expand Up @@ -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
Expand Down
50 changes: 31 additions & 19 deletions llvm/docs/Coroutines.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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:

Expand Down Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions llvm/docs/ReleaseNotes.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
---------------------------------

Expand Down
3 changes: 2 additions & 1 deletion llvm/include/llvm/IR/FixedMetadataKinds.def
Original file line number Diff line number Diff line change
Expand Up @@ -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)
1 change: 1 addition & 0 deletions llvm/include/llvm/IR/Intrinsics.td
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
16 changes: 16 additions & 0 deletions llvm/include/llvm/Transforms/Coroutines/CoroInstr.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 2 additions & 0 deletions llvm/include/llvm/Transforms/Coroutines/CoroShape.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -69,6 +70,7 @@ struct Shape {
CoroSuspends.clear();
CoroAwaitSuspends.clear();
SymmetricTransfers.clear();
OutsideFrames.clear();

SwiftErrorOps.clear();

Expand Down
6 changes: 6 additions & 0 deletions llvm/lib/Transforms/Coroutines/CoroSplit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
3 changes: 3 additions & 0 deletions llvm/lib/Transforms/Coroutines/Coroutines.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,9 @@ void coro::Shape::analyze(Function &F,
}
}
break;
case Intrinsic::coro_outside_frame:
OutsideFrames.push_back(cast<CoroOutsideFrameInst>(II));
break;
}
}
}
Expand Down
19 changes: 11 additions & 8 deletions llvm/lib/Transforms/Coroutines/SpillUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -414,10 +414,10 @@ struct AllocaUseVisitor : PtrUseVisitor<AllocaUseVisitor> {
};
} // namespace

static void collectFrameAlloca(AllocaInst *AI, const coro::Shape &Shape,
const SuspendCrossingInfo &Checker,
SmallVectorImpl<AllocaInfo> &Allocas,
const DominatorTree &DT) {
static void collectFrameAlloca(
AllocaInst *AI, const coro::Shape &Shape,
const SuspendCrossingInfo &Checker, SmallVectorImpl<AllocaInfo> &Allocas,
const DominatorTree &DT, const SmallPtrSetImpl<Value *> &OutsideFrameSet) {
if (Shape.CoroSuspends.empty())
return;

Expand All @@ -426,9 +426,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
Expand Down Expand Up @@ -464,6 +463,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.
Expand Down Expand Up @@ -497,7 +500,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;
}

Expand Down
3 changes: 2 additions & 1 deletion llvm/lib/Transforms/Utils/PromoteMemoryToRegister.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
30 changes: 21 additions & 9 deletions llvm/test/Transforms/Coroutines/coro-alloca-outside-frame.ll
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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()
Expand All @@ -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)
declare void @free(ptr)
13 changes: 13 additions & 0 deletions llvm/test/Transforms/Mem2Reg/ignore-coro-outside-frame.ll
Original file line number Diff line number Diff line change
@@ -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
}
Loading