[debuginfo][coro] Emit debug info labels for coroutine resume points #141937
+389
−85
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
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
|
@llvm/pr-subscribers-mlir @llvm/pr-subscribers-coroutines Author: Adrian Vogelsgesang (vogelsgesang) ChangesWith this commit, we add That way, debugging scripts can figure out the exact line / column at which a coroutine was suspended by looking up current While this is an artificial compiler-generated label, I did not apply the DW_AT_artificial tag to it. The DWARFv5 standard only allows that tag on type and variable definitions, but not on labels. In gdb, those line numebers can then be looked up using the command LLDB unfortunately does not parse DW_TAG_label debug information, yet. As such, this debug information is currently not useful in LLDB. Corresponding RFC in https://discourse.llvm.org/t/rfc-debug-info-for-coroutine-suspension-locations-take-2/86606 Full diff: https://github.com/llvm/llvm-project/pull/141937.diff 2 Files Affected:
diff --git a/llvm/lib/Transforms/Coroutines/CoroFrame.cpp b/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
index 2f5f1089067bf..7fe19ca0dd02a 100644
--- a/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
+++ b/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
@@ -689,9 +689,8 @@ static DIType *solveDIType(DIBuilder &Builder, Type *Ty,
static void buildFrameDebugInfo(Function &F, coro::Shape &Shape,
FrameDataInfo &FrameData) {
DISubprogram *DIS = F.getSubprogram();
- // If there is no DISubprogram for F, it implies the Function are not compiled
- // with debug info. So we also don't need to generate debug info for the frame
- // neither.
+ // If there is no DISubprogram for F, it implies the function is compiled without
+ // debug info. So we also don't generate debug info for the frame, either.
if (!DIS || !DIS->getUnit() ||
!dwarf::isCPlusPlus(
(dwarf::SourceLanguage)DIS->getUnit()->getSourceLanguage()) ||
diff --git a/llvm/lib/Transforms/Coroutines/CoroSplit.cpp b/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
index f9a6c70fedc2d..d84886cd1ca4e 100644
--- a/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
+++ b/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
@@ -42,6 +42,7 @@
#include "llvm/IR/CFG.h"
#include "llvm/IR/CallingConv.h"
#include "llvm/IR/Constants.h"
+#include "llvm/IR/DIBuilder.h"
#include "llvm/IR/DataLayout.h"
#include "llvm/IR/DebugInfo.h"
#include "llvm/IR/DerivedTypes.h"
@@ -302,7 +303,7 @@ static void replaceFallthroughCoroEnd(AnyCoroEndInst *End,
}
// Mark a coroutine as done, which implies that the coroutine is finished and
-// never get resumed.
+// never gets resumed.
//
// In resume-switched ABI, the done state is represented by storing zero in
// ResumeFnAddr.
@@ -1492,6 +1493,14 @@ struct SwitchCoroutineSplitter {
static void createResumeEntryBlock(Function &F, coro::Shape &Shape) {
LLVMContext &C = F.getContext();
+ DIBuilder DBuilder(*F.getParent(), /*AllowUnresolved*/ false);
+ DISubprogram *DIS = F.getSubprogram();
+ // If there is no DISubprogram for F, it implies the function is compiled without
+ // debug info. So we also don't generate debug info for the suspension points, either.
+ bool AddDebugLabels = (DIS && DIS->getUnit() &&
+ (DIS->getUnit()->getEmissionKind() == DICompileUnit::DebugEmissionKind::FullDebug ||
+ DIS->getUnit()->getEmissionKind() == DICompileUnit::DebugEmissionKind::LineTablesOnly));
+
// resume.entry:
// %index.addr = getelementptr inbounds %f.Frame, %f.Frame* %FramePtr, i32
// 0, i32 2 % index = load i32, i32* %index.addr switch i32 %index, label
@@ -1514,6 +1523,7 @@ struct SwitchCoroutineSplitter {
Builder.CreateSwitch(Index, UnreachBB, Shape.CoroSuspends.size());
Shape.SwitchLowering.ResumeSwitch = Switch;
+ // Split all coro.suspend calls
size_t SuspendIndex = 0;
for (auto *AnyS : Shape.CoroSuspends) {
auto *S = cast<CoroSuspendInst>(AnyS);
@@ -1552,6 +1562,7 @@ struct SwitchCoroutineSplitter {
// br label %resume.0.landing
//
// resume.0: ; <--- jump from the switch in the resume.entry
+ // XXX: label
// %0 = tail call i8 @llvm.coro.suspend(token none, i1 false)
// br label %resume.0.landing
//
@@ -1574,11 +1585,21 @@ struct SwitchCoroutineSplitter {
PN->addIncoming(Builder.getInt8(-1), SuspendBB);
PN->addIncoming(S, ResumeBB);
+ if (AddDebugLabels) {
+ if (DebugLoc SuspendLoc = S->getDebugLoc()) {
+ std::string labelName = ("__coro_resume_" + Twine(SuspendIndex)).str();
+ DILocation& DILoc = *SuspendLoc.get();
+ DILabel *ResumeLabel = DBuilder.createLabel(DIS, labelName, DILoc.getFile(), SuspendLoc.getLine());
+ DBuilder.insertLabel(ResumeLabel, &DILoc, ResumeBB->begin());
+ }
+ }
+
++SuspendIndex;
}
Builder.SetInsertPoint(UnreachBB);
Builder.CreateUnreachable();
+ DBuilder.finalize();
Shape.SwitchLowering.ResumeEntryBlock = NewEntry;
}
|
vogelsgesang
force-pushed
the
coro-suspend-labels-2
branch
from
8629fc6 to
8eef12d
Compare
vogelsgesang
commented
May 29, 2025
|
CC @iains @ArsenArsen, since from the gcc commit log it seems you are working on coroutine support in gcc. Would be great if clang/LLVM and gcc could agree on a common approach to emit debug info for coroutine suspension points 🙂 See https://discourse.llvm.org/t/rfc-debug-info-for-coroutine-suspension-locations-take-2/86606 for the actual discussion |
vogelsgesang
force-pushed
the
coro-suspend-labels-2
branch
from
8eef12d to
831267a
Compare
llvmbot
added
clang:codegen
vogelsgesang
force-pushed
the
coro-suspend-labels-2
branch
from
8bb6424 to
a251525
Compare
|
@ChuanqiXu9 @dwblaikie please let me know if you feel comfortable reviewing this change or if we also need input from others 🙂 |
I like the work but my knowledge to dwarf is limited. But I'd like to approve this if no one gives feedbacks. I do think it is better to move on. |
|
Could you review clang and the coroutine-specific pieces? I think the DWARF and |
ChuanqiXu9
approved these changes
Jun 27, 2025
What I would have had in mind: The integration in the
Thanks! I will still wait for a 2nd review from someone with debuginfo expertise. We still have 3 weeks before the branch cut for clang-21, so we are not in a hurry :) |
|
Nevermind, I got confused. This code path does not influence Swift, afaict |
Yup, Swift bypasses the problem by not having a common entry point for all continuation points; instead, the continuation is always the start of a unique function with a line table entry that also points to the correct line number. |
With this commit, we add `DILabel` debug infos (lowering to DW_TAG_label in DWARF) to the various resume points of a coroutine. Those labels use the naming convention `__coro_resume_<N>` where `<N>` is the suspension point id. That way, debugging scripts can figure out the exact line / column at which a coroutine was suspended by looking up current `__coro_index` value inside the coroutines frame, and then searching for the corresponding `__coro_resume_<N>` inside the coroutine's resume function. While this is an artificial compiler-generated label, I did not apply the DW_AT_artificial tag to it. The DWARFv5 standard only allows that tag on type and variable definitions, but not on labels. In gdb, those line numebers can then be looked up using the command `info line -function my_coroutine -label __coro_resume_1`. LLDB unfortunately does not parse DW_TAG_label debug information, yet. As such, this debug information is currently not useful in LLDB.
vogelsgesang
force-pushed
the
coro-suspend-labels-2
branch
from
7d34cac to
37dd0d1
Compare
tromey
reviewed
Jul 1, 2025
tromey
reviewed
Jul 1, 2025
|
FWIW I read through the DWARF/debuginfo bits and it looked reasonable to me. I found a couple small nits. I didn't read the other parts though. |
pogo59
reviewed
Jul 1, 2025
vogelsgesang
force-pushed
the
coro-suspend-labels-2
branch
from
6efe94b to
83f23e0
Compare
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/123/builds/22644 Here is the relevant piece of the build log for the reference |
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.
RFC on discourse: https://discourse.llvm.org/t/rfc-debug-info-for-coroutine-suspension-locations-take-2/86606
With this commit, we add
DILabeldebug infos to the resume points of a coroutine. Those labels can be used by debugging scripts to figure out the exact line and column at which a coroutine was suspended by looking up current__coro_indexvalue inside the coroutines frame, and then searching for the corresponding label inside the coroutine's resume function.The DWARF information generated for such a label looks like:
The labels can be mapped to their corresponding
__coro_idxvalues either via their naming convention__coro_resume_<N>or using the newDW_AT_LLVM_coro_suspend_idxattribute. In gdb, those line numebrs can be looked up usinginfo line -function my_coroutine -label __coro_resume_1. LLDB unfortunately does not understand DW_TAG_label debug information, yet.Given this is an artificial compiler-generated label, I did apply the DW_AT_artificial tag to it. The DWARFv5 standard only allows that tag on type and variable definitions, but this is a natural extension and was also blessed in the RFC on discourse.
Also, this commit adds
DW_AT_decl_columnto labels, not only for coroutines but also for normal C and C++ labels. While not strictly necessary, I am doing so now because it would be harder to do so later without breaking the binary LLVM-IR formatDrive-by fixes: While reading the existing test cases to understand how to write my own test case, I did a couple of small typo fixes and comment improvements