Skip to content

[Coroutines] Mark parameter allocas with coro.outside.frame metadata #127653

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.

Sign up for GitHub

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

Merged
merged 8 commits into from
Feb 28, 2025
Merged

[Coroutines] Mark parameter allocas with coro.outside.frame metadata #127653

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
cde82a2
[Coroutines] Mark parameter allocas with coro.outside.frame metadata
zmodem Feb 18, 2025
4605efb
fix the TrivialABI move ctor
zmodem Feb 18, 2025
26e2610
introduce and use an @llvm.coro.outside.frame intrinsic instead
zmodem Feb 20, 2025
b88b064
add x86_64-pc-win32 test case
zmodem Feb 21, 2025
225c37c
handle coro.outside.frame in mem2reg
zmodem Feb 24, 2025
acb4146
add nocapture and memory(none) to the intrinsic
zmodem Feb 24, 2025
52a918e
revert back to using the metadata
zmodem Feb 27, 2025
ce9cefc
expand comment
zmodem Feb 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions clang/lib/CodeGen/CGCoroutine.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -855,6 +855,20 @@ void CodeGenFunction::EmitCoroutineBody(const CoroutineBodyStmt &S) {
// Create parameter copies. We do it before creating a promise, since an
// evolution of coroutine TS may allow promise constructor to observe
// parameter copies.
for (const ParmVarDecl *Parm : FnArgs) {
// If the original param is in an alloca, exclude it from the coroutine
// frame. The parameter copy will be part of the frame, but the original
// parameter memory should remain on the stack. This is necessary to
// ensure that parameters destroyed in callees, as with `trivial_abi` or
// 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(), {}));
}
}
for (auto *PM : S.getParamMoves()) {
EmitStmt(PM);
ParamReplacer.addCopy(cast<DeclStmt>(PM));
Expand Down
73 changes: 64 additions & 9 deletions clang/test/CodeGenCoroutines/coro-params.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// Vefifies that parameter copies are used in the body of the coroutine
// Verifies that parameter copies are used to construct the promise type, if that type has a matching constructor
// RUN: %clang_cc1 -std=c++20 -triple=x86_64-unknown-linux-gnu -emit-llvm -o - %s -disable-llvm-passes -fexceptions | FileCheck %s
// RUN: %clang_cc1 -std=c++20 -triple=x86_64-pc-win32 -emit-llvm -o - %s -disable-llvm-passes -fexceptions | FileCheck %s --check-prefix=MSABI

namespace std {
template <typename... T> struct coroutine_traits;
Expand Down Expand Up @@ -59,46 +60,65 @@ struct MoveAndCopy {
~MoveAndCopy();
};

void consume(int,int,int) noexcept;
struct [[clang::trivial_abi]] TrivialABI {
int val;
TrivialABI(TrivialABI&&) noexcept;
~TrivialABI();
};

void consume(int,int,int,int) noexcept;

// TODO: Add support for CopyOnly params
// CHECK: define{{.*}} void @_Z1fi8MoveOnly11MoveAndCopy(i32 noundef %val, ptr noundef %[[MoParam:.+]], ptr noundef %[[McParam:.+]]) #0 personality ptr @__gxx_personality_v0
void f(int val, MoveOnly moParam, MoveAndCopy mcParam) {
// 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: 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]]) #
// CHECK-NEXT: call void @llvm.lifetime.start.p0(
// CHECK-NEXT: invoke void @_ZNSt16coroutine_traitsIJvi8MoveOnly11MoveAndCopyEE12promise_typeC1Ev(
// CHECK-NEXT: call void @_ZN10TrivialABIC1EOS_(ptr {{[^,]*}} %[[TrivialCopy]], ptr {{[^,]*}} %[[TrivialAlloca]])
// CHECK-NEXT: call void @llvm.lifetime.start.p0(
// CHECK-NEXT: invoke void @_ZNSt16coroutine_traitsIJvi8MoveOnly11MoveAndCopy10TrivialABIEE12promise_typeC1Ev(

// CHECK: call void @_ZN14suspend_always12await_resumeEv(
// CHECK: %[[IntParam:.+]] = load i32, ptr %{{.*}}
// CHECK: %[[MoGep:.+]] = getelementptr inbounds nuw %struct.MoveOnly, ptr %[[MoCopy]], i32 0, i32 0
// CHECK: %[[MoVal:.+]] = load i32, ptr %[[MoGep]]
// CHECK: %[[McGep:.+]] = getelementptr inbounds nuw %struct.MoveAndCopy, ptr %[[McCopy]], i32 0, i32 0
// CHECK: %[[McGep:.+]] = getelementptr inbounds nuw %struct.MoveAndCopy, ptr %[[McCopy]], i32 0, i32 0
// CHECK: %[[McVal:.+]] = load i32, ptr %[[McGep]]
// CHECK: call void @_Z7consumeiii(i32 noundef %[[IntParam]], i32 noundef %[[MoVal]], i32 noundef %[[McVal]])
// CHECK: %[[TrivialGep:.+]] = getelementptr inbounds nuw %struct.TrivialABI, ptr %[[TrivialCopy]], i32 0, i32 0
// CHECK: %[[TrivialVal:.+]] = load i32, ptr %[[TrivialGep]]
// CHECK: call void @_Z7consumeiiii(i32 noundef %[[IntParam]], i32 noundef %[[MoVal]], i32 noundef %[[McVal]], i32 noundef %[[TrivialVal]])

consume(val, moParam.val, mcParam.val);
consume(val, moParam.val, mcParam.val, trivialParam.val);
co_return;

// Skip to final suspend:
// CHECK: call void @_ZNSt16coroutine_traitsIJvi8MoveOnly11MoveAndCopyEE12promise_type13final_suspendEv(
// CHECK: call void @_ZNSt16coroutine_traitsIJvi8MoveOnly11MoveAndCopy10TrivialABIEE12promise_type13final_suspendEv(
// CHECK: call void @_ZN14suspend_always12await_resumeEv(

// Destroy promise, then parameter copies:
// CHECK: call void @_ZNSt16coroutine_traitsIJvi8MoveOnly11MoveAndCopyEE12promise_typeD1Ev(ptr {{[^,]*}} %__promise)
// CHECK: call void @_ZNSt16coroutine_traitsIJvi8MoveOnly11MoveAndCopy10TrivialABIEE12promise_typeD1Ev(ptr {{[^,]*}} %__promise)
// CHECK-NEXT: call void @llvm.lifetime.end.p0(
// CHECK-NEXT: call void @_ZN10TrivialABID1Ev(ptr {{[^,]*}} %[[TrivialCopy]])
// CHECK-NEXT: call void @llvm.lifetime.end.p0(
// CHECK-NEXT: call void @_ZN11MoveAndCopyD1Ev(ptr {{[^,]*}} %[[McCopy]])
// CHECK-NEXT: call void @llvm.lifetime.end.p0(
// CHECK-NEXT: call void @_ZN8MoveOnlyD1Ev(ptr {{[^,]*}} %[[MoCopy]]
// CHECK-NEXT: call void @llvm.lifetime.end.p0(
// CHECK-NEXT: call void @llvm.lifetime.end.p0(
// 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 @_ZN10TrivialABID1Ev(ptr {{[^,]*}} %[[TrivialAlloca]])
}

// CHECK-LABEL: void @_Z16dependent_paramsI1A1BEvT_T0_S3_(ptr noundef %x, ptr noundef %0, ptr noundef %y)
Expand Down Expand Up @@ -190,3 +210,38 @@ method some_class::good_coroutine_calls_custom_constructor(float) {
// CHECK: invoke void @_ZNSt16coroutine_traitsIJ6methodR10some_classfEE12promise_typeC1ES2_f(ptr {{[^,]*}} %__promise, ptr noundef nonnull align 1 dereferenceable(1) %{{.+}}, float
co_return;
}


struct MSParm {
int val;
~MSParm();
};

void consume(int) noexcept;

// Similarly to the [[clang::trivial_abi]] parameters, with the MSVC ABI
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, this is a good edge case to motivate generalizing this for all parameters.

// parameters are also destroyed by the callee, and on x86-64 such parameters
// may get passed in registers. In that case it's again important that the
// parameter's local alloca does not become part of the coro frame since that
// 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

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
// MSABI: %[[Val:.+]] = load i32, ptr %[[ValPtr]]
// MSABI: call void @"?consume@@YAXH@Z"(i32{{.*}} %[[Val]])

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 @"??1MSParm@@QEAA@XZ"(ptr{{.*}} %[[ParamAlloca]])
}
Loading