[BranchFolding] Drop location when hoisting

[davide]

Summary

Fixes https://bugs.llvm.org/show_bug.cgi?id=46009
See the link for a more detailed analysis

[davide]

llvm/test/DebugInfo/MIR/X86/branch-folder-hoisting-location.mir

1–6

Probably this test can be trimmed, but I wanted to put this online to get a general feeling of the idea.

[davide]

Also, looks like this one exposed a bug in LiveDbgValues. I'll look at it while waiting for the review:

******************** TEST 'LLVM :: CodeGen/X86/dbg-changes-codegen-branch-folding.ll' FAILED ********************
Script:
--
: 'RUN: at line 1';   /Users/davide/work/build/bin/llc -mtriple=x86_64-linux < /Users/davide/work/llvm-project/llvm/test/CodeGen/X86/dbg-changes-codegen-branch-folding.ll | /Users/davide/work/build/bin/FileCheck /Users/davide/work/llvm-project/llvm/test/CodeGen/X86/dbg-changes-codegen-branch-folding.ll
: 'RUN: at line 2';   /Users/davide/work/build/bin/opt -strip-debug < /Users/davide/work/llvm-project/llvm/test/CodeGen/X86/dbg-changes-codegen-branch-folding.ll | /Users/davide/work/build/bin/llc -mtriple=x86_64-linux | /Users/davide/work/build/bin/FileCheck /Users/davide/work/llvm-project/llvm/test/CodeGen/X86/dbg-changes-codegen-branch-folding.ll
--
Exit Code: 2

Command Output (stderr):
--
PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash backtrace.
Stack dump:
0.	Program arguments: /Users/davide/work/build/bin/llc -mtriple=x86_64-linux 
1.	Running pass 'Function Pass Manager' on module '<stdin>'.
2.	Running pass 'Live DEBUG_VALUE analysis' on function '@_Z3barii'
0  llc                      0x00000001031bc825 llvm::sys::PrintStackTrace(llvm::raw_ostream&) + 37
1  llc                      0x00000001031bb6a8 llvm::sys::RunSignalHandlers() + 248
2  llc                      0x00000001031bce26 SignalHandler(int) + 262
3  libsystem_platform.dylib 0x00007fff7223355d _sigtramp + 29
4  libsystem_platform.dylib 000000000000000000 _sigtramp + 18446603338601253568
5  llc                      0x000000010285958d llvm::MachineFunctionPass::runOnFunction(llvm::Function&) + 285
6  llc                      0x0000000102b82658 llvm::FPPassManager::runOnFunction(llvm::Function&) + 1064
7  llc                      0x0000000102b82933 llvm::FPPassManager::runOnModule(llvm::Module&) + 67
8  llc                      0x0000000102b82dd9 llvm::legacy::PassManagerImpl::run(llvm::Module&) + 937
9  llc                      0x0000000101627b3f compileModule(char**, llvm::LLVMContext&) + 9647
10 llc                      0x000000010162508c main + 1308
11 libdyld.dylib            0x00007fff72026259 start + 1
12 libdyld.dylib            0x0000000000000002 start + 18446603338603404714
FileCheck error: '<stdin>' is empty.
FileCheck command line:  /Users/davide/work/build/bin/FileCheck /Users/davide/work/llvm-project/llvm/test/CodeGen/X86/dbg-changes-codegen-branch-folding.ll

[davide]

Yes, I captured the MIR before my patch and realized LiveDebugValues crashes without it. It's just the MIR presented to the pass that causes the problem. I'm going to file a separate PR for it.

[davide]

https://bugs.llvm.org/show_bug.cgi?id=46086

[vsk]

llvm/lib/CodeGen/BranchFolding.cpp

2028

To summarize some offline discussions:

• We haven't yet nailed down what should happen to hoisted debug insts, we should discuss (maybe in a follow-up to D80052)
• D80670 flags the issue here; if we're going to hoist DBG_VALUEs here, they need a location

[vsk]

llvm/lib/CodeGen/BranchFolding.cpp

2028

I've drafted some rules for updating debug locs here: https://reviews.llvm.org/D81198. We haven't reached consensus around those rules, but my $0.02 is that HoistCommonCodeInSuccs should: a) clear the debug locs on the instructions it hoists if it found identical instructions in multiple successor blocks, and b) /not/ hoist any DBG_VALUEs. I haven't written up rules for dealing with DBG_VALUEs yet, but I think the guiding principle is going to be 'touch dbg.value/DBG_VALUE as little as possible' to avoid having assignments show up too early.

[davide]

Back to this. I think what Vedant suggest is the right approach and I'll try implementing it now.

[davide]

I won't have time to look at this in the future.

llvm/lib/CodeGen/BranchFolding.cpp

   if (!HasDups)
     return false;
 
