[Coroutines] Replace coro.outside.frame metadata with an intrinsic

zmodem · zmodem · commit c1fe663ceacd · 2025-02-28T15:43:43.000+01:00
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 #127653.
diff --git 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
@@ -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]] = !{}
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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)
+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
@@ -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
+}
