[Clang] fix coroutine await suspend wrapper linkage types #151224
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR fixes the linkage type for coroutine await suspend wrapper functions from private to internal linkage to improve debugging support. The change enables debugging facilities to properly symbolize these wrapper function frames.
- Changed
PrivateLinkagetoInternalLinkagefor await suspend wrapper functions - Updated test expectations to verify the
internallinkage in generated code
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| clang/lib/CodeGen/CGCoroutine.cpp | Modified the generateAwaitSuspendWrapper function to use InternalLinkage instead of PrivateLinkage |
| clang/test/CodeGenCoroutines/coro-await.cpp | Updated test expectations to check for internal linkage in the generated wrapper functions |
|
|
||
| llvm::Function *Fn = llvm::Function::Create( | ||
| LTy, llvm::GlobalValue::PrivateLinkage, FuncName, &CGM.getModule()); | ||
| LTy, llvm::GlobalValue::InternalLinkage, FuncName, &CGM.getModule()); |
There was a problem hiding this comment.
Changing the linkage from PrivateLinkage to InternalLinkage for await suspend wrapper functions could affect control flow analysis and performance profiling. Since these functions are part of coroutine control flow mechanisms, this change may impact how performance profilers and debug information track execution through coroutine suspend points. Consider whether this linkage change could interfere with accurate profiling data collection for coroutine execution paths.
llvmbot
added
clang
Member
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-coroutines Author: Yuxuan Chen (yuxuanchen1997) ChangesWe currently create these wrappers in private linkage. This results in debugging facilities not being able to symbolize its frame. For example, this test in folly symbolizer would fail. Change it to internal linkage for the benefit of debugging. Full diff: https://github.com/llvm/llvm-project/pull/151224.diff 2 Files Affected:
diff --git a/clang/lib/CodeGen/CGCoroutine.cpp b/clang/lib/CodeGen/CGCoroutine.cpp
index 5ee908922b5a3..827385f9c1a1f 100644
--- a/clang/lib/CodeGen/CGCoroutine.cpp
+++ b/clang/lib/CodeGen/CGCoroutine.cpp
@@ -435,7 +435,7 @@ CodeGenFunction::generateAwaitSuspendWrapper(Twine const &CoroName,
llvm::FunctionType *LTy = CGM.getTypes().GetFunctionType(FI);
llvm::Function *Fn = llvm::Function::Create(
- LTy, llvm::GlobalValue::PrivateLinkage, FuncName, &CGM.getModule());
+ LTy, llvm::GlobalValue::InternalLinkage, FuncName, &CGM.getModule());
Fn->addParamAttr(0, llvm::Attribute::AttrKind::NonNull);
Fn->addParamAttr(0, llvm::Attribute::AttrKind::NoUndef);
diff --git a/clang/test/CodeGenCoroutines/coro-await.cpp b/clang/test/CodeGenCoroutines/coro-await.cpp
index 8dd53a712b16f..7b7d14109d0f8 100644
--- a/clang/test/CodeGenCoroutines/coro-await.cpp
+++ b/clang/test/CodeGenCoroutines/coro-await.cpp
@@ -100,7 +100,7 @@ extern "C" void f0() {
// CHECK: call i8 @llvm.coro.suspend(token %[[FINALSP_ID]], i1 true)
// Await suspend wrapper
- // CHECK: define{{.*}} @f0.__await_suspend_wrapper__await(ptr {{[^,]*}} %[[AWAITABLE_ARG:.+]], ptr {{[^,]*}} %[[FRAME_ARG:.+]])
+ // CHECK: define internal {{.*}} @f0.__await_suspend_wrapper__await(ptr {{[^,]*}} %[[AWAITABLE_ARG:.+]], ptr {{[^,]*}} %[[FRAME_ARG:.+]])
// CHECK: store ptr %[[AWAITABLE_ARG]], ptr %[[AWAITABLE_TMP:.+]],
// CHECK: store ptr %[[FRAME_ARG]], ptr %[[FRAME_TMP:.+]],
// CHECK: %[[AWAITABLE:.+]] = load ptr, ptr %[[AWAITABLE_TMP]]
@@ -162,7 +162,7 @@ extern "C" void f1(int) {
// CHECK: call void @_ZN13suspend_maybe12await_resumeEv(ptr {{[^,]*}} %[[AWAITABLE]])
// Await suspend wrapper
- // CHECK: define {{.*}} i1 @f1.__await_suspend_wrapper__yield(ptr {{[^,]*}} %[[AWAITABLE_ARG:.+]], ptr {{[^,]*}} %[[FRAME_ARG:.+]])
+ // CHECK: define internal {{.*}} i1 @f1.__await_suspend_wrapper__yield(ptr {{[^,]*}} %[[AWAITABLE_ARG:.+]], ptr {{[^,]*}} %[[FRAME_ARG:.+]])
// CHECK: store ptr %[[AWAITABLE_ARG]], ptr %[[AWAITABLE_TMP:.+]],
// CHECK: store ptr %[[FRAME_ARG]], ptr %[[FRAME_TMP:.+]],
// CHECK: %[[AWAITABLE:.+]] = load ptr, ptr %[[AWAITABLE_TMP]]
@@ -379,7 +379,7 @@ extern "C" void TestTailcall() {
// CHECK-NEXT: ]
// Await suspend wrapper
- // CHECK: define {{.*}} ptr @TestTailcall.__await_suspend_wrapper__await(ptr {{[^,]*}} %[[AWAITABLE_ARG:.+]], ptr {{[^,]*}} %[[FRAME_ARG:.+]])
+ // CHECK: define internal {{.*}} ptr @TestTailcall.__await_suspend_wrapper__await(ptr {{[^,]*}} %[[AWAITABLE_ARG:.+]], ptr {{[^,]*}} %[[FRAME_ARG:.+]])
// CHECK: store ptr %[[AWAITABLE_ARG]], ptr %[[AWAITABLE_TMP:.+]],
// CHECK: store ptr %[[FRAME_ARG]], ptr %[[FRAME_TMP:.+]],
// CHECK: %[[AWAITABLE:.+]] = load ptr, ptr %[[AWAITABLE_TMP]]
|
Member
|
@llvm/pr-subscribers-clang-codegen Author: Yuxuan Chen (yuxuanchen1997) ChangesWe currently create these wrappers in private linkage. This results in debugging facilities not being able to symbolize its frame. For example, this test in folly symbolizer would fail. Change it to internal linkage for the benefit of debugging. Full diff: https://github.com/llvm/llvm-project/pull/151224.diff 2 Files Affected:
diff --git a/clang/lib/CodeGen/CGCoroutine.cpp b/clang/lib/CodeGen/CGCoroutine.cpp
index 5ee908922b5a3..827385f9c1a1f 100644
--- a/clang/lib/CodeGen/CGCoroutine.cpp
+++ b/clang/lib/CodeGen/CGCoroutine.cpp
@@ -435,7 +435,7 @@ CodeGenFunction::generateAwaitSuspendWrapper(Twine const &CoroName,
llvm::FunctionType *LTy = CGM.getTypes().GetFunctionType(FI);
llvm::Function *Fn = llvm::Function::Create(
- LTy, llvm::GlobalValue::PrivateLinkage, FuncName, &CGM.getModule());
+ LTy, llvm::GlobalValue::InternalLinkage, FuncName, &CGM.getModule());
Fn->addParamAttr(0, llvm::Attribute::AttrKind::NonNull);
Fn->addParamAttr(0, llvm::Attribute::AttrKind::NoUndef);
diff --git a/clang/test/CodeGenCoroutines/coro-await.cpp b/clang/test/CodeGenCoroutines/coro-await.cpp
index 8dd53a712b16f..7b7d14109d0f8 100644
--- a/clang/test/CodeGenCoroutines/coro-await.cpp
+++ b/clang/test/CodeGenCoroutines/coro-await.cpp
@@ -100,7 +100,7 @@ extern "C" void f0() {
// CHECK: call i8 @llvm.coro.suspend(token %[[FINALSP_ID]], i1 true)
// Await suspend wrapper
- // CHECK: define{{.*}} @f0.__await_suspend_wrapper__await(ptr {{[^,]*}} %[[AWAITABLE_ARG:.+]], ptr {{[^,]*}} %[[FRAME_ARG:.+]])
+ // CHECK: define internal {{.*}} @f0.__await_suspend_wrapper__await(ptr {{[^,]*}} %[[AWAITABLE_ARG:.+]], ptr {{[^,]*}} %[[FRAME_ARG:.+]])
// CHECK: store ptr %[[AWAITABLE_ARG]], ptr %[[AWAITABLE_TMP:.+]],
// CHECK: store ptr %[[FRAME_ARG]], ptr %[[FRAME_TMP:.+]],
// CHECK: %[[AWAITABLE:.+]] = load ptr, ptr %[[AWAITABLE_TMP]]
@@ -162,7 +162,7 @@ extern "C" void f1(int) {
// CHECK: call void @_ZN13suspend_maybe12await_resumeEv(ptr {{[^,]*}} %[[AWAITABLE]])
// Await suspend wrapper
- // CHECK: define {{.*}} i1 @f1.__await_suspend_wrapper__yield(ptr {{[^,]*}} %[[AWAITABLE_ARG:.+]], ptr {{[^,]*}} %[[FRAME_ARG:.+]])
+ // CHECK: define internal {{.*}} i1 @f1.__await_suspend_wrapper__yield(ptr {{[^,]*}} %[[AWAITABLE_ARG:.+]], ptr {{[^,]*}} %[[FRAME_ARG:.+]])
// CHECK: store ptr %[[AWAITABLE_ARG]], ptr %[[AWAITABLE_TMP:.+]],
// CHECK: store ptr %[[FRAME_ARG]], ptr %[[FRAME_TMP:.+]],
// CHECK: %[[AWAITABLE:.+]] = load ptr, ptr %[[AWAITABLE_TMP]]
@@ -379,7 +379,7 @@ extern "C" void TestTailcall() {
// CHECK-NEXT: ]
// Await suspend wrapper
- // CHECK: define {{.*}} ptr @TestTailcall.__await_suspend_wrapper__await(ptr {{[^,]*}} %[[AWAITABLE_ARG:.+]], ptr {{[^,]*}} %[[FRAME_ARG:.+]])
+ // CHECK: define internal {{.*}} ptr @TestTailcall.__await_suspend_wrapper__await(ptr {{[^,]*}} %[[AWAITABLE_ARG:.+]], ptr {{[^,]*}} %[[FRAME_ARG:.+]])
// CHECK: store ptr %[[AWAITABLE_ARG]], ptr %[[AWAITABLE_TMP:.+]],
// CHECK: store ptr %[[FRAME_ARG]], ptr %[[FRAME_TMP:.+]],
// CHECK: %[[AWAITABLE:.+]] = load ptr, ptr %[[AWAITABLE_TMP]]
|
hokein
approved these changes
Jul 29, 2025
Collaborator
hokein
left a comment
hokein
left a comment
There was a problem hiding this comment.
I'm not familiar with LLVM middle/back-end, but this change looks good from my side.
yuxuanchen1997
deleted the
users/yuxuanchen1997/clang-coro-await-suspend-wrapper-internal-linkage
branch
dpaoliello
added a commit
that referenced
this pull request
Sep 18, 2025
`__NoopCoro_ResumeDestroy` currently has private linkage, which causes [issues for Arm64EC](#158341). The Arm64EC lowering is trying to mangle and add thunks for `__NoopCoro_ResumeDestroy`, since it sees that it's address is taken (and, therefore, might be called from x64 code via a function pointer). MSVC's linker requires that the function be placed in COMDAT (`LNK1361: non COMDAT symbol '.L#__NoopCoro_ResumeDestroy' in hybrid binary`) which trips an assert in the verifier (`comdat global value has private linkage`) and the subsequent linking step fails since the private symbol isn't in the symbol table. Since there is no reason to use private linkage for `__NoopCoro_ResumeDestroy` and other coro related functions have also been [switched to internal linkage to improve debugging](#151224), this change switches to using internal linkage. Fixes #158341
llvm-sync bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Sep 18, 2025
`__NoopCoro_ResumeDestroy` currently has private linkage, which causes [issues for Arm64EC](llvm/llvm-project#158341). The Arm64EC lowering is trying to mangle and add thunks for `__NoopCoro_ResumeDestroy`, since it sees that it's address is taken (and, therefore, might be called from x64 code via a function pointer). MSVC's linker requires that the function be placed in COMDAT (`LNK1361: non COMDAT symbol '.L#__NoopCoro_ResumeDestroy' in hybrid binary`) which trips an assert in the verifier (`comdat global value has private linkage`) and the subsequent linking step fails since the private symbol isn't in the symbol table. Since there is no reason to use private linkage for `__NoopCoro_ResumeDestroy` and other coro related functions have also been [switched to internal linkage to improve debugging](llvm/llvm-project#151224), this change switches to using internal linkage. Fixes #158341
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We currently create these wrappers in private linkage. This results in debugging facilities not being able to symbolize its frame. For example, this test in folly symbolizer would fail.
Change it to internal linkage for the benefit of debugging.