[llvm] [DWARF] Emit a worst-case prologue_end flag for pathological inputs (PR #107849)

Jeremy Morse via llvm-commits llvm-commits at lists.llvm.org
Mon Nov 11 08:24:12 PST 2024


https://github.com/jmorse updated https://github.com/llvm/llvm-project/pull/107849

>From 36077acbda36b64c0bb61be417305f6d7520dc7a Mon Sep 17 00:00:00 2001
From: Jeremy Morse <jeremy.morse at sony.com>
Date: Tue, 3 Sep 2024 12:22:38 +0100
Subject: [PATCH 1/3] Emit a worst-case prologue_end flag for pathological
 inputs

prologue_end usually indicates where the end of the function-initialization
lies, and is where debuggers usually choose to put the initial breakpoint
for a function. Our current algorithm piggy-backs it on the first available
source-location: which doesn't necessarily have anything to do with the
start of the function.

To avoid this in heavily-optimised code that lacks many useful source
locations, pick a worst-case "if all else fails" prologue_end location, of
the first instruction that appears to do meaningful computation. It'll be
given the function-scope line number, which should run-on from the start of
the function anyway. This means if your code is completely inverted by the
optimiser, you can at least put a breakpoint at the _start_ like you
expect, even if it's difficult to then step through.
---
 llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp    | 133 ++++++++++++++---
 llvm/test/CodeGen/X86/pseudo_cmov_lower2.ll   |   4 +-
 .../MIR/X86/dbg-prologue-backup-loc2.mir      | 134 ++++++++++++++++++
 llvm/test/DebugInfo/MIR/X86/empty-inline.mir  |   5 +-
 .../X86/dbg-prolog-end-backup-loc.ll          |  86 +++++++++++
 llvm/test/DebugInfo/X86/dbg-prolog-end.ll     |  51 +++++++
 llvm/test/DebugInfo/X86/empty-line-info.ll    |   7 +-
 llvm/test/DebugInfo/X86/loop-align-debug.ll   |   1 +
 8 files changed, 396 insertions(+), 25 deletions(-)
 create mode 100644 llvm/test/DebugInfo/MIR/X86/dbg-prologue-backup-loc2.mir
 create mode 100644 llvm/test/DebugInfo/X86/dbg-prolog-end-backup-loc.ll

diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
index 148b620c2b62b7..6830fd41b8ccce 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
@@ -2061,6 +2061,16 @@ void DwarfDebug::beginInstruction(const MachineInstr *MI) {
   unsigned LastAsmLine =
       Asm->OutStreamer->getContext().getCurrentDwarfLoc().getLine();
 
+  if (!DL && MI == PrologEndLoc) {
+    // In rare situations, we might want to place the end of the prologue
+    // somewhere that doesn't have a source location already. It should be in
+    // the entry block.
+    assert(MI->getParent() == &*MI->getMF()->begin());
+    recordSourceLine(SP->getScopeLine(), 0, SP,
+                     DWARF2_FLAG_PROLOGUE_END | DWARF2_FLAG_IS_STMT);
+    return;
+  }
+
   bool PrevInstInDiffBB = PrevInstBB && PrevInstBB != MI->getParent();
   if (DL == PrevInstLoc && !PrevInstInDiffBB) {
     // If we have an ongoing unspecified location, nothing to do here.
@@ -2131,36 +2141,121 @@ void DwarfDebug::beginInstruction(const MachineInstr *MI) {
     PrevInstLoc = DL;
 }
 
-static std::pair<const MachineInstr *, bool>
+std::pair<const MachineInstr *, bool>
 findPrologueEndLoc(const MachineFunction *MF) {
   // First known non-DBG_VALUE and non-frame setup location marks
   // the beginning of the function body.
-  const MachineInstr *LineZeroLoc = nullptr;
+  const auto &TII = *MF->getSubtarget().getInstrInfo();
+  const MachineInstr *LineZeroLoc = nullptr, *NonTrivialInst = nullptr;
   const Function &F = MF->getFunction();
 
   // Some instructions may be inserted into prologue after this function. Must
   // keep prologue for these cases.
   bool IsEmptyPrologue =
       !(F.hasPrologueData() || F.getMetadata(LLVMContext::MD_func_sanitize));
-  for (const auto &MBB : *MF) {
-    for (const auto &MI : MBB) {
-      if (!MI.isMetaInstruction()) {
-        if (!MI.getFlag(MachineInstr::FrameSetup) && MI.getDebugLoc()) {
-          // Scan forward to try to find a non-zero line number. The
-          // prologue_end marks the first breakpoint in the function after the
-          // frame setup, and a compiler-generated line 0 location is not a
-          // meaningful breakpoint. If none is found, return the first
-          // location after the frame setup.
-          if (MI.getDebugLoc().getLine())
-            return std::make_pair(&MI, IsEmptyPrologue);
-
-          LineZeroLoc = &MI;
-        }
-        IsEmptyPrologue = false;
-      }
+
+  // Helper lambda to examine each instruction and potentially return it
+  // as the prologue_end point.
+  auto ExamineInst = [&](const MachineInstr &MI)
+      -> std::optional<std::pair<const MachineInstr *, bool>> {
+    // Is this instruction trivial data shuffling or frame-setup?
+    bool isCopy = (TII.isCopyInstr(MI) ? true : false);
+    bool isTrivRemat = TII.isTriviallyReMaterializable(MI);
+    bool isFrameSetup = MI.getFlag(MachineInstr::FrameSetup);
+
+    if (!isFrameSetup && MI.getDebugLoc()) {
+      // Scan forward to try to find a non-zero line number. The
+      // prologue_end marks the first breakpoint in the function after the
+      // frame setup, and a compiler-generated line 0 location is not a
+      // meaningful breakpoint. If none is found, return the first
+      // location after the frame setup.
+      if (MI.getDebugLoc().getLine())
+        return std::make_pair(&MI, IsEmptyPrologue);
+
+      LineZeroLoc = &MI;
+    }
+
+    // Keep track of the first "non-trivial" instruction seen, i.e. anything
+    // that doesn't involve shuffling data around or is a frame-setup.
+    if (!isCopy && !isTrivRemat && !isFrameSetup && !NonTrivialInst)
+      NonTrivialInst = &MI;
+
+    IsEmptyPrologue = false;
+    return std::nullopt;
+  };
+
+  // Examine all the instructions at the start of the function. This doesn't
+  // necessarily mean just the entry block: unoptimised code can fall-through
+  // into an initial loop, and it makes sense to put the initial breakpoint on
+  // the first instruction of such a loop. However, if we pass branches, we're
+  // better off synthesising an early prologue_end.
+  auto CurBlock = MF->begin();
+  auto CurInst = CurBlock->begin();
+  while (true) {
+    // Skip empty blocks, in rare cases the entry can be empty.
+    if (CurInst == CurBlock->end()) {
+      ++CurBlock;
+      CurInst = CurBlock->begin();
+      continue;
+    }
+
+    // Check whether this non-meta instruction a good position for prologue_end.
+    if (!CurInst->isMetaInstruction()) {
+      auto FoundInst = ExamineInst(*CurInst);
+      if (FoundInst)
+        return *FoundInst;
+    }
+
+    // Try to continue searching, but use a backup-location if substantive
+    // computation is happening.
+    auto NextInst = std::next(CurInst);
+    if (NextInst == CurInst->getParent()->end()) {
+      // We've reached the end of the block. Did we just look at a terminator?
+      if (CurInst->isTerminator())
+        // Some kind of "real" control flow is occurring. At the very least
+        // we would have to start exploring the CFG, a good signal that the
+        // prologue is over.
+        break;
+
+      // If we've already fallen through into a loop, don't fall through
+      // further, use a backup-location.
+      if (CurBlock->pred_size() > 1)
+        break;
+
+      // Fall-through from entry to the next block. This is common at -O0 when
+      // there's no initialisation in the function.
+      auto NextBBIter = std::next(CurInst->getParent()->getIterator());
+      // Bail if we're also at the end of the function.
+      if (NextBBIter == MF->end())
+        break;
+      CurBlock = NextBBIter;
+      CurInst = NextBBIter->begin();
+    } else {
+      // Continue examining the current block.
+      CurInst = NextInst;
     }
   }
-  return std::make_pair(LineZeroLoc, IsEmptyPrologue);
+
+  // We didn't find a "good" prologue_end, so consider backup locations.
+  // Was there an empty-line location? Return that, and it'll have the
+  // scope-line "flow" into it when it becomes the prologue_end position.
+  if (LineZeroLoc)
+    return std::make_pair(LineZeroLoc, IsEmptyPrologue);
+
+  // We couldn't find any source-location, suggesting all meaningful information
+  // got optimised away. Set the prologue_end to be the first non-trivial
+  // instruction, which will get the scope line number. This is better than
+  // nothing.
+  // Only do this in the entry block, as we'll be giving it the scope line for
+  // the function. Return IsEmptyPrologue==true if we've picked the first
+  // instruction.
+  if (NonTrivialInst && NonTrivialInst->getParent() == &*MF->begin()) {
+    IsEmptyPrologue = NonTrivialInst == &*MF->begin()->begin();
+    return std::make_pair(NonTrivialInst, IsEmptyPrologue);
+  }
+
+  // If the entry path is empty, just don't have a prologue_end at all.
+  return std::make_pair(nullptr, IsEmptyPrologue);
 }
 
 /// Register a source line with debug info. Returns the  unique label that was
diff --git a/llvm/test/CodeGen/X86/pseudo_cmov_lower2.ll b/llvm/test/CodeGen/X86/pseudo_cmov_lower2.ll
index 19253d67c14945..63640747d2aa2c 100644
--- a/llvm/test/CodeGen/X86/pseudo_cmov_lower2.ll
+++ b/llvm/test/CodeGen/X86/pseudo_cmov_lower2.ll
@@ -203,9 +203,9 @@ declare void @llvm.dbg.value(metadata, metadata, metadata)
 ; locations in the function.
 define double @foo1_g(float %p1, double %p2, double %p3) nounwind !dbg !4 {
 ; CHECK-LABEL: foo1_g:
-; CHECK:       .file   1 "." "test.c"
-; CHECK-NEXT:  .loc    1 3 0
 ; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    .file   1 "." "test.c"
+; CHECK-NEXT:    .loc 1 3 0 prologue_end
 ; CHECK-NEXT:    xorps %xmm3, %xmm3
 ; CHECK-NEXT:    ucomiss %xmm3, %xmm0
 ; CHECK-NEXT:    movsd {{.*#+}} xmm0 = [1.25E+0,0.0E+0]
diff --git a/llvm/test/DebugInfo/MIR/X86/dbg-prologue-backup-loc2.mir b/llvm/test/DebugInfo/MIR/X86/dbg-prologue-backup-loc2.mir
new file mode 100644
index 00000000000000..378d579cee62e1
--- /dev/null
+++ b/llvm/test/DebugInfo/MIR/X86/dbg-prologue-backup-loc2.mir
@@ -0,0 +1,134 @@
+# RUN: llc %s -start-before=livedebugvalues -o - | \
+# RUN:     FileCheck %s --implicit-check-not=prologue_end
+#
+## When picking a "backup" location of the first non-trivial instruction in
+## a function, don't select a location outside of the entry block. We have to
+## give it the functions scope-line, and installing that outside of the entry
+## block is liable to be misleading.
+##
+## Produced from the C below with "clang -O2 -g -mllvm
+## -stop-before=livedebugvalues", then modified to unrotate and shift early
+## insts into the loop block. This means the MIR is meaningless, we only test
+## whether the scope-line will leak into the loop block or not.
+##
+## int glob = 0;
+## int foo(int arg, int sum) {
+##   arg += sum;
+##   while (arg) {
+##     glob--;
+##     arg %= glob;
+##   }
+##   return 0;
+## }
+#
+# CHECK-LABEL: foo:
+# CHECK:        .loc    0 2 0
+# CHECK:        # %bb.0:
+# CHECK-NEXT:   movl    %edi, %edx
+# CHECK-NEXT:   .loc    0 0 0 is_stmt 0
+# CHECK-NEXT:   .Ltmp0:
+# CHECK-NEXT:   .p2align        4, 0x90
+# CHECK-NEXT:   .LBB0_1:
+# CHECK-LABEL:  addl    %esi, %edx
+
+ 
+--- |
+  ; ModuleID = 'out2.ll'
+  source_filename = "foo.c"
+  target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"
+  target triple = "x86_64-unknown-linux-gnu"
+  
+  @glob = dso_local local_unnamed_addr global i32 0, align 4, !dbg !0
+  
+  define dso_local noundef i32 @foo(i32 noundef %arg, i32 noundef %sum) local_unnamed_addr !dbg !9 {
+  entry:
+    %add = add nsw i32 %sum, %arg
+    br label %while.body.preheader
+  
+  while.body.preheader:                             ; preds = %entry
+    %glob.promoted = load i32, ptr @glob, align 4
+    br label %while.body, !dbg !13
+  
+  while.body:                                       ; preds = %while.body, %while.body.preheader
+    %arg.addr.06 = phi i32 [ %rem, %while.body ], [ %add, %while.body.preheader ]
+    %dec35 = phi i32 [ %dec, %while.body ], [ %glob.promoted, %while.body.preheader ]
+    %dec = add nsw i32 %dec35, -1, !dbg !14
+    %0 = add i32 %dec35, -1, !dbg !16
+    %rem = srem i32 %arg.addr.06, %0, !dbg !16
+    %tobool.not = icmp eq i32 %rem, 0, !dbg !13
+    br i1 %tobool.not, label %while.cond.while.end_crit_edge, label %while.body, !dbg !13
+  
+  while.cond.while.end_crit_edge:                   ; preds = %while.body
+    store i32 %dec, ptr @glob, align 4, !dbg !14
+    br label %while.end, !dbg !13
+  
+  while.end:                                        ; preds = %while.cond.while.end_crit_edge
+    ret i32 0, !dbg !17
+  }
+  
+  !llvm.dbg.cu = !{!2}
+  !llvm.module.flags = !{!6, !7}
+  !llvm.ident = !{!8}
+  
+  !0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression())
+  !1 = distinct !DIGlobalVariable(name: "glob", scope: !2, file: !3, line: 1, type: !5, isLocal: false, isDefinition: true)
+  !2 = distinct !DICompileUnit(language: DW_LANG_C11, file: !3, producer: "clang", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, globals: !4, splitDebugInlining: false, nameTableKind: None)
+  !3 = !DIFile(filename: "foo.c", directory: "")
+  !4 = !{!0}
+  !5 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+  !6 = !{i32 7, !"Dwarf Version", i32 5}
+  !7 = !{i32 2, !"Debug Info Version", i32 3}
+  !8 = !{!"clang"}
+  !9 = distinct !DISubprogram(name: "foo", scope: !3, file: !3, line: 2, type: !10, scopeLine: 2, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !2, retainedNodes: !12)
+  !10 = !DISubroutineType(types: !11)
+  !11 = !{!5, !5, !5}
+  !12 = !{}
+  !13 = !DILocation(line: 4, column: 3, scope: !9)
+  !14 = !DILocation(line: 5, column: 9, scope: !15)
+  !15 = distinct !DILexicalBlock(scope: !9, file: !3, line: 4, column: 15)
+  !16 = !DILocation(line: 6, column: 9, scope: !15)
+  !17 = !DILocation(line: 8, column: 3, scope: !9)
+
+...
+---
+name:            foo
+alignment:       16
+tracksRegLiveness: true
+debugInstrRef:   true
+tracksDebugUserValues: true
+liveins:
+  - { reg: '$edi' }
+  - { reg: '$esi' }
+frameInfo:
+  maxAlignment:    1
+  maxCallFrameSize: 0
+  isCalleeSavedInfoValid: true
+machineFunctionInfo:
+  amxProgModel:    None
+body:             |
+  bb.0.entry:
+    liveins: $edi, $esi
+  
+    $edx = MOV32rr $edi
+  
+  bb.1.while.body (align 16):
+    successors: %bb.2(0x04000000), %bb.1(0x7c000000)
+    liveins: $ecx, $edx, $esi
+
+    renamable $edx = nsw ADD32rr killed renamable $edx, renamable $esi, implicit-def dead $eflags
+    renamable $ecx = MOV32rm $rip, 1, $noreg, @glob, $noreg :: (dereferenceable load (s32) from @glob)
+    renamable $ecx = DEC32r killed renamable $ecx, implicit-def dead $eflags
+    $eax = MOV32rr killed $edx
+    CDQ implicit-def $eax, implicit-def $edx, implicit $eax
+    IDIV32r renamable $ecx, implicit-def dead $eax, implicit-def $edx, implicit-def dead $eflags, implicit $eax, implicit $edx
+    TEST32rr renamable $edx, renamable $edx, implicit-def $eflags
+    JCC_1 %bb.1, 5, implicit killed $eflags
+  
+  bb.2.while.cond.while.end_crit_edge:
+    liveins: $ecx, $esi
+  
+    MOV32mr $rip, 1, $noreg, @glob, $noreg, killed renamable $ecx, debug-location !14 :: (store (s32) into @glob)
+    $eax = XOR32rr undef $eax, undef $eax, implicit-def dead $eflags, debug-location !17
+    RET64 $eax, debug-location !17
+
+...
diff --git a/llvm/test/DebugInfo/MIR/X86/empty-inline.mir b/llvm/test/DebugInfo/MIR/X86/empty-inline.mir
index 58775e8cd852fb..c489339b4c8aa6 100644
--- a/llvm/test/DebugInfo/MIR/X86/empty-inline.mir
+++ b/llvm/test/DebugInfo/MIR/X86/empty-inline.mir
@@ -13,11 +13,12 @@
 #
 # CHECK: Address            Line   Column File   ISA Discriminator OpIndex Flags
 # CHECK-NEXT:                ---
-# CHECK-NEXT:                 25      0      1   0             0         0 is_stmt
+# CHECK-NEXT:                 25      0      1   0             0         0 is_stmt prologue_end
 # CHECK-NEXT:                  0      0      1   0             0         0
-# CHECK-NEXT:                 29     28      1   0             0         0 is_stmt prologue_end
+# CHECK-NEXT:                 29     28      1   0             0         0 is_stmt
 # CHECK-NEXT:                 29     28      1   0             0         0 is_stmt
 # CHECK-NEXT:                 29     28      1   0             0         0 is_stmt end_sequence
+
 --- |
   source_filename = "t.ll"
   target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
diff --git a/llvm/test/DebugInfo/X86/dbg-prolog-end-backup-loc.ll b/llvm/test/DebugInfo/X86/dbg-prolog-end-backup-loc.ll
new file mode 100644
index 00000000000000..2425df608275c5
--- /dev/null
+++ b/llvm/test/DebugInfo/X86/dbg-prolog-end-backup-loc.ll
@@ -0,0 +1,86 @@
+; RUN: llc %s -o - | FileCheck %s
+
+;; This test has had source-locations removed from the prologue, to simulate
+;; heavily-optimised scenarios where a lot of debug-info gets dropped. Check
+;; that we can pick a "worst-case" prologue_end position, of the first
+;; instruction that does any meaningful computation (the add). It's better to
+;; put the prologue_end flag here rather than deeper into the loop, past the
+;; early-exit check.
+;;
+;; Generated from this code at -O2 -g in clang, with source locations then
+;; deleted.
+;;
+;; int glob = 0;
+;; int foo(int arg, int sum) {
+;;   arg += sum;
+;;   while (arg) {
+;;     glob--;
+;;     arg %= glob;
+;;   }
+;;   return 0;
+;; }
+
+; CHECK-LABEL: foo:
+;; Scope-line location:
+; CHECK:       .loc    0 2 0
+;; Entry block:
+; CHECK:        movl    %edi, %edx
+; CHECK-NEXT:   .loc    0 2 0 prologue_end
+; CHECK-NEXT:   addl    %esi, %edx
+; CHECK-NEXT:   je      .LBB0_4
+; CHECK-LABEL: # %bb.1:
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+ at glob = dso_local local_unnamed_addr global i32 0, align 4, !dbg !0
+
+define dso_local noundef i32 @foo(i32 noundef %arg, i32 noundef %sum) local_unnamed_addr !dbg !9 {
+entry:
+  %add = add nsw i32 %sum, %arg
+  %tobool.not4 = icmp eq i32 %add, 0
+  br i1 %tobool.not4, label %while.end, label %while.body.preheader
+
+while.body.preheader:
+  %glob.promoted = load i32, ptr @glob, align 4
+  br label %while.body, !dbg !14
+
+while.body:
+  %arg.addr.06 = phi i32 [ %rem, %while.body ], [ %add, %while.body.preheader ]
+  %dec35 = phi i32 [ %dec, %while.body ], [ %glob.promoted, %while.body.preheader ]
+  %dec = add nsw i32 %dec35, -1, !dbg !15
+  %rem = srem i32 %arg.addr.06, %dec, !dbg !17
+  %tobool.not = icmp eq i32 %rem, 0, !dbg !14
+  br i1 %tobool.not, label %while.cond.while.end_crit_edge, label %while.body, !dbg !14
+
+while.cond.while.end_crit_edge:
+  store i32 %dec, ptr @glob, align 4, !dbg !15
+  br label %while.end, !dbg !14
+
+while.end:
+  ret i32 0, !dbg !18
+}
+
+!llvm.dbg.cu = !{!2}
+!llvm.module.flags = !{!6, !7}
+!llvm.ident = !{!8}
+
+!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression())
+!1 = distinct !DIGlobalVariable(name: "glob", scope: !2, file: !3, line: 1, type: !5, isLocal: false, isDefinition: true)
+!2 = distinct !DICompileUnit(language: DW_LANG_C11, file: !3, producer: "clang", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, globals: !4, splitDebugInlining: false, nameTableKind: None)
+!3 = !DIFile(filename: "foo.c", directory: "")
+!4 = !{!0}
+!5 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!6 = !{i32 7, !"Dwarf Version", i32 5}
+!7 = !{i32 2, !"Debug Info Version", i32 3}
+!8 = !{!"clang"}
+!9 = distinct !DISubprogram(name: "foo", scope: !3, file: !3, line: 2, type: !10, scopeLine: 2, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !2, retainedNodes: !12)
+!10 = !DISubroutineType(types: !11)
+!11 = !{!5, !5, !5}
+!12 = !{}
+!13 = !DILocation(line: 3, column: 7, scope: !9)
+!14 = !DILocation(line: 4, column: 3, scope: !9)
+!15 = !DILocation(line: 5, column: 9, scope: !16)
+!16 = distinct !DILexicalBlock(scope: !9, file: !3, line: 4, column: 15)
+!17 = !DILocation(line: 6, column: 9, scope: !16)
+!18 = !DILocation(line: 8, column: 3, scope: !9)
diff --git a/llvm/test/DebugInfo/X86/dbg-prolog-end.ll b/llvm/test/DebugInfo/X86/dbg-prolog-end.ll
index 1703323fc7ee1d..3d29f8e266301f 100644
--- a/llvm/test/DebugInfo/X86/dbg-prolog-end.ll
+++ b/llvm/test/DebugInfo/X86/dbg-prolog-end.ll
@@ -36,6 +36,50 @@ entry:
   ret i32 %call, !dbg !16
 }
 
+;; int foo(int arg) {
+;;   while (arg)
+;;    arg--;
+;;  return 0;
+;; }
+;;
+;; In this function, the entry block will fall through to while.cond, with no
+;; instructions having source-locations. The expectations at -O0 is that we'll
+;; put prologue_end on the first instruction of the loop, after %arg.addr is
+;; initialized.
+
+; CHECK:      _bar:
+; CHECK-NEXT: Lfunc_begin2:
+; CHECK-NEXT:     .loc    1 11 0 is_stmt 1
+; CHECK-NEXT:     .cfi_startproc
+; CHECK-NEXT: ## %bb.0:
+; CHECK-NEXT:     movl    %edi, -4(%rsp)
+; CHECK-NEXT: LBB2_1:
+; CHECK-NEXT:                  ## =>This Inner Loop Header: Depth=1
+; CHECK-NEXT: Ltmp4:
+; CHECK-NEXT:     .loc    1 12 3 prologue_end
+; CHECK-NEXT:     cmpl    $0, -4(%rsp)
+
+define dso_local i32 @bar(i32 noundef %arg) !dbg !30 {
+entry:
+  %arg.addr = alloca i32, align 4
+  store i32 %arg, ptr %arg.addr, align 4
+  br label %while.cond, !dbg !37
+
+while.cond:                                       ; preds = %while.body, %entry
+  %0 = load i32, ptr %arg.addr, align 4, !dbg !38
+  %tobool = icmp ne i32 %0, 0, !dbg !37
+  br i1 %tobool, label %while.body, label %while.end, !dbg !37
+
+while.body:                                       ; preds = %while.cond
+  %1 = load i32, ptr %arg.addr, align 4, !dbg !39
+  %dec = add nsw i32 %1, -1, !dbg !39
+  store i32 %dec, ptr %arg.addr, align 4, !dbg !39
+  br label %while.cond, !dbg !37
+
+while.end:                                        ; preds = %while.cond
+  ret i32 0, !dbg !42
+}
+
 !llvm.dbg.cu = !{!0}
 !llvm.module.flags = !{!21}
 !18 = !{!1, !6}
@@ -62,3 +106,10 @@ entry:
 !20 = !{}
 !21 = !{i32 1, !"Debug Info Version", i32 3}
 !22 = !DILocation(line: 0, column: 0, scope: !17)
+!30 = distinct !DISubprogram(name: "bar", scope: !2, file: !2, line: 10, type: !3, scopeLine: 11, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !34)
+!34 = !{}
+!36 = !DILocation(line: 11, column: 13, scope: !30)
+!37 = !DILocation(line: 12, column: 3, scope: !30)
+!38 = !DILocation(line: 12, column: 10, scope: !30)
+!39 = !DILocation(line: 13, column: 8, scope: !30)
+!42 = !DILocation(line: 14, column: 3, scope: !30)
diff --git a/llvm/test/DebugInfo/X86/empty-line-info.ll b/llvm/test/DebugInfo/X86/empty-line-info.ll
index e726e5c882460c..f00403ccc194c6 100644
--- a/llvm/test/DebugInfo/X86/empty-line-info.ll
+++ b/llvm/test/DebugInfo/X86/empty-line-info.ll
@@ -9,7 +9,9 @@
 ; OPTS-LABEL: foo:
 ; OPTS-NEXT:   .Lfunc_begin0:
 ; OPTS-NEXT:   .file   0 "." "foobar.c"
-; OPTS-NEXT:   .loc    0 1 0
+; OPTS-NEXT:   .cfi_startproc
+; OPTS-NEXT:   # %bb.0:
+; OPTS-NEXT:   .loc    0 1 0 prologue_end
 ; OPTS-LABEL: bar:
 
 define dso_local noundef i32 @foo(ptr nocapture noundef writeonly %bar) local_unnamed_addr !dbg !10 {
@@ -25,8 +27,9 @@ entry:
 
 ; UNOPT-LABEL: bar:
 ; UNOPT-NEXT:   .Lfunc_begin1:
-; UNOPT-NEXT:   .loc    0 11 0
+; UNOPT-NEXT:   .cfi_startproc
 ; UNOPT-LABEL: %bb.0:
+; UNOPT-NEXT:   .loc    0 11 0 prologue_end
 ; UNOPT-NEXT:    movq    %rdi, -8(%rsp)
 ; UNOPT-NEXT:    jmp     .LBB1_1
 ; UNOPT-LABEL: .LBB1_1:
diff --git a/llvm/test/DebugInfo/X86/loop-align-debug.ll b/llvm/test/DebugInfo/X86/loop-align-debug.ll
index a0302d08faa0c3..c6a730e213e669 100644
--- a/llvm/test/DebugInfo/X86/loop-align-debug.ll
+++ b/llvm/test/DebugInfo/X86/loop-align-debug.ll
@@ -5,6 +5,7 @@
 
 ; OBJ: 1:{{.*}}nop
 
+;; prologue_end goes on start-of-loop, the call to @g.
 ;;     Address            Line   Column File   ISA Discriminator OpIndex Flags
 ; DBG: 0x0000000000000000      3      0      0   0             0       0  is_stmt
 ; DBG: 0x0000000000000001      0      0      0   0             0       0

>From c9f60272af6d01590f5b6b0c5114eee60a85c20a Mon Sep 17 00:00:00 2001
From: Jeremy Morse <jeremy.morse at sony.com>
Date: Mon, 11 Nov 2024 11:01:12 +0000
Subject: [PATCH 2/3] Review feedback

---
 llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp    | 74 +++++++++----------
 .../X86/no-non-zero-debug-loc-prologue.ll     |  6 +-
 .../MIR/X86/dbg-prologue-backup-loc2.mir      |  2 +-
 3 files changed, 41 insertions(+), 41 deletions(-)

diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
index 6830fd41b8ccce..1ecc9a7f5a4312 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
@@ -2141,12 +2141,12 @@ void DwarfDebug::beginInstruction(const MachineInstr *MI) {
     PrevInstLoc = DL;
 }
 
-std::pair<const MachineInstr *, bool>
+static std::pair<const MachineInstr *, bool>
 findPrologueEndLoc(const MachineFunction *MF) {
   // First known non-DBG_VALUE and non-frame setup location marks
   // the beginning of the function body.
   const auto &TII = *MF->getSubtarget().getInstrInfo();
-  const MachineInstr *LineZeroLoc = nullptr, *NonTrivialInst = nullptr;
+  const MachineInstr *NonTrivialInst = nullptr;
   const Function &F = MF->getFunction();
 
   // Some instructions may be inserted into prologue after this function. Must
@@ -2171,8 +2171,6 @@ findPrologueEndLoc(const MachineFunction *MF) {
       // location after the frame setup.
       if (MI.getDebugLoc().getLine())
         return std::make_pair(&MI, IsEmptyPrologue);
-
-      LineZeroLoc = &MI;
     }
 
     // Keep track of the first "non-trivial" instruction seen, i.e. anything
@@ -2209,38 +2207,32 @@ findPrologueEndLoc(const MachineFunction *MF) {
     // Try to continue searching, but use a backup-location if substantive
     // computation is happening.
     auto NextInst = std::next(CurInst);
-    if (NextInst == CurInst->getParent()->end()) {
-      // We've reached the end of the block. Did we just look at a terminator?
-      if (CurInst->isTerminator())
-        // Some kind of "real" control flow is occurring. At the very least
-        // we would have to start exploring the CFG, a good signal that the
-        // prologue is over.
-        break;
-
-      // If we've already fallen through into a loop, don't fall through
-      // further, use a backup-location.
-      if (CurBlock->pred_size() > 1)
-        break;
-
-      // Fall-through from entry to the next block. This is common at -O0 when
-      // there's no initialisation in the function.
-      auto NextBBIter = std::next(CurInst->getParent()->getIterator());
-      // Bail if we're also at the end of the function.
-      if (NextBBIter == MF->end())
-        break;
-      CurBlock = NextBBIter;
-      CurInst = NextBBIter->begin();
-    } else {
+    if (NextInst != CurInst->getParent()->end()) {
       // Continue examining the current block.
       CurInst = NextInst;
+      continue;
+    }
+
+    // We've reached the end of the block. Did we just look at a terminator?
+    if (CurInst->isTerminator()) {
+      // Some kind of "real" control flow is occurring. At the very least
+      // we would have to start exploring the CFG, a good signal that the
+      // prologue is over.
+      break;
     }
-  }
 
-  // We didn't find a "good" prologue_end, so consider backup locations.
-  // Was there an empty-line location? Return that, and it'll have the
-  // scope-line "flow" into it when it becomes the prologue_end position.
-  if (LineZeroLoc)
-    return std::make_pair(LineZeroLoc, IsEmptyPrologue);
+    // If we've already fallen through into a loop, don't fall through
+    // further, use a backup-location.
+    if (CurBlock->pred_size() > 1)
+      break;
+
+    // Fall-through from entry to the next block. This is common at -O0 when
+    // there's no initialisation in the function. Bail if we're also at the
+    // end of the function.
+    if (++CurBlock == MF->end())
+      break;
+    CurInst = CurBlock->begin();
+  }
 
   // We couldn't find any source-location, suggesting all meaningful information
   // got optimised away. Set the prologue_end to be the first non-trivial
@@ -2287,12 +2279,20 @@ DwarfDebug::emitInitialLocDirective(const MachineFunction &MF, unsigned CUID) {
   bool IsEmptyPrologue = PrologEnd.second;
 
   // If the prolog is empty, no need to generate scope line for the proc.
-  if (IsEmptyPrologue)
+  if (IsEmptyPrologue) {
     // In degenerate cases, we can have functions with no source locations
-    // at all. These want a scope line, to avoid a totally empty function.
-    // Thus, only skip scope line if there's location to place prologue_end.
-    if (PrologEndLoc)
-      return PrologEndLoc;
+    // at all, or only empty ones. These want a scope line, to avoid a totally
+    // empty function. Thus, only skip the scope line if there's location to
+    // place prologue_end.
+    if (PrologEndLoc) {
+      const DebugLoc &DL = PrologEndLoc->getDebugLoc();
+      if (!DL || DL->getLine() != 0)
+        return PrologEndLoc;
+
+      // Later, don't place the prologue_end flag on this line-zero location.
+      PrologEndLoc = nullptr;
+    }
+  }
 
   // Ensure the compile unit is created if the function is called before
   // beginFunction().
diff --git a/llvm/test/CodeGen/X86/no-non-zero-debug-loc-prologue.ll b/llvm/test/CodeGen/X86/no-non-zero-debug-loc-prologue.ll
index a9c71d1d9c36b2..6ea373b67bad41 100644
--- a/llvm/test/CodeGen/X86/no-non-zero-debug-loc-prologue.ll
+++ b/llvm/test/CodeGen/X86/no-non-zero-debug-loc-prologue.ll
@@ -1,10 +1,10 @@
-; RUN: llc -filetype=asm -mtriple=x86_64-apple-macosx12.0.0 -O0 %s -o - | FileCheck %s
+; RUN: llc -filetype=asm -mtriple=x86_64-apple-macosx12.0.0 -O0 %s -o - | FileCheck %s --implicit-check-not=prologue_end
 ; CHECK: Lfunc_begin0:
 ; CHECK-NEXT: .file{{.+}}
 ; CHECK-NEXT: .loc 1 1 0 ## test-small.c:1:0{{$}}
 ; CHECK-NEXT: .cfi_startproc
 ; CHECK-NEXT: ## %bb.{{[0-9]+}}:
-; CHECK-NEXT: .loc 1 0 1 prologue_end{{.*}}
+; CHECK-NEXT: .loc 1 0 1
 define void @test() #0 !dbg !9 {
   ret void, !dbg !12
 }
@@ -19,4 +19,4 @@ define void @test() #0 !dbg !9 {
 !9 = distinct !DISubprogram(name: "test", scope: !10, file: !10, line: 1, type: !11, scopeLine: 1, spFlags: DISPFlagDefinition, unit: !5, retainedNodes: !7)
 !10 = !DIFile(filename: "test-small.c", directory: "/Users/shubham/Development/test")
 !11 = !DISubroutineType(types: !7)
-!12 = !DILocation(line: 0, column: 1, scope: !9)
\ No newline at end of file
+!12 = !DILocation(line: 0, column: 1, scope: !9)
diff --git a/llvm/test/DebugInfo/MIR/X86/dbg-prologue-backup-loc2.mir b/llvm/test/DebugInfo/MIR/X86/dbg-prologue-backup-loc2.mir
index 378d579cee62e1..b02de42f7a5a31 100644
--- a/llvm/test/DebugInfo/MIR/X86/dbg-prologue-backup-loc2.mir
+++ b/llvm/test/DebugInfo/MIR/X86/dbg-prologue-backup-loc2.mir
@@ -3,7 +3,7 @@
 #
 ## When picking a "backup" location of the first non-trivial instruction in
 ## a function, don't select a location outside of the entry block. We have to
-## give it the functions scope-line, and installing that outside of the entry
+## give it the function's scope-line, and installing that outside of the entry
 ## block is liable to be misleading.
 ##
 ## Produced from the C below with "clang -O2 -g -mllvm

>From 8563bfe484d67b560a05dd25c72cbf4b186913a4 Mon Sep 17 00:00:00 2001
From: Jeremy Morse <jeremy.morse at sony.com>
Date: Mon, 11 Nov 2024 16:23:19 +0000
Subject: [PATCH 3/3] Reword some comments

---
 llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
index 1ecc9a7f5a4312..16e4fc18fa7d4d 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
@@ -2280,11 +2280,12 @@ DwarfDebug::emitInitialLocDirective(const MachineFunction &MF, unsigned CUID) {
 
   // If the prolog is empty, no need to generate scope line for the proc.
   if (IsEmptyPrologue) {
-    // In degenerate cases, we can have functions with no source locations
-    // at all, or only empty ones. These want a scope line, to avoid a totally
-    // empty function. Thus, only skip the scope line if there's location to
-    // place prologue_end.
+    // If there's nowhere to put a prologue_end flag, emit a scope line in case
+    // there are simply no source locations anywhere in the function.
     if (PrologEndLoc) {
+      // Avoid trying to assign prologue_end to a line-zero location.
+      // Instructions with no DebugLoc at all are fine, they'll be given the
+      // scope line nuumber.
       const DebugLoc &DL = PrologEndLoc->getDebugLoc();
       if (!DL || DL->getLine() != 0)
         return PrologEndLoc;



More information about the llvm-commits mailing list