[CoroSplit][DebugInfo] Adjust heuristic for moving DIScope of funclets

[felipepiovezan]

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.

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-coroutines

Author: Felipe de Azevedo Piovezan (felipepiovezan)

Changes

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.

---

Full diff: https://github.com/llvm/llvm-project/pull/108611.diff

2 Files Affected:

• (modified) llvm/lib/Transforms/Coroutines/CoroSplit.cpp (+41-10)
• (modified) llvm/test/Transforms/Coroutines/swift-async-dbg.ll (+8-4)

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)