[clang] b9e3e65 - [Clang][coro] Fix `coro.free` in `.resume` clones with `[[clang::coro_await_elidable]]` (#207799)
via cfe-commits
cfe-commits at lists.llvm.org
Wed Jul 22 07:39:35 PDT 2026
Author: Yuxuan Chen
Date: 2026-07-22T07:39:30-07:00
New Revision: b9e3e6546dcb4b33d023b7a336163d5650cda9d4
URL: https://github.com/llvm/llvm-project/commit/b9e3e6546dcb4b33d023b7a336163d5650cda9d4
DIFF: https://github.com/llvm/llvm-project/commit/b9e3e6546dcb4b33d023b7a336163d5650cda9d4.diff
LOG: [Clang][coro] Fix `coro.free` in `.resume` clones with `[[clang::coro_await_elidable]]` (#207799)
Fixes https://github.com/llvm/llvm-project/issues/188230
CoroAnnotationElide rewrites annotated safe calls to the `.noalloc`
variant. The noalloc frame is caller-owned, but its `.resume` clone is
shared with ordinary heap-allocated instances.
With a `suspend_never` final suspend, normal resumption falls through to
the `coro.free` deallocation path. Regular frontend cleanup has already
run before this point. `coro.free` must therefore produce the frame
pointer for a heap instance and null for a `.noalloc` instance.
Use the frame destroy slot as a per-instance allocation tag. Cache its
value at resume entry, before user code can resume and release the
enclosing caller frame, then compare it with the cleanup clone. Replace
each `coro.free` result with the frame pointer for a heap instance and
null for an elided instance.
Update the CoroSplit and Clang CodeGen checks to cover the conditional
deallocation and the original suspend_never final-suspend shape.
Assisted-By: Codex GPT 5.5
Added:
clang/test/CodeGenCoroutines/gh188230-coro-await-elidable-suspend-never-final.cpp
llvm/test/Transforms/Coroutines/coro-split-resume-fallthrough-destroy-slot.ll
Modified:
llvm/include/llvm/Transforms/Coroutines/CoroShape.h
llvm/lib/Transforms/Coroutines/CoroSplit.cpp
llvm/lib/Transforms/Coroutines/Coroutines.cpp
llvm/test/Transforms/Coroutines/coro-split-00.ll
llvm/test/Transforms/Coroutines/coro-split-addrspace.ll
Removed:
################################################################################
diff --git a/clang/test/CodeGenCoroutines/gh188230-coro-await-elidable-suspend-never-final.cpp b/clang/test/CodeGenCoroutines/gh188230-coro-await-elidable-suspend-never-final.cpp
new file mode 100644
index 0000000000000..bcd2cff989620
--- /dev/null
+++ b/clang/test/CodeGenCoroutines/gh188230-coro-await-elidable-suspend-never-final.cpp
@@ -0,0 +1,79 @@
+// Tests that a coro_await_elidable coroutine with a suspend_never final suspend
+// suppresses only the deallocation for a resumed elided callee.
+//
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -std=c++20 -O2 \
+// RUN: -mllvm -coro-elide-branch-ratio=0 -emit-llvm %s -o - | FileCheck %s
+
+#include "Inputs/coroutine.h"
+
+struct gate {
+ std::coroutine_handle<> waiter = nullptr;
+ bool open = false;
+
+ struct awaiter {
+ gate &g;
+ bool await_ready() noexcept { return g.open; }
+ void await_suspend(std::coroutine_handle<> h) noexcept { g.waiter = h; }
+ void await_resume() noexcept {}
+ };
+
+ awaiter operator co_await() noexcept { return {*this}; }
+};
+
+struct [[clang::coro_await_elidable]] task {
+ struct promise_type {
+ std::coroutine_handle<> continuation = nullptr;
+
+ task get_return_object() noexcept {
+ return {std::coroutine_handle<promise_type>::from_promise(*this)};
+ }
+
+ std::suspend_never initial_suspend() noexcept { return {}; }
+ std::suspend_never final_suspend() noexcept { return {}; }
+
+ void return_void() noexcept {
+ if (continuation)
+ continuation.resume();
+ }
+
+ void unhandled_exception() noexcept { __builtin_abort(); }
+ };
+
+ std::coroutine_handle<promise_type> handle;
+
+ bool await_ready() noexcept { return false; }
+
+ void await_suspend(std::coroutine_handle<> h) noexcept {
+ handle.promise().continuation = h;
+ }
+
+ void await_resume() noexcept {}
+};
+
+task callee(gate &g, int &value) {
+ co_await g;
+ value = 42;
+}
+
+task caller(gate &g, int &value, bool &finished) {
+ co_await callee(g, value);
+ finished = true;
+}
+
+// CHECK-LABEL: define internal void @_Z6calleeR4gateRi.resume(
+// CHECK: %[[DESTROY_ADDR:.+]] = getelementptr inbounds{{.*}} i8, ptr %{{.+}}, i64 8
+// CHECK-NEXT: %[[DESTROY:.+]] = load ptr, ptr %[[DESTROY_ADDR]]
+// CHECK-NEXT: %[[IS_ELIDED:.+]] = icmp eq ptr %[[DESTROY]], @_Z6calleeR4gateRi.cleanup
+// CHECK: store i32 42,
+// CHECK: br i1 %[[IS_ELIDED]], label %[[CORO_END:.+]], label %[[CORO_FREE:.+]]
+// CHECK: [[CORO_FREE]]:
+// CHECK-NEXT: tail call void @_Zdl
+// CHECK: [[CORO_END]]:
+// CHECK-NEXT: ret void
+
+// CHECK-LABEL: define internal {{.*}}void @_Z6calleeR4gateRi.destroy(
+// CHECK: call void @_Zdl
+
+// CHECK-LABEL: define internal {{.*}}void @_Z6calleeR4gateRi.cleanup(
+// CHECK-NOT: call void @_Zdl
+// CHECK: ret void
diff --git a/llvm/include/llvm/Transforms/Coroutines/CoroShape.h b/llvm/include/llvm/Transforms/Coroutines/CoroShape.h
index 28931e3260e68..534e63f28ddc1 100644
--- a/llvm/include/llvm/Transforms/Coroutines/CoroShape.h
+++ b/llvm/include/llvm/Transforms/Coroutines/CoroShape.h
@@ -112,6 +112,7 @@ struct Shape {
unsigned IndexOffset;
bool HasFinalSuspend;
bool HasUnwindCoroEnd;
+ bool HasCoroElideNoAllocVariant;
};
struct RetconLoweringStorage {
diff --git a/llvm/lib/Transforms/Coroutines/CoroSplit.cpp b/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
index 7915fbf4cce05..c788c1623587a 100644
--- a/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
+++ b/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
@@ -164,6 +164,50 @@ static void maybeFreeRetconStorage(IRBuilder<> &Builder,
Shape.emitDealloc(Builder, FramePtr, CG);
}
+/// Create a pointer to the switch destroy function field in the coroutine
+/// frame.
+static Value *createSwitchDestroyPtr(const coro::Shape &Shape,
+ IRBuilder<> &Builder, Value *FramePtr) {
+ auto *Offset = ConstantInt::get(Type::getInt64Ty(FramePtr->getContext()),
+ Shape.SwitchLowering.DestroyOffset);
+ return Builder.CreateInBoundsPtrAdd(FramePtr, Offset, "destroy.addr");
+}
+
+/// Make resume-clone coro.free conditional on whether the frame is elided.
+///
+/// The destroy slot holds the cleanup clone for an elided frame and the destroy
+/// clone for a heap frame. Load it before user code can reentrantly destroy the
+/// enclosing caller frame, then use the cached comparison to suppress only the
+/// deallocation. The resume clone has already performed the shared coroutine
+/// cleanup, so calling either clone here would run that cleanup twice.
+static void replaceSwitchResumeCoroFree(const coro::Shape &Shape,
+ Function &Resume, Function &Cleanup) {
+ Value *FramePtr = Resume.getArg(0);
+ IRBuilder<> EntryBuilder(Resume.getEntryBlock().getTerminator());
+ Value *DestroyAddr = createSwitchDestroyPtr(Shape, EntryBuilder, FramePtr);
+ Value *DestroyFn = EntryBuilder.CreateLoad(Shape.getSwitchResumePointerType(),
+ DestroyAddr, "destroy");
+ Value *CleanupFn =
+ EntryBuilder.CreatePointerCast(&Cleanup, DestroyFn->getType());
+ Value *IsElided =
+ EntryBuilder.CreateICmpEQ(DestroyFn, CleanupFn, "is.elided");
+
+ SmallVector<CoroFreeInst *, 4> CoroFrees;
+ for (User *U : FramePtr->users()) {
+ if (auto *CF = dyn_cast<CoroFreeInst>(U))
+ CoroFrees.push_back(CF);
+ }
+
+ for (CoroFreeInst *CF : CoroFrees) {
+ IRBuilder<> Builder(CF);
+ auto *Null = ConstantPointerNull::get(cast<PointerType>(CF->getType()));
+ Value *Replacement =
+ Builder.CreateSelect(IsElided, Null, FramePtr, "coro.free");
+ CF->replaceAllUsesWith(Replacement);
+ CF->eraseFromParent();
+ }
+}
+
/// Replace an llvm.coro.end.async.
/// Will inline the must tail call function call if there is one.
/// \returns true if cleanup of the coro.end block is needed, false otherwise.
@@ -1382,6 +1426,9 @@ struct SwitchCoroutineSplitter {
auto *CleanupClone = coro::SwitchCloner::createClone(
F, ".cleanup", Shape, coro::CloneKind::SwitchCleanup, TTI);
+ if (Shape.SwitchLowering.HasCoroElideNoAllocVariant)
+ replaceSwitchResumeCoroFree(Shape, *ResumeClone, *CleanupClone);
+
postSplitCleanup(*ResumeClone);
postSplitCleanup(*DestroyClone);
postSplitCleanup(*CleanupClone);
@@ -2014,6 +2061,9 @@ static void doSplitCoroutine(Function &F, SmallVectorImpl<Function *> &Clones,
bool shouldCreateNoAllocVariant =
!isNoSuspendCoroutine && Shape.ABI == coro::ABI::Switch &&
hasSafeElideCaller(F) && !F.hasFnAttribute(llvm::Attribute::NoInline);
+ if (Shape.ABI == coro::ABI::Switch)
+ Shape.SwitchLowering.HasCoroElideNoAllocVariant =
+ shouldCreateNoAllocVariant;
// If there are no suspend points, no split required, just remove
// the allocation and deallocation blocks, they are not needed.
@@ -2052,10 +2102,22 @@ static LazyCallGraph::SCC &updateCallGraphAfterCoroutineSplit(
if (!Clones.empty()) {
switch (Shape.ABI) {
case coro::ABI::Switch:
- // Each clone in the Switch lowering is independent of the other clones.
- // Let the LazyCallGraph know about each one separately.
- for (Function *Clone : Clones)
- CG.addSplitFunction(N.getFunction(), *Clone);
+ // The resume clone's elided-frame check holds a reference to the cleanup
+ // clone. Add the cleanup clone first, so populating the resume node does
+ // not materialize an unregistered cleanup node.
+ if (Shape.SwitchLowering.HasCoroElideNoAllocVariant) {
+ assert(Clones.size() >= 3 && "expected switch coroutine clones");
+ CG.addSplitFunction(N.getFunction(), *Clones[2]);
+ CG.addSplitFunction(N.getFunction(), *Clones[1]);
+ CG.addSplitFunction(N.getFunction(), *Clones[0]);
+ for (Function *Clone : drop_begin(Clones, 3))
+ CG.addSplitFunction(N.getFunction(), *Clone);
+ } else {
+ // Each clone in the Switch lowering is independent of the other
+ // clones. Let the LazyCallGraph know about each one separately.
+ for (Function *Clone : Clones)
+ CG.addSplitFunction(N.getFunction(), *Clone);
+ }
break;
case coro::ABI::Async:
case coro::ABI::Retcon:
diff --git a/llvm/lib/Transforms/Coroutines/Coroutines.cpp b/llvm/lib/Transforms/Coroutines/Coroutines.cpp
index a922099a1f43f..2ecf66d18b5d3 100644
--- a/llvm/lib/Transforms/Coroutines/Coroutines.cpp
+++ b/llvm/lib/Transforms/Coroutines/Coroutines.cpp
@@ -294,6 +294,7 @@ void coro::Shape::analyze(Function &F,
ABI = coro::ABI::Switch;
SwitchLowering.HasFinalSuspend = HasFinalSuspend;
SwitchLowering.HasUnwindCoroEnd = HasUnwindCoroEnd;
+ SwitchLowering.HasCoroElideNoAllocVariant = false;
auto SwitchId = getSwitchCoroId();
SwitchLowering.ResumeSwitch = nullptr;
diff --git a/llvm/test/Transforms/Coroutines/coro-split-00.ll b/llvm/test/Transforms/Coroutines/coro-split-00.ll
index 727b9b2b9776e..8a8f65de97f0c 100644
--- a/llvm/test/Transforms/Coroutines/coro-split-00.ll
+++ b/llvm/test/Transforms/Coroutines/coro-split-00.ll
@@ -51,11 +51,15 @@ entry:
; CHECK: ret ptr %hdl
; CHECK-LABEL: @f.resume({{.*}}) {
+; CHECK: %[[DESTROY_ADDR:.+]] = getelementptr inbounds i8, ptr %hdl, i64 8
+; CHECK-NEXT: %[[DESTROY:.+]] = load ptr, ptr %[[DESTROY_ADDR]]
+; CHECK-NEXT: %[[IS_ELIDED:.+]] = icmp eq ptr %[[DESTROY]], @f.cleanup
; CHECK-NOT: call ptr @malloc
; CHECK-NOT: call void @print(i32 0)
; CHECK: call void @print(i32 1)
; CHECK-NOT: call void @print(i32 0)
-; CHECK: call void @free(
+; CHECK: %[[CORO_FREE:.+]] = select i1 %[[IS_ELIDED]], ptr null, ptr %hdl
+; CHECK-NEXT: call void @free(ptr %[[CORO_FREE]])
; CHECK: ret void
; CHECK-LABEL: @f.destroy({{.*}}) {
diff --git a/llvm/test/Transforms/Coroutines/coro-split-addrspace.ll b/llvm/test/Transforms/Coroutines/coro-split-addrspace.ll
index 24db35d141b35..fe08a7dd8b525 100644
--- a/llvm/test/Transforms/Coroutines/coro-split-addrspace.ll
+++ b/llvm/test/Transforms/Coroutines/coro-split-addrspace.ll
@@ -52,11 +52,15 @@ entry:
; CHECK: ret ptr %hdl
; CHECK-LABEL: @f.resume({{.*}}) addrspace(200) {
+; CHECK: %[[DESTROY_ADDR:.+]] = getelementptr inbounds i8, ptr %hdl, i64 8
+; CHECK-NEXT: %[[DESTROY:.+]] = load ptr, ptr %[[DESTROY_ADDR]]
+; CHECK-NEXT: %[[IS_ELIDED:.+]] = icmp eq ptr %[[DESTROY]], addrspacecast (ptr addrspace(200) @f.cleanup to ptr)
; CHECK-NOT: call ptr @malloc
; CHECK-NOT: call void @print(i32 0)
; CHECK: call void @print(i32 1)
; CHECK-NOT: call void @print(i32 0)
-; CHECK: call void @free(
+; CHECK: %[[CORO_FREE:.+]] = select i1 %[[IS_ELIDED]], ptr null, ptr %hdl
+; CHECK-NEXT: call void @free(ptr %[[CORO_FREE]])
; CHECK: ret void
; CHECK-LABEL: @f.destroy({{.*}}) addrspace(200) {
diff --git a/llvm/test/Transforms/Coroutines/coro-split-resume-fallthrough-destroy-slot.ll b/llvm/test/Transforms/Coroutines/coro-split-resume-fallthrough-destroy-slot.ll
new file mode 100644
index 0000000000000..9641a7253a5a0
--- /dev/null
+++ b/llvm/test/Transforms/Coroutines/coro-split-resume-fallthrough-destroy-slot.ll
@@ -0,0 +1,84 @@
+; Tests that a switch coroutine resume clone which falls through to coro.end
+; suppresses only the deallocation for a stack-elided frame. This matters for
+; allocation elision: the frame slot contains the cleanup clone in that case.
+;
+; RUN: opt < %s -passes='cgscc(coro-split),simplifycfg,early-cse' -S | FileCheck %s
+
+define ptr @f() presplitcoroutine {
+entry:
+ %id = call token @llvm.coro.id(i32 0, ptr null, ptr @f, ptr null)
+ %need.alloc = call i1 @llvm.coro.alloc(token %id)
+ br i1 %need.alloc, label %dyn.alloc, label %begin
+
+dyn.alloc:
+ %size = call i32 @llvm.coro.size.i32()
+ %alloc = call ptr @malloc(i32 %size)
+ br label %begin
+
+begin:
+ %phi = phi ptr [ null, %entry ], [ %alloc, %dyn.alloc ]
+ %hdl = call ptr @llvm.coro.begin(token %id, ptr %phi)
+ call void @print(i32 0)
+ %0 = call i8 @llvm.coro.suspend(token none, i1 false)
+ switch i8 %0, label %suspend [i8 0, label %resume
+ i8 1, label %cleanup]
+
+resume:
+ call void @print(i32 1)
+ br label %cleanup
+
+cleanup:
+ %mem = call ptr @llvm.coro.free(token %id, ptr %hdl)
+ call void @free(ptr %mem)
+ br label %suspend
+
+suspend:
+ call void @llvm.coro.end(ptr %hdl, i1 false, token none)
+ ret ptr %hdl
+}
+
+define void @caller() presplitcoroutine {
+entry:
+ %ptr = call ptr @f() #0
+ ret void
+}
+
+; CHECK-LABEL: define ptr @f(
+; CHECK: %[[NEED_ALLOC:.+]] = call i1 @llvm.coro.alloc(
+; CHECK: %[[DESTROY_OR_CLEANUP:.+]] = select i1 %[[NEED_ALLOC]], ptr @f.destroy, ptr @f.cleanup
+; CHECK: %[[DESTROY_ADDR:.+]] = getelementptr inbounds i8, ptr %hdl, i64 8
+; CHECK-NEXT: store ptr %[[DESTROY_OR_CLEANUP]], ptr %[[DESTROY_ADDR]]
+
+; CHECK-LABEL: define internal void @f.resume(
+; CHECK: %[[DESTROY_ADDR:.+]] = getelementptr inbounds i8, ptr %hdl, i64 8
+; CHECK-NEXT: %[[DESTROY:.+]] = load ptr, ptr %[[DESTROY_ADDR]]
+; CHECK-NEXT: %[[IS_ELIDED:.+]] = icmp eq ptr %[[DESTROY]], @f.cleanup
+; CHECK: call void @print(i32 1)
+; CHECK: %[[CORO_FREE:.+]] = select i1 %[[IS_ELIDED]], ptr null, ptr %hdl
+; CHECK-NEXT: call void @free(ptr %[[CORO_FREE]])
+; CHECK-NEXT: ret void
+
+; CHECK-LABEL: define internal void @f.destroy(
+; CHECK: call void @free(
+
+; CHECK-LABEL: define internal void @f.cleanup(
+; CHECK-NOT: call void @free(
+; CHECK: ret void
+
+; CHECK-LABEL: define internal ptr @f.noalloc(
+; CHECK: %[[NOALLOC_DESTROY_ADDR:.+]] = getelementptr inbounds i8, ptr %{{.+}}, i64 8
+; CHECK-NEXT: store ptr @f.cleanup, ptr %[[NOALLOC_DESTROY_ADDR]]
+
+declare ptr @llvm.coro.free(token, ptr)
+declare i32 @llvm.coro.size.i32()
+declare i8 @llvm.coro.suspend(token, i1)
+declare token @llvm.coro.id(i32, ptr, ptr, ptr)
+declare i1 @llvm.coro.alloc(token)
+declare ptr @llvm.coro.begin(token, ptr)
+declare void @llvm.coro.end(ptr, i1, token)
+
+declare noalias ptr @malloc(i32) allockind("alloc,uninitialized") "alloc-family"="malloc"
+declare void @print(i32)
+declare void @free(ptr) willreturn allockind("free") "alloc-family"="malloc"
+
+attributes #0 = { coro_elide_safe }
More information about the cfe-commits
mailing list