[llvm] [CoroSplit][DebugInfo] Adjust heuristic for moving DIScope of funclets (PR #108611)
Felipe de Azevedo Piovezan via llvm-commits
llvm-commits at lists.llvm.org
Fri Sep 13 10:27:39 PDT 2024
https://github.com/felipepiovezan created https://github.com/llvm/llvm-project/pull/108611
CoroSplit has a heuristic where the scope line for funclets is adjusted to match the line of the suspend intrinsic that caused the split. This is useful as it avoids a jump on the line table from the original function declaration to the line where the split happens.
However, very often using the line of the split is not ideal: if we can avoid it, we should not have a line entry for the split location, as this would cause breakpoints by line to match against two functions: the funclet before and the funclet after the split.
This patch adjusts the heuristics to look for the first instruction with a non-zero line number after the split. In other words, this patch makes breakpoints on `await foo()` lines behave much more like a regular function call.
>From 3c80a8e6c7293b2c732dd79bda00133cdad988e7 Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan <fpiovezan at apple.com>
Date: Thu, 12 Sep 2024 15:51:38 -0700
Subject: [PATCH] [CoroSplit][DebugInfo] Adjust heuristic for moving DIScope of
funclets
CoroSplit has a heuristic where the scope line for funclets is adjusted to
match the line of the suspend intrinsic that caused the split. This is useful as
it avoids a jump on the line table from the original function declaration to the
line where the split happens.
However, very often using the line of the split is not ideal: if we can avoid
it, we should not have a line entry for the split location, as this would cause
breakpoints by line to match against two functions: the funclet before and the
funclet after the split.
This patch adjusts the heuristics to look for the first instruction with a
non-zero line number after the split. In other words, this patch makes
breakpoints on `await foo()` lines behave much more like a regular function
call.
---
llvm/lib/Transforms/Coroutines/CoroSplit.cpp | 51 +++++++++++++++----
.../Transforms/Coroutines/swift-async-dbg.ll | 12 +++--
2 files changed, 49 insertions(+), 14 deletions(-)
diff --git a/llvm/lib/Transforms/Coroutines/CoroSplit.cpp b/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
index 8ea460badaad5d..2f43c82b1a1d83 100644
--- a/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
+++ b/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
@@ -887,6 +887,45 @@ Value *CoroCloner::deriveNewFramePointer() {
llvm_unreachable("bad ABI");
}
+// Adjust the scope line of the funclet to the first line number after the
+// suspend point. This avoids a jump in the line table from the function
+// declaration (where prologue instructions are attributed to) to the suspend
+// point.
+// Only adjust the scope line when the files are the same.
+// If no candidate line number is found, fallback to the line of ActiveSuspend.
+static void updateScopeLine(Instruction *ActiveSuspend,
+ DISubprogram &SPToUpdate) {
+ if (!ActiveSuspend)
+ return;
+
+ auto *Successor = ActiveSuspend->getNextNonDebugInstruction();
+ // Corosplit splits the BB around ActiveSuspend, so the meaningful
+ // instructions are not in the same BB.
+ if (auto *Branch = dyn_cast_or_null<BranchInst>(Successor);
+ Branch && Branch->isUnconditional())
+ Successor = Branch->getSuccessor(0)->getFirstNonPHIOrDbg();
+
+ // Find the first successor of ActiveSuspend with a non-zero line location.
+ // If that matches the file of ActiveSuspend, use it.
+ for (; Successor; Successor = Successor->getNextNonDebugInstruction()) {
+ auto DL = Successor->getDebugLoc();
+ if (!DL || DL.getLine() == 0)
+ continue;
+
+ if (SPToUpdate.getFile() == DL->getFile()) {
+ SPToUpdate.setScopeLine(DL.getLine());
+ return;
+ }
+
+ break;
+ }
+
+ // If the search above failed, fallback to the location of ActiveSuspend.
+ if (auto DL = ActiveSuspend->getDebugLoc())
+ if (SPToUpdate.getFile() == DL->getFile())
+ SPToUpdate.setScopeLine(DL->getLine());
+}
+
static void addFramePointerAttrs(AttributeList &Attrs, LLVMContext &Context,
unsigned ParamIndex, uint64_t Size,
Align Alignment, bool NoAlias) {
@@ -955,18 +994,10 @@ void CoroCloner::create() {
auto &Context = NewF->getContext();
- // For async functions / continuations, adjust the scope line of the
- // clone to the line number of the suspend point. However, only
- // adjust the scope line when the files are the same. This ensures
- // line number and file name belong together. The scope line is
- // associated with all pre-prologue instructions. This avoids a jump
- // in the linetable from the function declaration to the suspend point.
if (DISubprogram *SP = NewF->getSubprogram()) {
assert(SP != OrigF.getSubprogram() && SP->isDistinct());
- if (ActiveSuspend)
- if (auto DL = ActiveSuspend->getDebugLoc())
- if (SP->getFile() == DL->getFile())
- SP->setScopeLine(DL->getLine());
+ updateScopeLine(ActiveSuspend, *SP);
+
// Update the linkage name to reflect the modified symbol name. It
// is necessary to update the linkage name in Swift, since the
// mangling changes for resume functions. It might also be the
diff --git a/llvm/test/Transforms/Coroutines/swift-async-dbg.ll b/llvm/test/Transforms/Coroutines/swift-async-dbg.ll
index 2cd0f1c42b730b..c0b7247ff49ada 100644
--- a/llvm/test/Transforms/Coroutines/swift-async-dbg.ll
+++ b/llvm/test/Transforms/Coroutines/swift-async-dbg.ll
@@ -37,9 +37,9 @@ define swifttailcc void @coroutineA(ptr swiftasync %arg) !dbg !48 {
%i7 = call ptr @llvm.coro.async.resume(), !dbg !54
%i10 = call { ptr } (i32, ptr, ptr, ...) @llvm.coro.suspend.async.sl_p0s(i32 0, ptr %i7, ptr nonnull @__swift_async_resume_get_context, ptr nonnull @coroutineA.1, ptr %i7, i64 0, i64 0, ptr %arg), !dbg !54
- %i11 = extractvalue { ptr } %i10, 0, !dbg !54
- %i12 = call ptr @__swift_async_resume_get_context(ptr %i11), !dbg !54
- call void @dont_optimize(ptr %var_with_dbg_value, ptr %var_with_dbg_declare)
+ %i11 = extractvalue { ptr } %i10, 0, !dbg !55
+ %i12 = call ptr @__swift_async_resume_get_context(ptr %i11), !dbg !55
+ call void @dont_optimize(ptr %var_with_dbg_value, ptr %var_with_dbg_declare), !dbg !100
call void @llvm.dbg.value(metadata ptr %var_with_dbg_value, metadata !50, metadata !DIExpression(DW_OP_deref)), !dbg !54
%i17 = load i32, ptr getelementptr inbounds (<{i32, i32}>, ptr @coroutineBTu, i64 0, i32 1), align 8, !dbg !54
call void @llvm.dbg.value(metadata !DIArgList(ptr %var_with_dbg_value, i32 %i17), metadata !501, metadata !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_arg, 1, DW_OP_plus, DW_OP_deref)), !dbg !54
@@ -47,7 +47,7 @@ define swifttailcc void @coroutineA(ptr swiftasync %arg) !dbg !48 {
%i19 = call swiftcc ptr @swift_task_alloc(i64 %i18), !dbg !54
; CHECK-NOT: define
; CHECK-LABEL: define {{.*}} @coroutineATY0_(
-; CHECK-SAME: ptr swiftasync %[[frame_ptr:.*]])
+; CHECK-SAME: ptr swiftasync %[[frame_ptr:.*]]) !dbg ![[ATY0:[0-9]*]]
; CHECK: #dbg_declare(ptr %[[frame_ptr]], {{.*}} !DIExpression(
; CHECK-SAME: DW_OP_LLVM_entry_value, 1, DW_OP_plus_uconst, 24)
; CHECK: #dbg_value(ptr %[[frame_ptr]], {{.*}} !DIExpression(
@@ -88,6 +88,8 @@ define swifttailcc void @coroutineA(ptr swiftasync %arg) !dbg !48 {
; CHECK-SAME: DW_OP_LLVM_entry_value, 1, DW_OP_plus_uconst, 24)
}
+; CHECK: ![[ATY0]] = {{.*}}DISubprogram(linkageName: "coroutineATY0_", {{.*}} scopeLine: 42
+
; Everything from here on is just support code for the coroutines.
@coroutineBTu = global <{i32, i32}> <{ i32 trunc (i64 sub (i64 ptrtoint (ptr @"coroutineB" to i64), i64 ptrtoint (ptr @"coroutineBTu" to i64)) to i32), i32 16 }>, align 8
@@ -172,5 +174,7 @@ declare { ptr } @llvm.coro.suspend.async.sl_p0s(i32, ptr, ptr, ...)
!71 = !DILocation(line: 0, scope: !70)
!73 = !DILocation(line: 0, scope: !72)
!54 = !DILocation(line: 6, scope: !48)
+!55 = !DILocation(line: 0, scope: !48)
!42 = !DILocation(line: 3, scope: !37)
!47 = !DILocation(line: 0, scope: !44)
+!100 = !DILocation(line: 42, scope: !48)
More information about the llvm-commits
mailing list