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

Conversation

zmodem
Copy link
Collaborator

@zmodem zmodem commented Feb 28, 2025

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.

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.
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:codegen IR generation bugs: mangling, exceptions, etc. coroutines C++20 coroutines llvm:ir llvm:transforms labels Feb 28, 2025
@llvmbot
Copy link
Member

llvmbot commented Feb 28, 2025

@llvm/pr-subscribers-llvm-ir

@llvm/pr-subscribers-clang-codegen

Author: Hans Wennborg (zmodem)

Changes

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 --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
+}

@llvmbot
Copy link
Member

llvmbot commented Feb 28, 2025

@llvm/pr-subscribers-coroutines

Author: Hans Wennborg (zmodem)

Changes

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 --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
+}

@llvmbot
Copy link
Member

llvmbot commented Feb 28, 2025

@llvm/pr-subscribers-clang

Author: Hans Wennborg (zmodem)

Changes

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 --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
+}

Copy link

github-actions bot commented Feb 28, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

@zmodem
Copy link
Collaborator Author

zmodem commented Feb 28, 2025

This is a follow-up to the discussion on the previous PR:

  • I think that discussion ended with the conclusion that we need an explicit mechanism for marking out-of-frame allocas.
  • We agree it should not be metadata
  • I think an intrinsic may be better than a flag directly on the alloca instruction (like inalloca), because we would need to teach optimizations about such a flag. For example, that an "out-of-frame" alloca is not replaceable with a regular alloca.
  • I'm not entirely sure how to deprecate a metadata. I think we need to parse it for backwards compatibility, at least at the bitcode level?

@zmodem zmodem closed this Feb 28, 2025
@zmodem
Copy link
Collaborator Author

zmodem commented Feb 28, 2025

(sorry, wrong button)

@zmodem zmodem reopened this Feb 28, 2025
@zmodem
Copy link
Collaborator Author

zmodem commented Mar 10, 2025

Ping?

Copy link
Member

@ChuanqiXu9 ChuanqiXu9 left a 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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:codegen IR generation bugs: mangling, exceptions, etc. clang Clang issues not falling into any other category coroutines C++20 coroutines llvm:ir llvm:transforms
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants