Reapply "[Coroutines] Add llvm.coro.is_in_ramp and drop return value of llvm.coro.end #153404"

clang/lib/CodeGen/CGCoroutine.cpp

@@ -575,17 +575,19 @@ struct CallCoroEnd final : public EHScopeStack::Cleanup {
     llvm::Function *CoroEndFn = CGM.getIntrinsic(llvm::Intrinsic::coro_end);
     // See if we have a funclet bundle to associate coro.end with. (WinEH)
     auto Bundles = getBundlesForCoroEnd(CGF);
-    auto *CoroEnd =
-      CGF.Builder.CreateCall(CoroEndFn,
-                             {NullPtr, CGF.Builder.getTrue(),
-                              llvm::ConstantTokenNone::get(CoroEndFn->getContext())},
-                             Bundles);
+    CGF.Builder.CreateCall(
+        CoroEndFn,
+        {NullPtr, CGF.Builder.getTrue(),
+         llvm::ConstantTokenNone::get(CoroEndFn->getContext())},
+        Bundles);
     if (Bundles.empty()) {
       // Otherwise, (landingpad model), create a conditional branch that leads
       // either to a cleanup block or a block with EH resume instruction.
       auto *ResumeBB = CGF.getEHResumeBlock(/*isCleanup=*/true);
       auto *CleanupContBB = CGF.createBasicBlock("cleanup.cont");
-      CGF.Builder.CreateCondBr(CoroEnd, ResumeBB, CleanupContBB);
+      auto *CoroIsInRampFn = CGM.getIntrinsic(llvm::Intrinsic::coro_is_in_ramp);
+      auto *CoroIsInRamp = CGF.Builder.CreateCall(CoroIsInRampFn);
+      CGF.Builder.CreateCondBr(CoroIsInRamp, CleanupContBB, ResumeBB);
       CGF.EmitBlock(CleanupContBB);
     }
   }

clang/test/CodeGenCoroutines/coro-builtins.c

