[llvm] 06c1aa3 - [CoroEarly][IR] Clarify semantic of llvm.coro.end (#191752)
via llvm-commits
llvm-commits at lists.llvm.org
Tue Apr 14 02:31:21 PDT 2026
Author: Weibo He
Date: 2026-04-14T17:31:15+08:00
New Revision: 06c1aa3ca74cf73df07d1ef3fff9b476e5aedaab
URL: https://github.com/llvm/llvm-project/commit/06c1aa3ca74cf73df07d1ef3fff9b476e5aedaab
DIFF: https://github.com/llvm/llvm-project/commit/06c1aa3ca74cf73df07d1ef3fff9b476e5aedaab.diff
LOG: [CoroEarly][IR] Clarify semantic of llvm.coro.end (#191752)
We introduced a workaround for the following pattern in #139243:
``` LLVM
define void @fn() presplitcoroutine {
%__promise = alloca ptr, align 8
...
coro.ret:
call void @llvm.coro.end(ptr null, i1 false, token none)
store ptr null, ptr %__promise, align 8
ret void
}
```
where DSE considers `__promise` dead after the return and eliminates the
store, leading to a miscompilation.
However, after #151067, the problematic pattern is gone. And it
currently looks like:
``` LLVM
gro.conv:
store ptr null, ptr %__promise, align 8
br label %after.gro.conv
after.gro.conv:
br i1 %body.done, label %coro.cleanup, label %coro.ret
coro.ret:
call void @llvm.coro.end(ptr null, i1 false, token none)
ret void
```
DSE cannot eliminate the store because `__promise` escapes through
`coro.id`, and `coro.end` post-dominates the store. It turns out that
accessing the coroutine frame after `coro.end` is not safe. This patch
proposes clarifying the semantics of `coro.end` and reverting the
workaround, as it confuses alias analyses. I have scanned the tests and
updated the uses of the coroutine frame after `coro.end`.
Added:
Modified:
llvm/docs/Coroutines.rst
llvm/include/llvm/Transforms/Coroutines/CoroShape.h
llvm/lib/Transforms/Coroutines/CoroEarly.cpp
llvm/lib/Transforms/Coroutines/Coroutines.cpp
llvm/test/Transforms/Coroutines/coro-debug.ll
Removed:
llvm/test/Transforms/Coroutines/gh105595.ll
################################################################################
diff --git a/llvm/docs/Coroutines.rst b/llvm/docs/Coroutines.rst
index d1c4ffe4c6f35..bb38343ef294e 100644
--- a/llvm/docs/Coroutines.rst
+++ b/llvm/docs/Coroutines.rst
@@ -1493,9 +1493,7 @@ A frontend should emit function attribute `presplitcoroutine` for the coroutine.
Overview:
"""""""""
-The '``llvm.coro.end``' marks the point where execution of the resume part of
-the coroutine should end and control should return to the caller.
-
+The '``llvm.coro.end``' marks the point where access to the coroutine frame ends.
Arguments:
""""""""""
@@ -1517,9 +1515,14 @@ Only none token is allowed for coro.end calls in unwind sections
Semantics:
""""""""""
-The purpose of this intrinsic is to allow frontends to mark the cleanup and
-other code that is only relevant during the initial invocation of the coroutine
-and should not be present in resume and destroy parts.
+The purposes of `coro.end` are:
+
+- Allow frontends to mark the cleanup and other code that is only relevant during
+ the initial invocation of the coroutine and should not be present in resume and
+ destroy parts.
+
+- Coroutine frame typically escapes. The call to `coro.end` prevents other
+ optimizations from erasing stores to frame before returning.
In returned-continuation lowering, ``llvm.coro.end`` fully destroys the
coroutine frame. If the second argument is `false`, it also returns from
@@ -2193,8 +2196,7 @@ CoroEarly
The CoroEarly pass ensures later middle end passes correctly interpret coroutine
semantics and lowers coroutine intrinsics that not needed to be preserved to
help later coroutine passes. This pass lowers `coro.promise`_, `coro.frame`_ and
-`coro.done`_ intrinsics. Afterwards, it replaces uses of promise alloca with
-`coro.promise`_ intrinsic.
+`coro.done`_ intrinsics.
.. _CoroSplit:
diff --git a/llvm/include/llvm/Transforms/Coroutines/CoroShape.h b/llvm/include/llvm/Transforms/Coroutines/CoroShape.h
index c90d188556960..f4673d61f097d 100644
--- a/llvm/include/llvm/Transforms/Coroutines/CoroShape.h
+++ b/llvm/include/llvm/Transforms/Coroutines/CoroShape.h
@@ -82,8 +82,7 @@ struct Shape {
// Scan the function and collect the above intrinsics for later processing
LLVM_ABI void analyze(Function &F,
SmallVectorImpl<CoroFrameInst *> &CoroFrames,
- SmallVectorImpl<CoroSaveInst *> &UnusedCoroSaves,
- CoroPromiseInst *&CoroPromise);
+ SmallVectorImpl<CoroSaveInst *> &UnusedCoroSaves);
// If for some reason, we were not able to find coro.begin, bailout.
LLVM_ABI void
invalidateCoroutine(Function &F,
@@ -91,9 +90,9 @@ struct Shape {
// Perform ABI related initial transformation
LLVM_ABI void initABI();
// Remove orphaned and unnecessary intrinsics
- LLVM_ABI void cleanCoroutine(SmallVectorImpl<CoroFrameInst *> &CoroFrames,
- SmallVectorImpl<CoroSaveInst *> &UnusedCoroSaves,
- CoroPromiseInst *CoroPromise);
+ LLVM_ABI void
+ cleanCoroutine(SmallVectorImpl<CoroFrameInst *> &CoroFrames,
+ SmallVectorImpl<CoroSaveInst *> &UnusedCoroSaves);
coro::ABI ABI;
@@ -254,14 +253,13 @@ struct Shape {
explicit Shape(Function &F) {
SmallVector<CoroFrameInst *, 8> CoroFrames;
SmallVector<CoroSaveInst *, 2> UnusedCoroSaves;
- CoroPromiseInst *CoroPromise = nullptr;
- analyze(F, CoroFrames, UnusedCoroSaves, CoroPromise);
+ analyze(F, CoroFrames, UnusedCoroSaves);
if (!CoroBegin) {
invalidateCoroutine(F, CoroFrames);
return;
}
- cleanCoroutine(CoroFrames, UnusedCoroSaves, CoroPromise);
+ cleanCoroutine(CoroFrames, UnusedCoroSaves);
}
};
diff --git a/llvm/lib/Transforms/Coroutines/CoroEarly.cpp b/llvm/lib/Transforms/Coroutines/CoroEarly.cpp
index e003812f81192..0c67ceccf4c4d 100644
--- a/llvm/lib/Transforms/Coroutines/CoroEarly.cpp
+++ b/llvm/lib/Transforms/Coroutines/CoroEarly.cpp
@@ -27,7 +27,6 @@ class Lowerer : public coro::LowererBase {
void lowerResumeOrDestroy(CallBase &CB, CoroSubFnInst::ResumeKind);
void lowerCoroPromise(CoroPromiseInst *Intrin);
void lowerCoroDone(IntrinsicInst *II);
- void hidePromiseAlloca(CoroIdInst *CoroId, CoroBeginInst *CoroBegin);
public:
Lowerer(Module &M)
@@ -94,34 +93,6 @@ void Lowerer::lowerCoroDone(IntrinsicInst *II) {
II->eraseFromParent();
}
-// Later middle-end passes will assume promise alloca dead after coroutine
-// suspend, leading to misoptimizations. We hide promise alloca using
-// coro.promise and will lower it back to alloca at CoroSplit.
-void Lowerer::hidePromiseAlloca(CoroIdInst *CoroId, CoroBeginInst *CoroBegin) {
- auto *PA = CoroId->getPromise();
- if (!PA || !CoroBegin)
- return;
- Builder.SetInsertPoint(*CoroBegin->getInsertionPointAfterDef());
-
- auto *Alignment = Builder.getInt32(PA->getAlign().value());
- auto *FromPromise = Builder.getInt1(false);
- SmallVector<Value *, 3> Arg{CoroBegin, Alignment, FromPromise};
- auto *PI = Builder.CreateIntrinsic(
- Builder.getPtrTy(), Intrinsic::coro_promise, Arg, {}, "promise.addr");
- PI->setCannotDuplicate();
- // Remove lifetime markers, as these are only allowed on allocas.
- for (User *U : make_early_inc_range(PA->users())) {
- auto *I = cast<Instruction>(U);
- if (I->isLifetimeStartOrEnd())
- I->eraseFromParent();
- }
- PA->replaceUsesWithIf(PI, [CoroId](Use &U) {
- bool IsBitcast = U == U.getUser()->stripPointerCasts();
- bool IsCoroId = U.getUser() == CoroId;
- return !IsBitcast && !IsCoroId;
- });
-}
-
// Prior to CoroSplit, calls to coro.begin needs to be marked as NoDuplicate,
// as CoroSplit assumes there is exactly one coro.begin. After CoroSplit,
// NoDuplicate attribute will be removed from coro.begin otherwise, it will
@@ -207,8 +178,6 @@ void Lowerer::lowerEarlyIntrinsics(Function &F) {
// we allow specifying none and fixing it up here.
for (CoroFreeInst *CF : CoroFrees)
CF->setArgOperand(0, CoroId);
-
- hidePromiseAlloca(CoroId, CoroBegin);
}
// Coroutine suspention could potentially lead to any argument modified
diff --git a/llvm/lib/Transforms/Coroutines/Coroutines.cpp b/llvm/lib/Transforms/Coroutines/Coroutines.cpp
index a9f96e0309d2f..9e798ccbaacee 100644
--- a/llvm/lib/Transforms/Coroutines/Coroutines.cpp
+++ b/llvm/lib/Transforms/Coroutines/Coroutines.cpp
@@ -182,8 +182,7 @@ static CoroSaveInst *createCoroSave(CoroBeginInst *CoroBegin,
// Collect "interesting" coroutine intrinsics.
void coro::Shape::analyze(Function &F,
SmallVectorImpl<CoroFrameInst *> &CoroFrames,
- SmallVectorImpl<CoroSaveInst *> &UnusedCoroSaves,
- CoroPromiseInst *&CoroPromise) {
+ SmallVectorImpl<CoroSaveInst *> &UnusedCoroSaves) {
clear();
bool HasFinalSuspend = false;
@@ -280,11 +279,6 @@ void coro::Shape::analyze(Function &F,
case Intrinsic::coro_is_in_ramp:
CoroIsInRampInsts.push_back(cast<CoroIsInRampInst>(II));
break;
- case Intrinsic::coro_promise:
- assert(CoroPromise == nullptr &&
- "CoroEarly must ensure coro.promise unique");
- CoroPromise = cast<CoroPromiseInst>(II);
- break;
}
}
}
@@ -476,7 +470,7 @@ void coro::AnyRetconABI::init() {
void coro::Shape::cleanCoroutine(
SmallVectorImpl<CoroFrameInst *> &CoroFrames,
- SmallVectorImpl<CoroSaveInst *> &UnusedCoroSaves, CoroPromiseInst *PI) {
+ SmallVectorImpl<CoroSaveInst *> &UnusedCoroSaves) {
// The coro.frame intrinsic is always lowered to the result of coro.begin.
for (CoroFrameInst *CF : CoroFrames) {
CF->replaceAllUsesWith(CoroBegin);
@@ -488,13 +482,6 @@ void coro::Shape::cleanCoroutine(
for (CoroSaveInst *CoroSave : UnusedCoroSaves)
CoroSave->eraseFromParent();
UnusedCoroSaves.clear();
-
- if (PI) {
- PI->replaceAllUsesWith(PI->isFromPromise()
- ? cast<Value>(CoroBegin)
- : cast<Value>(getPromiseAlloca()));
- PI->eraseFromParent();
- }
}
static void propagateCallAttrsFromCallee(CallInst *Call, Function *Callee) {
diff --git a/llvm/test/Transforms/Coroutines/coro-debug.ll b/llvm/test/Transforms/Coroutines/coro-debug.ll
index 109be51ffea85..4c66cb94667b5 100644
--- a/llvm/test/Transforms/Coroutines/coro-debug.ll
+++ b/llvm/test/Transforms/Coroutines/coro-debug.ll
@@ -62,17 +62,16 @@ indirect.dest:
br label %coro_Cleanup
coro_Cleanup: ; preds = %sw.epilog, %sw.bb1
- %5 = load ptr, ptr %coro_hdl, align 8, !dbg !24
- %6 = call ptr @llvm.coro.free(token %0, ptr %5), !dbg !24
- call void @free(ptr %6), !dbg !24
+ %mem = call ptr @llvm.coro.free(token %0, ptr %2), !dbg !24
+ call void @free(ptr %mem), !dbg !24
call void @llvm.dbg.value(metadata i32 %asm_res, metadata !32, metadata !13), !dbg !16
br label %coro_Suspend, !dbg !24
coro_Suspend: ; preds = %coro_Cleanup, %sw.default
call void @llvm.coro.end(ptr null, i1 false, token none), !dbg !24
- %7 = load ptr, ptr %coro_hdl, align 8, !dbg !24
+ %ret = load ptr, ptr %coro_hdl, align 8, !dbg !24
store i32 0, ptr %late_local, !dbg !24
- ret ptr %7, !dbg !24
+ ret ptr %ret, !dbg !24
ehcleanup:
%ex = landingpad { ptr, i32 }
@@ -163,10 +162,9 @@ attributes #0 = { noinline nounwind presplitcoroutine }
; Also check that it contains `#dbg_declare` and `#dbg_value` debug instructions
; making the debug variables available to the debugger.
;
-; CHECK: define internal fastcc void @flink.resume(ptr noundef nonnull align 8 dereferenceable(40) %0) #0 personality i32 0 !dbg ![[RESUME:[0-9]+]]
+; CHECK: define internal fastcc void @flink.resume(ptr noundef nonnull align 8 dereferenceable(32) %0) #0 personality i32 0 !dbg ![[RESUME:[0-9]+]]
; CHECK: entry.resume:
; CHECK: %[[DBG_PTR:.*]] = alloca ptr
-; CHECK-NEXT: #dbg_declare(ptr %[[DBG_PTR]], ![[RESUME_COROHDL:[0-9]+]], !DIExpression(DW_OP_deref, DW_OP_plus_uconst,
; CHECK-NEXT: #dbg_declare(ptr %[[DBG_PTR]], ![[RESUME_X:[0-9]+]], !DIExpression(DW_OP_deref, DW_OP_plus_uconst, [[EXPR_TAIL:.*]])
; CHECK-NEXT: store ptr {{.*}}, ptr %[[DBG_PTR]]
; CHECK-NOT: alloca ptr
@@ -188,8 +186,8 @@ attributes #0 = { noinline nounwind presplitcoroutine }
; Check that the destroy and cleanup functions are present and capture their debug info id.
;
-; CHECK: define internal fastcc void @flink.destroy(ptr noundef nonnull align 8 dereferenceable(40) %0) #0 personality i32 0 !dbg ![[DESTROY:[0-9]+]]
-; CHECK: define internal fastcc void @flink.cleanup(ptr noundef nonnull align 8 dereferenceable(40) %0) #0 personality i32 0 !dbg ![[CLEANUP:[0-9]+]]
+; CHECK: define internal fastcc void @flink.destroy(ptr noundef nonnull align 8 dereferenceable(32) %0) #0 personality i32 0 !dbg ![[DESTROY:[0-9]+]]
+; CHECK: define internal fastcc void @flink.cleanup(ptr noundef nonnull align 8 dereferenceable(32) %0) #0 personality i32 0 !dbg ![[CLEANUP:[0-9]+]]
; Check that the linkage name of the original function is set correctly.
;
@@ -198,7 +196,6 @@ attributes #0 = { noinline nounwind presplitcoroutine }
; Check that metadata for local variables in the resume function is set correctly.
;
-; CHECK: ![[RESUME_COROHDL]] = !DILocalVariable(name: "coro_hdl", scope: ![[RESUME]]
; CHECK: ![[RESUME_X]] = !DILocalVariable(name: "x", arg: 1, scope: ![[RESUME]]
; CHECK: ![[RESUME_CONST]] = !DILocalVariable(name: "direct_const", scope: ![[RESUME]]
; CHECK: ![[RESUME_DIRECT]] = !DILocalVariable(name: "direct_mem", scope: ![[RESUME]]
diff --git a/llvm/test/Transforms/Coroutines/gh105595.ll b/llvm/test/Transforms/Coroutines/gh105595.ll
deleted file mode 100644
index 0efe21216e998..0000000000000
--- a/llvm/test/Transforms/Coroutines/gh105595.ll
+++ /dev/null
@@ -1,31 +0,0 @@
-; Test that store-load operation that crosses suspension point will not be eliminated by DSE
-; Coro result conversion function that attempts to modify promise shall produce this pattern
-; RUN: opt < %s -passes='coro-early,dse' -S | FileCheck %s
-
-define void @fn() presplitcoroutine {
- %__promise = alloca ptr, align 8
- %id = call token @llvm.coro.id(i32 16, ptr %__promise, ptr @fn, ptr null)
- %hdl = call ptr @llvm.coro.begin(token %id, ptr null)
-; CHECK: %promise.addr = call ptr @llvm.coro.promise(ptr %hdl, i32 8, i1 false)
- %save = call token @llvm.coro.save(ptr null)
- %sp = call i8 @llvm.coro.suspend(token %save, i1 false)
- %flag = icmp ule i8 %sp, 1
- br i1 %flag, label %resume, label %suspend
-
-resume:
-; CHECK: call void @use_value(ptr %promise.addr)
- call void @use_value(ptr %__promise)
- br label %suspend
-
-suspend:
-; load value when resume
-; CHECK: %null = load ptr, ptr %promise.addr, align 8
- %null = load ptr, ptr %__promise, align 8
- call void @use_value(ptr %null)
-; store value when suspend
-; CHECK: store ptr null, ptr %promise.addr, align 8
- store ptr null, ptr %__promise, align 8
- ret void
-}
-
-declare void @use_value(ptr)
More information about the llvm-commits
mailing list