Do not emit prologue_end for line 0 locs if there is a non-zero loc present

[rastogishubham]

Summary

This change fixes a bug where the compiler generates a prologue_end for line 0 locs. That is because line 0 is not associated with any source location, so there should not be a prolgoue_end at a location that doesn't correspond to a source location.

There were some LLVM tests that were explicitly checking for line 0 prologue_end's as well since I believe that to be incorrect, I had to change those tests as well.

[probinson]

llvm/test/CodeGen/X86/line-zero-prologue-end.ll

15

This seems like an excessive amount of metadata for a test that cares only about line numbers. I assume you generated this from some Swift example; could you compile it with -gline-tables-only instead of just -g to reduce the fluff?

llvm/test/DebugInfo/MIR/X86/debug-loc-0.mir

8

This change looks like it causes the test to stop verifying what the comment on line 1 says: collapse line-0 directives. The changed test would pass if we emitted

.loc 1 0 0
.loc 1 0 0
.loc 1 37 1 prologue_end

which is not what this test wants.

[rastogishubham]

llvm/test/CodeGen/X86/line-zero-prologue-end.ll

15

Thanks for the feedback Paul, yes this is a swift example, I will write a small test.c file and try to get a smaller testcase. Thanks

llvm/test/DebugInfo/MIR/X86/debug-loc-0.mir

8

I see what my error is here, so I assume this test is trying to see that multiple line 0 locs should be collapsed into one. So all I need to do is something like.

# CHECK: Ltmp0:
# CHECK: .loc 1 0 0
# CHECK-NOT: .loc 1 0 0

[rastogishubham]

llvm/test/DebugInfo/MIR/X86/debug-loc-0.mir

8

I meant:

# CHECK: Ltmp0:
# CHECK: .loc 1 0 0
# CHECK-NOT: .loc 1 0 0
# CHECK: .loc 1 37 1 prologue_end

[rastogishubham]

As suggested by Adrian and Paul, I have reduced the original testcase to be smaller by using a small C sample and compiling only with -gline-tables-only. I have also added another test case where if the only .loc is a line 0 loc, we want to have a prologue_end there, and only move the prologue_end to a non-zero .loc if it exists.

I have also edited my change to llvm/test/DebugInfo/MIR/X86/debug-loc-0.mir to be consistent with what it is testing

[rastogishubham]

Fixed the clang-tidy warnings

[probinson]

It may feel like I'm being a bit picky, but the prologue_end business is a bit picky, and it's helpful to have the tests be as clear as possible about what's expected.

llvm/test/CodeGen/X86/line-zero-prologue-end.ll

5

Should this have prologue_end on it?

8

Should this check that prologue_end is *not* on it?

26

Please make !9 and !14 use different line numbers, to make it more obvious which one is the source for the .loc directive.

[aprantl]

Thanks, this LGTM after addressing all of @probinson s comments!

llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp

2097

Maybe we should also explain the rationale behind this:

// 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.

2100

This is also a problem with the original code, but findPrologueEndLoc should probably not scan past the first basic block. Your patch doesn't make this any worse though.

[rastogishubham]

llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp

2097

Completely agree, thanks for that comments

llvm/test/CodeGen/X86/line-zero-prologue-end.ll

5

This should *not* have a prologue_end on it, I will add a regex check for the end of the line to make sure prologue_end doesn't show up

8

This also should not have a prologue_end on it, I will add a regex check for it too.

26

Sure, I will just change it to line 3

[rastogishubham]

Updated the test files make sure there are check's for where prologue_end's should not be present. Added a better comment to explain the change made to findPrologueEndLoc as suggested

[rastogishubham]

Added a regex check for ensuring no prologue_end is there on no-non-zero-debug-loc-prologue.ll

[rastogishubham]

@probinson Hi Paul, is it possible for you to take another look? I have updated the test files to be more strict with the prologue_ends.

Thanks,

Shubham

[probinson]

Thanks for the ping, I hadn't added this to my to-do list. LGTM

llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp

   // If we're not at line 0, remember this location.
   if (DL.getLine())
     PrevInstLoc = DL;
 }
 
 static DebugLoc findPrologueEndLoc(const MachineFunction *MF) {
   // First known non-DBG_VALUE and non-frame setup location marks
   // the beginning of the function body.
+  DebugLoc lineZeroLoc;
   for (const auto &MBB : *MF)
     for (const auto &MI : MBB)
       if (!MI.isMetaInstruction() && !MI.getFlag(MachineInstr::FrameSetup) &&
-          MI.getDebugLoc())
-        return MI.getDebugLoc();
-  return DebugLoc();
+          MI.getDebugLoc()) {
+        // If the debug location is of a non-zero line number, return. Otherwise
+        // save off line 0 .loc location and return that if no non-zero location
+        // is found
+        if (MI.getDebugLoc().getLine())
+          return MI.getDebugLoc();
+        else
+          lineZeroLoc = MI.getDebugLoc();
+      }
+  return lineZeroLoc;
 }
 
 /// Register a source line with debug info. Returns the  unique label that was
 /// emitted and which provides correspondence to the source line list.
 static void recordSourceLine(AsmPrinter &Asm, unsigned Line, unsigned Col,
                              const MDNode *S, unsigned Flags, unsigned CUID,
                              uint16_t DwarfVersion,
                              ArrayRef<std::unique_ptr<DwarfCompileUnit>> DCUs) {

llvm/test/CodeGen/X86/line-zero-prologue-end.ll

+; RUN: llc -filetype=asm -mtriple=x86_64-apple-macosx12.0.0 -O0 %s -o - | FileCheck %s
+; CHECK: Lfunc_begin0:
+; CHECK-NEXT: .file{{.+}}
+; CHECK-NEXT: .loc 1 2 0
+; CHECK-NEXT: .cfi_startproc
+; CHECK-NEXT: ## %bb.{{[0-9]+}}:
+; CHECK-NEXT: .loc 1 0 5 is_stmt{{.*}}
+@x = common global i32 0, align 4
+define void @test() #0 !dbg !9 {
+  store i32 1, i32* @x, align 4, !dbg !12
+  ret void, !dbg !14
+}
+!llvm.module.flags = !{!0,!2,!4}
+!llvm.dbg.cu = !{!5}
+!0 = !{i32 2, !"SDK Version", [2 x i32] [i32 12, i32 0]}
+!2 = !{i32 2, !"Debug Info Version", i32 3}
+!4 = !{i32 7, !"PIC Level", i32 2}
+!5 = distinct !DICompileUnit(language: DW_LANG_C99, file: !6, producer: "Apple clang version 13.0.0 (clang-1300.0.29.3)", isOptimized: false, runtimeVersion: 0, emissionKind: LineTablesOnly, enums: !7, nameTableKind: None, sysroot: "/Library/Developer/CommandLineTools/SDKs/MacOSX12.0.sdk", sdk: "MacOSX12.0.sdk")
+!6 = !DIFile(filename: "/Users/shubham/Development/test/test.c", directory: "/Users/shubham/Development/deltaTest")
+!7 = !{}
+!9 = distinct !DISubprogram(name: "test", scope: !10, file: !10, line: 2, type: !11, scopeLine: 2, spFlags: DISPFlagDefinition, unit: !5, retainedNodes: !7)
+!10 = !DIFile(filename: "test/test.c", directory: "/Users/shubham/Development")
+!11 = !DISubroutineType(types: !7)
+!12 = !DILocation(line: 0, column: 5, scope: !9)
+!14 = !DILocation(line: 2, column: 1, scope: !9)

llvm/test/CodeGen/X86/no-non-zero-debug-loc-prologue.ll

+; RUN: llc -filetype=asm -mtriple=x86_64-apple-macosx12.0.0 -O0 %s -o - | FileCheck %s
+; CHECK: Lfunc_begin0:
+; CHECK-NEXT: .file{{.+}}
+; CHECK-NEXT: .loc 1 1 0
+; CHECK-NEXT: .cfi_startproc
+; CHECK-NEXT: ## %bb.{{[0-9]+}}:
+; CHECK-NEXT: .loc 1 0 1 prologue_end{{.*}}
+define void @test() #0 !dbg !9 {
+  ret void, !dbg !12
+}
+!llvm.module.flags = !{!0, !2, !4}
+!llvm.dbg.cu = !{!5}
+!0 = !{i32 2, !"SDK Version", [2 x i32] [i32 12, i32 0]}
+!2 = !{i32 2, !"Debug Info Version", i32 3}
+!4 = !{i32 7, !"PIC Level", i32 2}
+!5 = distinct !DICompileUnit(language: DW_LANG_C99, file: !6, producer: "Apple clang version 13.0.0 (clang-1300.0.29.3)", isOptimized: false, runtimeVersion: 0, emissionKind: LineTablesOnly, enums: !7, nameTableKind: None, sysroot: "/Library/Developer/CommandLineTools/SDKs/MacOSX12.0.sdk", sdk: "MacOSX12.0.sdk")
+!6 = !DIFile(filename: "/Users/shubham/Development/test/test-small.c", directory: "/Users/shubham/Development/test")
+!7 = !{}
+!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)

llvm/test/DebugInfo/MIR/X86/debug-loc-0.mir

 # Check that subsequent .loc 0 0 directives are collapsed.
 
 # RUN: llc -start-before=machine-cp -O2 -filetype=asm -mtriple=x86_64-apple-macosx10.9.0 -o - %s | FileCheck %s
 
 # CHECK: Ltmp0:
-# CHECK: .loc 1 0 0 prologue_end
+# CHECK: .loc 1 0 0
 # CHECK-NOT: .loc 1 0 0
-# CHECK: .loc 1 37 1
 
 --- |
   ; ModuleID = '<stdin>'
   source_filename = "<stdin>"
   target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
   target triple = "x86_64-apple-macosx10.9.0"
 
   %swift.opaque = type opaque
   %swift.metadata_response = type { %swift.type*, i64 }

llvm/test/DebugInfo/X86/dbg-prolog-end.ll

entry:
   store i32 %add, i32* %j, align 4, !dbg !14
   %tmp3 = load i32, i32* %j, align 4, !dbg !15
   ret i32 %tmp3, !dbg !15
 }
 
 declare void @llvm.dbg.declare(metadata, metadata, metadata) nounwind readnone
 
 ;CHECK-LABEL: main:
-;CHECK: .loc 1 0 0 prologue_end
+;CHECK: .loc 1 8 2 prologue_end
 
 define i32 @main() nounwind ssp !dbg !6 {
 entry:
   %retval = alloca i32, align 4
   store i32 0, i32* %retval, !dbg !22
   %call = call i32 @foo(i32 21), !dbg !16
   ret i32 %call, !dbg !16
 }