@@ -37,7 +37,7 @@ void f(int n) {
   // CHECK-NEXT: call ptr @llvm.coro.free(token %[[COROID]], ptr %[[FRAME]])
   __builtin_coro_free(__builtin_coro_frame());
 
-  // CHECK-NEXT: call i1  @llvm.coro.end(ptr %[[FRAME]], i1 false, token none)
+  // CHECK-NEXT: call void  @llvm.coro.end(ptr %[[FRAME]], i1 false, token none)
   __builtin_coro_end(__builtin_coro_frame(), 0);
 
   // CHECK-NEXT: call i8 @llvm.coro.suspend(token none, i1 true)

clang/test/CodeGenCoroutines/coro-eh-cleanup.cpp

@@ -60,7 +60,7 @@ coro_t f() {
 
 // CHECK: [[COROENDBB]]:
 // CHECK-NEXT: %[[CLPAD:.+]] = cleanuppad within none
-// CHECK-NEXT: call i1 @llvm.coro.end(ptr null, i1 true, token none) [ "funclet"(token %[[CLPAD]]) ]
+// CHECK-NEXT: call void @llvm.coro.end(ptr null, i1 true, token none) [ "funclet"(token %[[CLPAD]]) ]
 // CHECK-NEXT: cleanupret from %[[CLPAD]] unwind label
 
 // CHECK-LPAD: @_Z1fv(
@@ -76,8 +76,8 @@ coro_t f() {
 // CHECK-LPAD:             to label %{{.+}} unwind label %[[UNWINDBB:.+]]
 
 // CHECK-LPAD: [[UNWINDBB]]:
-// CHECK-LPAD:   %[[I1RESUME:.+]] = call i1 @llvm.coro.end(ptr null, i1 true, token none)
-// CHECK-LPAD:   br i1  %[[I1RESUME]], label %[[EHRESUME:.+]], label
+// CHECK-LPAD:   %[[InRamp:.+]] = call i1 @llvm.coro.is_in_ramp()
+// CHECK-LPAD:   br i1  %[[InRamp]], label %{{.+}}, label %[[EHRESUME:.+]]
 // CHECK-LPAD: [[EHRESUME]]:
 // CHECK-LPAD-NEXT:  %[[exn:.+]] = load ptr, ptr %exn.slot, align 8
 // CHECK-LPAD-NEXT:  %[[sel:.+]] = load i32, ptr %ehselector.slot, align 4

clang/test/CodeGenCoroutines/coro-lambda.cpp

@@ -55,4 +55,4 @@ void f() {
 //   CHECK: alloca %"struct.Task::promise_type"
 //   CHECK: call token @llvm.coro.id(
 //   CHECK: call i8 @llvm.coro.suspend(
-//   CHECK: call i1 @llvm.coro.end(
+//   CHECK: call void @llvm.coro.end(

clang/test/CodeGenCoroutines/coro-params.cpp

@@ -117,7 +117,7 @@ void f(int val, MoveOnly moParam, MoveAndCopy mcParam, TrivialABI trivialParam)
   // CHECK-NEXT: call ptr @llvm.coro.free(
 
   // The original trivial_abi parameter is destroyed when returning from the ramp.
-  // CHECK: call i1 @llvm.coro.end
+  // CHECK: call void @llvm.coro.end
   // CHECK: call void @_ZN10TrivialABID1Ev(ptr {{[^,]*}} %[[TrivialAlloca]])
 }
 
@@ -242,6 +242,6 @@ void msabi(MSParm p) {
   co_return;
 
   // The local alloca is used for the destructor call at the end of the ramp.
-  // MSABI: call i1 @llvm.coro.end
+  // MSABI: call void @llvm.coro.end
   // MSABI: call void @"??1MSParm@@QEAA@XZ"(ptr{{.*}} %[[ParamAlloca]])
 }

llvm/docs/Coroutines.rst

@@ -303,7 +303,7 @@ The LLVM IR for this coroutine looks like this:
     call void @free(ptr %mem)
     br label %suspend
   suspend:
-    %unused = call i1 @llvm.coro.end(ptr %hdl, i1 false, token none)
+    call void @llvm.coro.end(ptr %hdl, i1 false, token none)
     ret ptr %hdl
   }
 
@@ -637,7 +637,7 @@ store the current value produced by a coroutine.
     call void @free(ptr %mem)
     br label %suspend
   suspend:
-    %unused = call i1 @llvm.coro.end(ptr %hdl, i1 false, token none)
+    call void @llvm.coro.end(ptr %hdl, i1 false, token none)
     ret ptr %hdl
   }
 
@@ -806,7 +806,7 @@ The LLVM IR for a coroutine using a Coroutine with a custom ABI looks like:
     call void @free(ptr %mem)
     br label %suspend
   suspend:
-    %unused = call i1 @llvm.coro.end(ptr %hdl, i1 false, token none)
+    call void @llvm.coro.end(ptr %hdl, i1 false, token none)
     ret ptr %hdl
   }
 
@@ -1444,7 +1444,7 @@ A frontend should emit function attribute `presplitcoroutine` for the coroutine.
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 ::
 
-  declare i1 @llvm.coro.end(ptr <handle>, i1 <unwind>, token <result.token>)
+  declare void @llvm.coro.end(ptr <handle>, i1 <unwind>, token <result.token>)
 
 Overview:
 """""""""
@@ -1502,8 +1502,9 @@ For landingpad based exception model, it is expected that frontend uses the
 .. code-block:: llvm
 
     ehcleanup:
-      %InResumePart = call i1 @llvm.coro.end(ptr null, i1 true, token none)
-      br i1 %InResumePart, label %eh.resume, label %cleanup.cont
+      call void @llvm.coro.end(ptr null, i1 true, token none)
+      %InRamp = call i1 @llvm.coro.is_in_ramp()
+      br i1 %InRamp, label %cleanup.cont, label %eh.resume
 
     cleanup.cont:
       ; rest of the cleanup
@@ -1515,10 +1516,10 @@ For landingpad based exception model, it is expected that frontend uses the
       %lpad.val29 = insertvalue { ptr, i32 } %lpad.val, i32 %sel, 1
       resume { ptr, i32 } %lpad.val29
 
-The `CoroSpit` pass replaces `coro.end` with ``True`` in the resume functions,
-thus leading to immediate unwind to the caller, whereas in start function it
-is replaced with ``False``, thus allowing to proceed to the rest of the cleanup
-code that is only needed during initial invocation of the coroutine.
+The `CoroSpit` pass replaces `coro.is_in_ramp` with ``True`` in the ramp functions,
+thus allowing to proceed to the rest of the cleanup code that is only needed during
+initial invocation of the coroutine. Otherwise, it is replaced with ``False``,
+thus leading to immediate unwind to the caller.
 
 For Windows Exception handling model, a frontend should attach a funclet bundle
 referring to an enclosing cleanuppad as follows:
@@ -1527,7 +1528,7 @@ referring to an enclosing cleanuppad as follows:
 
     ehcleanup:
       %tok = cleanuppad within none []
-      %unused = call i1 @llvm.coro.end(ptr null, i1 true, token none) [ "funclet"(token %tok) ]
+      call void @llvm.coro.end(ptr null, i1 true, token none) [ "funclet"(token %tok) ]
       cleanupret from %tok unwind label %RestOfTheCleanup
 
 The `CoroSplit` pass, if the funclet bundle is present, will insert
@@ -1592,7 +1593,7 @@ The number of arguments must match the return type of the continuation function:
 
   cleanup:
     %tok = call token (...) @llvm.coro.end.results(i8 %val)
-    call i1 @llvm.coro.end(ptr %hdl, i1 0, token %tok)
+    call void @llvm.coro.end(ptr %hdl, i1 0, token %tok)
     unreachable
 
   ...
@@ -1604,7 +1605,7 @@ The number of arguments must match the return type of the continuation function:
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 ::
 
-  declare i1 @llvm.coro.end.async(ptr <handle>, i1 <unwind>, ...)
+  declare void @llvm.coro.end.async(ptr <handle>, i1 <unwind>, ...)
 
 Overview:
 """""""""
@@ -1635,10 +1636,10 @@ the function call.
 
 .. code-block:: llvm
 
-  call i1 (ptr, i1, ...) @llvm.coro.end.async(
-                           ptr %hdl, i1 0,
-                           ptr @must_tail_call_return,
-                           ptr %ctxt, ptr %task, ptr %actor)
+  call void (ptr, i1, ...) @llvm.coro.end.async(
+                             ptr %hdl, i1 0,
+                             ptr @must_tail_call_return,
+                             ptr %ctxt, ptr %task, ptr %actor)
   unreachable
 
 .. _coro.suspend:
@@ -2117,6 +2118,30 @@ Example:
       %hdl.result = ... ; get address of returned coroutine handle
       ret ptr %hdl.result
 
+'llvm.coro.is_in_ramp' Intrinsic
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+::
+
+  declare i1 @llvm.coro.is_in_ramp()
+
+Overview:
+"""""""""
+
+The '``llvm.coro.is_in_ramp``' intrinsic returns a bool value that marks coroutine ramp
+function and resume/destroy function.
+
+Arguments:
+""""""""""
+
+None
+
+Semantics:
+""""""""""
+
+The `CoroSpit` pass replaces `coro.is_in_ramp` with ``True`` ramp functions.
+Otherwise, it is replaced with ``False``, allowing the frontend to separate
+ramp function and resume/destroy function.
+
 Coroutine Transformation Passes
 ===============================
 CoroEarly

llvm/include/llvm/IR/Intrinsics.td

@@ -1775,12 +1775,13 @@ def int_coro_free : Intrinsic<[llvm_ptr_ty], [llvm_token_ty, llvm_ptr_ty],
                               [IntrReadMem, IntrArgMemOnly,
                                ReadOnly<ArgIndex<1>>,
                                NoCapture<ArgIndex<1>>]>;
-def int_coro_end : Intrinsic<[llvm_i1_ty], [llvm_ptr_ty, llvm_i1_ty, llvm_token_ty], []>;
+def int_coro_end : Intrinsic<[], [llvm_ptr_ty, llvm_i1_ty, llvm_token_ty], []>;
 def int_coro_end_results : Intrinsic<[llvm_token_ty], [llvm_vararg_ty]>;
 def int_coro_end_async
-    : Intrinsic<[llvm_i1_ty], [llvm_ptr_ty, llvm_i1_ty, llvm_vararg_ty], []>;
+    : Intrinsic<[], [llvm_ptr_ty, llvm_i1_ty, llvm_vararg_ty], []>;
 
 def int_coro_frame : Intrinsic<[llvm_ptr_ty], [], [IntrNoMem]>;
+def int_coro_is_in_ramp : Intrinsic<[llvm_i1_ty], [], [IntrNoMem], "llvm.coro.is_in_ramp">;
 def int_coro_noop : Intrinsic<[llvm_ptr_ty], [], [IntrNoMem]>;
 def int_coro_size : Intrinsic<[llvm_anyint_ty], [], [IntrNoMem]>;
 def int_coro_align : Intrinsic<[llvm_anyint_ty], [], [IntrNoMem]>;

llvm/include/llvm/Transforms/Coroutines/CoroInstr.h

@@ -428,6 +428,18 @@ class CoroFrameInst : public IntrinsicInst {
   }
 };
 
+/// This represents the llvm.coro.is_in_ramp instruction.
+class CoroIsInRampInst : public IntrinsicInst {
+public:
+  // Methods to support type inquiry through isa, cast, and dyn_cast:
+  static bool classof(const IntrinsicInst *I) {
+    return I->getIntrinsicID() == Intrinsic::coro_is_in_ramp;
+  }
+  static bool classof(const Value *V) {
+    return isa<IntrinsicInst>(V) && classof(cast<IntrinsicInst>(V));
+  }
+};
+
 /// This represents the llvm.coro.free instruction.
 class CoroFreeInst : public IntrinsicInst {
   enum { IdArg, FrameArg };

llvm/include/llvm/Transforms/Coroutines/CoroShape.h

@@ -53,6 +53,7 @@ enum class ABI {
 struct Shape {
   CoroBeginInst *CoroBegin = nullptr;
   SmallVector<AnyCoroEndInst *, 4> CoroEnds;
+  SmallVector<CoroIsInRampInst *, 2> CoroIsInRampInsts;
   SmallVector<CoroSizeInst *, 2> CoroSizes;
   SmallVector<CoroAlignInst *, 2> CoroAligns;
   SmallVector<AnyCoroSuspendInst *, 4> CoroSuspends;
@@ -65,6 +66,7 @@ struct Shape {
   void clear() {
     CoroBegin = nullptr;
     CoroEnds.clear();
+    CoroIsInRampInsts.clear();
     CoroSizes.clear();
     CoroAligns.clear();
     CoroSuspends.clear();

llvm/lib/Transforms/Coroutines/CoroCleanup.cpp

@@ -75,8 +75,8 @@ bool Lowerer::lower(Function &F) {
       case Intrinsic::coro_subfn_addr:
         lowerSubFn(Builder, cast<CoroSubFnInst>(II));
         break;
-      case Intrinsic::coro_end:
       case Intrinsic::coro_suspend_retcon:
+      case Intrinsic::coro_is_in_ramp:
         if (IsPrivateAndUnprocessed) {
           II->replaceAllUsesWith(PoisonValue::get(II->getType()));
         } else

llvm/lib/Transforms/Coroutines/CoroCloner.h

@@ -120,6 +120,7 @@ class BaseCloner {
   void replaceRetconOrAsyncSuspendUses();
   void replaceCoroSuspends();
   void replaceCoroEnds();
+  void replaceCoroIsInRamp();
   void replaceSwiftErrorOps();
   void salvageDebugInfo();
   void handleFinalSuspend();

llvm/lib/Transforms/Coroutines/CoroSplit.cpp

@@ -213,7 +213,7 @@ static bool replaceCoroEndAsync(AnyCoroEndInst *End) {
 /// Replace a non-unwind call to llvm.coro.end.
 static void replaceFallthroughCoroEnd(AnyCoroEndInst *End,
                                       const coro::Shape &Shape, Value *FramePtr,
-                                      bool InResume, CallGraph *CG) {
+                                      bool InRamp, CallGraph *CG) {
   // Start inserting right before the coro.end.
   IRBuilder<> Builder(End);
 
@@ -225,7 +225,7 @@ static void replaceFallthroughCoroEnd(AnyCoroEndInst *End,
            "switch coroutine should not return any values");
     // coro.end doesn't immediately end the coroutine in the main function
     // in this lowering, because we need to deallocate the coroutine.
-    if (!InResume)
+    if (InRamp)
       return;
     Builder.CreateRetVoid();
     break;
@@ -345,8 +345,7 @@ static void markCoroutineAsDone(IRBuilder<> &Builder, const coro::Shape &Shape,
 
 /// Replace an unwind call to llvm.coro.end.
 static void replaceUnwindCoroEnd(AnyCoroEndInst *End, const coro::Shape &Shape,
-                                 Value *FramePtr, bool InResume,
-                                 CallGraph *CG) {
+                                 Value *FramePtr, bool InRamp, CallGraph *CG) {
   IRBuilder<> Builder(End);
 
   switch (Shape.ABI) {
@@ -359,7 +358,7 @@ static void replaceUnwindCoroEnd(AnyCoroEndInst *End, const coro::Shape &Shape,
     // FIXME: We should refactor this once there is other language
     // which uses Switch-Resumed style other than C++.
     markCoroutineAsDone(Builder, Shape, FramePtr);
-    if (!InResume)
+    if (InRamp)
       return;
     break;
   }
@@ -383,15 +382,11 @@ static void replaceUnwindCoroEnd(AnyCoroEndInst *End, const coro::Shape &Shape,
 }
 
 static void replaceCoroEnd(AnyCoroEndInst *End, const coro::Shape &Shape,
-                           Value *FramePtr, bool InResume, CallGraph *CG) {
+                           Value *FramePtr, bool InRamp, CallGraph *CG) {
   if (End->isUnwind())
-    replaceUnwindCoroEnd(End, Shape, FramePtr, InResume, CG);
+    replaceUnwindCoroEnd(End, Shape, FramePtr, InRamp, CG);
   else
-    replaceFallthroughCoroEnd(End, Shape, FramePtr, InResume, CG);
-
-  auto &Context = End->getContext();
-  End->replaceAllUsesWith(InResume ? ConstantInt::getTrue(Context)
-                                   : ConstantInt::getFalse(Context));
+    replaceFallthroughCoroEnd(End, Shape, FramePtr, InRamp, CG);
   End->eraseFromParent();
 }
 
@@ -558,7 +553,16 @@ void coro::BaseCloner::replaceCoroEnds() {
     // We use a null call graph because there's no call graph node for
     // the cloned function yet.  We'll just be rebuilding that later.
     auto *NewCE = cast<AnyCoroEndInst>(VMap[CE]);
-    replaceCoroEnd(NewCE, Shape, NewFramePtr, /*in resume*/ true, nullptr);
+    replaceCoroEnd(NewCE, Shape, NewFramePtr, /*in ramp*/ false, nullptr);
+  }
+}
+
+void coro::BaseCloner::replaceCoroIsInRamp() {
+  auto &Ctx = OrigF.getContext();
+  for (auto *II : Shape.CoroIsInRampInsts) {
+    auto *NewII = cast<CoroIsInRampInst>(VMap[II]);
+    NewII->replaceAllUsesWith(ConstantInt::getFalse(Ctx));
+    NewII->eraseFromParent();
   }
 }
 
@@ -1077,6 +1081,8 @@ void coro::BaseCloner::create() {
   // Remove coro.end intrinsics.
   replaceCoroEnds();
 
+  replaceCoroIsInRamp();
+
   // Salvage debug info that points into the coroutine frame.
   salvageDebugInfo();
 }
@@ -1956,14 +1962,19 @@ class PrettyStackTraceFunction : public PrettyStackTraceEntry {
 static void removeCoroEndsFromRampFunction(const coro::Shape &Shape) {
   if (Shape.ABI != coro::ABI::Switch) {
     for (auto *End : Shape.CoroEnds) {
-      replaceCoroEnd(End, Shape, Shape.FramePtr, /*in resume*/ false, nullptr);
+      replaceCoroEnd(End, Shape, Shape.FramePtr, /*in ramp*/ true, nullptr);
     }
   } else {
-    for (llvm::AnyCoroEndInst *End : Shape.CoroEnds) {
-      auto &Context = End->getContext();
-      End->replaceAllUsesWith(ConstantInt::getFalse(Context));
+    for (llvm::AnyCoroEndInst *End : Shape.CoroEnds)
       End->eraseFromParent();
-    }
+  }
+}
+
+static void removeCoroIsInRampFromRampFunction(const coro::Shape &Shape) {
+  for (auto *II : Shape.CoroIsInRampInsts) {
+    auto &Ctx = II->getContext();
+    II->replaceAllUsesWith(ConstantInt::getTrue(Ctx));
+    II->eraseFromParent();
   }
 }
 
@@ -2028,6 +2039,7 @@ static void doSplitCoroutine(Function &F, SmallVectorImpl<Function *> &Clones,
     coro::salvageDebugInfo(ArgToAllocaMap, *DVR, false /*UseEntryValue*/);
 
   removeCoroEndsFromRampFunction(Shape);
+  removeCoroIsInRampFromRampFunction(Shape);
 
   if (shouldCreateNoAllocVariant)
     SwitchCoroutineSplitter::createNoAllocVariant(F, Shape, Clones);