[CoroSplit][DebugInfo] Adjust heuristic for moving DIScope of funclets (#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.

Author: felipepiovezan

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

llvm/test/Transforms/Coroutines/swift-async-dbg.ll

@@ -37,17 +37,17 @@ 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
   %i18 = zext i32 %i17 to i64, !dbg !54
   %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)