+  // As we're hoisting instructions from a successor to a predecessor,
+  // we drop the locations to avoid jumpy stepping.
+  for (auto &TBBI : *TBB)
+    TBBI.setDebugLoc(DebugLoc());
+
   MBB->splice(Loc, TBB, TBB->begin(), TIB);
   FBB->erase(FBB->begin(), FIB);
 

llvm/test/DebugInfo/MIR/X86/branch-folder-hoisting-location.mir

+--- |
+  ; RUN: llc --run-pass=branch-folder %s -o - | FileCheck %s --match-full-lines
+
+  ; Make sure that the MOV32ri we're hoisting doesn't retain any debug location.
+  ; CHECK: DBG_VALUE $ecx, $noreg, !23, !DIExpression(), debug-location !24
+  ; CHECK-NEXT: $eax = MOV32ri 5
+  ; CHECK-NEXT: TEST32rr renamable $ecx, renamable $ecx, implicit-def $eflags, debug-location !30
+  target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+  target triple = "x86_64-apple-macosx10.16.0"
+  
+  @b = local_unnamed_addr global i32 0, align 4, !dbg !0
+  @c = local_unnamed_addr global i32 0, align 4, !dbg !10
+  @a = local_unnamed_addr global i32 0, align 4, !dbg !6
+  
+  ; Function Attrs: nofree norecurse nounwind ssp uwtable
+  define i32 @main() local_unnamed_addr #0 !dbg !17 {
+    call void @llvm.dbg.value(metadata i32 5, metadata !21, metadata !DIExpression()), !dbg !24
+    %1 = load i32, i32* @b, align 4, !dbg !25, !tbaa !26
+    call void @llvm.dbg.value(metadata i32 %1, metadata !23, metadata !DIExpression()), !dbg !24
+    %2 = icmp eq i32 %1, 0, !dbg !30
+    br i1 %2, label %5, label %3, !dbg !31
+  
+  3:                                                ; preds = %0
+    %4 = sdiv i32 5, %1, !dbg !32
+    br label %5, !dbg !31
+  
+  5:                                                ; preds = %3, %0
+    %6 = phi i32 [ %4, %3 ], [ 5, %0 ], !dbg !31
+    store i32 %6, i32* @c, align 4, !dbg !33, !tbaa !26
+    ret i32 0, !dbg !34
+  }
+  
+  ; Function Attrs: nounwind readnone speculatable willreturn
+  declare void @llvm.dbg.value(metadata, metadata, metadata) #1
+  
+  ; Function Attrs: nounwind
+  declare void @llvm.stackprotector(i8*, i8**) #2
+  
+  attributes #0 = { nofree norecurse nounwind ssp uwtable "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "frame-pointer"="all" "less-precise-fpmad"="false" "min-legal-vector-width"="0" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "target-cpu"="penryn" "target-features"="+cx16,+cx8,+fxsr,+mmx,+sahf,+sse,+sse2,+sse3,+sse4.1,+ssse3,+x87" "unsafe-fp-math"="false" "use-soft-float"="false" }
+  attributes #1 = { nounwind readnone speculatable willreturn }
+  attributes #2 = { nounwind }
+  
+  !llvm.dbg.cu = !{!2}
+  !llvm.module.flags = !{!12, !13, !14, !15}
+  !llvm.ident = !{!16}
+  
+  !0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression())
+  !1 = distinct !DIGlobalVariable(name: "b", scope: !2, file: !8, line: 1, type: !9, isLocal: false, isDefinition: true)
+  !2 = distinct !DICompileUnit(language: DW_LANG_C99, file: !3, producer: "clang version 11.0.0 (https://github.com/llvm/llvm-project 9abb0e8d5be2ffad06ccfcc2d5530997ad093b81)", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !4, globals: !5, nameTableKind: None, sysroot: "/")
+  !3 = !DIFile(filename: "/tmp/a.c", directory: "/Users/davide/work/build/bin")
+  !4 = !{}
+  !5 = !{!6, !0, !10}
+  !6 = !DIGlobalVariableExpression(var: !7, expr: !DIExpression())
+  !7 = distinct !DIGlobalVariable(name: "a", scope: !2, file: !8, line: 1, type: !9, isLocal: false, isDefinition: true)
+  !8 = !DIFile(filename: "/tmp/a.c", directory: "")
+  !9 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+  !10 = !DIGlobalVariableExpression(var: !11, expr: !DIExpression())
+  !11 = distinct !DIGlobalVariable(name: "c", scope: !2, file: !8, line: 1, type: !9, isLocal: false, isDefinition: true)
+  !12 = !{i32 7, !"Dwarf Version", i32 4}
+  !13 = !{i32 2, !"Debug Info Version", i32 3}
+  !14 = !{i32 1, !"wchar_size", i32 4}
+  !15 = !{i32 7, !"PIC Level", i32 2}
+  !16 = !{!"clang version 11.0.0 (https://github.com/llvm/llvm-project 9abb0e8d5be2ffad06ccfcc2d5530997ad093b81)"}
+  !17 = distinct !DISubprogram(name: "main", scope: !8, file: !8, line: 2, type: !18, scopeLine: 3, flags: DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !2, retainedNodes: !20)
+  !18 = !DISubroutineType(types: !19)
+  !19 = !{!9}
+  !20 = !{!21, !23}
+  !21 = !DILocalVariable(name: "ui1", scope: !22, file: !8, line: 4, type: !9)
+  !22 = distinct !DILexicalBlock(scope: !17, file: !8, line: 4, column: 5)
+  !23 = !DILocalVariable(name: "ui2", scope: !22, file: !8, line: 4, type: !9)
+  !24 = !DILocation(line: 0, scope: !22)
+  !25 = !DILocation(line: 4, column: 26, scope: !22)
+  !26 = !{!27, !27, i64 0}
+  !27 = !{!"int", !28, i64 0}
+  !28 = !{!"omnipotent char", !29, i64 0}
+  !29 = !{!"Simple C/C++ TBAA"}
+  !30 = !DILocation(line: 6, column: 15, scope: !22)
+  !31 = !DILocation(line: 6, column: 11, scope: !22)
+  !32 = !DILocation(line: 8, column: 16, scope: !22)
+  !33 = !DILocation(line: 5, column: 10, scope: !22)
+  !34 = !DILocation(line: 10, column: 1, scope: !17)
+
+...
+---
+name:            main
+alignment:       16
+exposesReturnsTwice: false
+legalized:       false
+regBankSelected: false
+selected:        false
+failedISel:      false
+tracksRegLiveness: true
+hasWinCFI:       false
+registers:       []
+liveins:         []
+frameInfo:
+  isFrameAddressTaken: false
+  isReturnAddressTaken: false
+  hasStackMap:     false
+  hasPatchPoint:   false
+  stackSize:       8
+  offsetAdjustment: -8
+  maxAlignment:    1
+  adjustsStack:    false
+  hasCalls:        false
+  stackProtector:  ''
+  maxCallFrameSize: 0
+  cvBytesOfCalleeSavedRegisters: 0
+  hasOpaqueSPAdjustment: false
+  hasVAStart:      false
+  hasMustTailInVarArgFunc: false
+  localFrameSize:  0
+  savePoint:       ''
+  restorePoint:    ''
+fixedStack:
+  - { id: 0, type: spill-slot, offset: -16, size: 8, alignment: 16, stack-id: default, 
+      callee-saved-register: '', callee-saved-restored: true, debug-info-variable: '', 
+      debug-info-expression: '', debug-info-location: '' }
+stack:           []
+callSites:       []
+constants:       []
+machineFunctionInfo: {}
+body:             |
+  bb.0 (%ir-block.0):
+    successors: %bb.1(0x30000000), %bb.2(0x50000000)
+  
+    frame-setup PUSH64r killed $rbp, implicit-def $rsp, implicit $rsp
+    CFI_INSTRUCTION def_cfa_offset 16
+    CFI_INSTRUCTION offset $rbp, -16
+    $rbp = frame-setup MOV64rr $rsp
+    CFI_INSTRUCTION def_cfa_register $rbp
+    DBG_VALUE 5, $noreg, !21, !DIExpression(), debug-location !24
+    renamable $ecx = MOV32rm $rip, 1, $noreg, @b, $noreg, debug-location !25 :: (dereferenceable load 4 from @b, !tbaa !26)
+    DBG_VALUE $ecx, $noreg, !23, !DIExpression(), debug-location !24
+    TEST32rr renamable $ecx, renamable $ecx, implicit-def $eflags, debug-location !30
+    JCC_1 %bb.2, 5, implicit killed $eflags, debug-location !31
+  
+  bb.1:
+    successors: %bb.3(0x80000000)
+  
+    renamable $eax = MOV32ri 5
+    JMP_1 %bb.3
+  
+  bb.2 (%ir-block.3):
+    successors: %bb.3(0x80000000)
+    liveins: $ecx
+  
+    $eax = MOV32ri 5, debug-location !32
+    $edx = MOV32r0 implicit-def dead $eflags, debug-location !32
+    IDIV32r killed renamable $ecx, implicit-def $eax, implicit-def dead $edx, implicit-def dead $eflags, implicit $eax, implicit killed $edx, debug-location !32
+  
+  bb.3 (%ir-block.5):
+    liveins: $eax
+  
+    MOV32mr $rip, 1, $noreg, @c, $noreg, killed renamable $eax, debug-location !33 :: (store 4 into @c, !tbaa !26)
+    $eax = MOV32r0 implicit-def dead $eflags, debug-location !34
+    $rbp = frame-destroy POP64r implicit-def $rsp, implicit $rsp, debug-location !34
+    RET 0, $eax, debug-location !34
+
+...