[DebugInfo] Track multiple registers in DbgEntityHistoryCalculator
authorDavid Stenberg <david.stenberg@ericsson.com>
Wed, 10 Apr 2019 11:28:28 +0000 (11:28 +0000)
committerDavid Stenberg <david.stenberg@ericsson.com>
Wed, 10 Apr 2019 11:28:28 +0000 (11:28 +0000)
Summary:
When calculating the debug value history, DbgEntityHistoryCalculator
would only keep track of register clobbering for the latest debug value
per inlined entity. This meant that preceding register-described debug
value fragments would live on until the next overlapping debug value,
ignoring any potential clobbering. This patch amends
DbgEntityHistoryCalculator so that it keeps track of all registers that
a inlined entity's currently live debug values are described by.

The DebugInfo/COFF/pieces.ll test case has had to be changed since
previously a register-described fragment would incorrectly outlive its
basic block.

The parent patch D59941 is expected to increase the coverage slightly,
as it makes sure that location list entries are inserted after clobbered
fragments, and this patch is expected to decrease it, as it stops
preceding register-described from living longer than they should. All in
all, this patch and the preceding patch has a negligible effect on the
output from `llvm-dwarfdump -statistics' for a clang-3.4 binary built
using the RelWithDebInfo build profile. "Scope bytes covered" increases
by 0.5%, and "variables with location" increases from 2212083 to
2212088, but it should improve the accuracy quite a bit.

This fixes PR40283.

Reviewers: aprantl, probinson, dblaikie, rnk, bjope

Reviewed By: aprantl

Subscribers: llvm-commits

Tags: #debug-info, #llvm

Differential Revision: https://reviews.llvm.org/D59942

llvm-svn: 358073

llvm/include/llvm/CodeGen/DbgEntityHistoryCalculator.h
llvm/lib/CodeGen/AsmPrinter/DbgEntityHistoryCalculator.cpp
llvm/test/DebugInfo/COFF/pieces.ll
llvm/test/DebugInfo/MIR/X86/clobbered-fragments.mir

index 717abc5..7eec75b 100644 (file)
@@ -83,10 +83,6 @@ public:
                      EntryIndex &NewIndex);
   EntryIndex startClobber(InlinedEntity Var, const MachineInstr &MI);
 
-  // Returns register currently describing @Var. If @Var is currently
-  // unaccessible or is not described by a register, returns 0.
-  unsigned getRegisterForVar(InlinedEntity Var) const;
-
   Entry &getEntry(InlinedEntity Var, EntryIndex Index) {
     auto &Entries = VarEntries[Var];
     return Entries[Index];
index 5dec591..c006f3c 100644 (file)
@@ -69,6 +69,10 @@ bool DbgValueHistoryMap::startDbgValue(InlinedEntity Var,
 EntryIndex DbgValueHistoryMap::startClobber(InlinedEntity Var,
                                             const MachineInstr &MI) {
   auto &Entries = VarEntries[Var];
+  // If an instruction clobbers multiple registers that the variable is
+  // described by, then we may have already created a clobbering instruction.
+  if (Entries.back().isClobber() && Entries.back().getInstr() == &MI)
+    return Entries.size() - 1;
   Entries.emplace_back(&MI, Entry::Clobber);
   return Entries.size() - 1;
 }
@@ -81,18 +85,6 @@ void DbgValueHistoryMap::Entry::endEntry(EntryIndex Index) {
   EndIndex = Index;
 }
 
-unsigned DbgValueHistoryMap::getRegisterForVar(InlinedEntity Var) const {
-  const auto &I = VarEntries.find(Var);
-  if (I == VarEntries.end())
-    return 0;
-  const auto &Entries = I->second;
-  if (Entries.empty() || Entries.back().isClosed())
-    return 0;
-  if (Entries.back().isClobber())
-    return 0;
-  return isDescribedByReg(*Entries.back().getInstr());
-}
-
 void DbgLabelInstrMap::addInstr(InlinedEntity Label, const MachineInstr &MI) {
   assert(MI.isDebugLabel() && "not a DBG_LABEL");
   LabelInstr[Label] = &MI;
@@ -135,18 +127,28 @@ static void addRegDescribedVar(RegDescribedVarsMap &RegVars, unsigned RegNo,
   VarSet.push_back(Var);
 }
 
+/// Create a clobbering entry and end all open debug value entries
+/// for \p Var that are described by \p RegNo using that entry.
 static void clobberRegEntries(InlinedEntity Var, unsigned RegNo,
                               const MachineInstr &ClobberingInstr,
                               DbgValueEntriesMap &LiveEntries,
                               DbgValueHistoryMap &HistMap) {
   EntryIndex ClobberIndex = HistMap.startClobber(Var, ClobberingInstr);
 
-  // TODO: Close all preceding live entries that are clobbered by this
-  // instruction.
-  EntryIndex ValueIndex = ClobberIndex - 1;
-  auto &ValueEntry = HistMap.getEntry(Var, ValueIndex);
-  ValueEntry.endEntry(ClobberIndex);
-  LiveEntries[Var].erase(ValueIndex);
+  // Close all entries whose values are described by the register.
+  SmallVector<EntryIndex, 4> IndicesToErase;
+  for (auto Index : LiveEntries[Var]) {
+    auto &Entry = HistMap.getEntry(Var, Index);
+    assert(Entry.isDbgValue() && "Not a DBG_VALUE in LiveEntries");
+    if (isDescribedByReg(*Entry.getInstr()) == RegNo) {
+      IndicesToErase.push_back(Index);
+      Entry.endEntry(ClobberIndex);
+    }
+  }
+
+  // Drop all entries that have ended.
+  for (auto Index : IndicesToErase)
+    LiveEntries[Var].erase(Index);
 }
 
 /// Add a new debug value for \p Var. Closes all overlapping debug values.
@@ -154,14 +156,10 @@ static void handleNewDebugValue(InlinedEntity Var, const MachineInstr &DV,
                                 RegDescribedVarsMap &RegVars,
                                 DbgValueEntriesMap &LiveEntries,
                                 DbgValueHistoryMap &HistMap) {
-  // TODO: We should track all registers which this variable is currently
-  // described by.
-
-  if (unsigned PrevReg = HistMap.getRegisterForVar(Var))
-    dropRegDescribedVar(RegVars, PrevReg, Var);
-
   EntryIndex NewIndex;
   if (HistMap.startDbgValue(Var, DV, NewIndex)) {
+    SmallDenseMap<unsigned, bool, 4> TrackedRegs;
+
     // If we have created a new debug value entry, close all preceding
     // live entries that overlap.
     SmallVector<EntryIndex, 4> IndicesToErase;
@@ -170,19 +168,34 @@ static void handleNewDebugValue(InlinedEntity Var, const MachineInstr &DV,
       auto &Entry = HistMap.getEntry(Var, Index);
       assert(Entry.isDbgValue() && "Not a DBG_VALUE in LiveEntries");
       const MachineInstr &DV = *Entry.getInstr();
-      if (DIExpr->fragmentsOverlap(DV.getDebugExpression())) {
+      bool Overlaps = DIExpr->fragmentsOverlap(DV.getDebugExpression());
+      if (Overlaps) {
         IndicesToErase.push_back(Index);
         Entry.endEntry(NewIndex);
       }
+      if (unsigned Reg = isDescribedByReg(DV))
+        TrackedRegs[Reg] |= !Overlaps;
+    }
+
+    // If the new debug value is described by a register, add tracking of
+    // that register if it is not already tracked.
+    if (unsigned NewReg = isDescribedByReg(DV)) {
+      if (!TrackedRegs.count(NewReg))
+        addRegDescribedVar(RegVars, NewReg, Var);
+      LiveEntries[Var].insert(NewIndex);
+      TrackedRegs[NewReg] = true;
     }
+
+    // Drop tracking of registers that are no longer used.
+    for (auto I : TrackedRegs)
+      if (!I.second)
+        dropRegDescribedVar(RegVars, I.first, Var);
+
     // Drop all entries that have ended, and mark the new entry as live.
     for (auto Index : IndicesToErase)
       LiveEntries[Var].erase(Index);
     LiveEntries[Var].insert(NewIndex);
   }
-
-  if (unsigned NewReg = isDescribedByReg(DV))
-    addRegDescribedVar(RegVars, NewReg, Var);
 }
 
 // Terminate the location range for variables described by register at
index e04255d..b218f07 100644 (file)
 ; ASM:        .asciz  "o"
 ; ASM:        .cv_def_range    [[oy_ox_start]] [[ox_start]], "C\021\030\000\000\000\000\000\000\000"
 ; ASM:        .cv_def_range    [[oy_ox_start]] [[oy_start]], "C\021\027\000\000\000\004\000\000\000"
-; ASM:        .cv_def_range    [[ox_start]] .Lfunc_end0, "C\021\030\000\000\000\000\000\000\000"
+; ASM:        .cv_def_range    [[ox_start]] [[oy_end]], "C\021\030\000\000\000\000\000\000\000"
 ; ASM:        .cv_def_range    [[oy_start]] [[oy_end]], "C\021\027\000\000\000\004\000\000\000"
 
 
index e43463e..3d4decd 100644 (file)
 #   return local[1];
 # }
 #
+# int test2() {
+#   int local[3] = {global1, global2, global3};
+#   ext3(local[0], local[1], local[2]);
+#   return local[0];
+# }
+#
 # Compiled using -O1 -g.
 
 --- |
 
   declare void @ext2(i32, i32)
 
+  ; Function Attrs: nounwind uwtable
+  define i32 @test2() #0 !dbg !18 {
+  entry:
+    %0 = load i32, i32* @global1, align 4, !dbg !20
+    call void @llvm.dbg.value(metadata i32 %0, metadata !19, metadata !DIExpression(DW_OP_LLVM_fragment, 0, 32)), !dbg !20
+    %1 = load i32, i32* @global2, align 4, !dbg !20
+    call void @llvm.dbg.value(metadata i32 %1, metadata !19, metadata !DIExpression(DW_OP_LLVM_fragment, 32, 32)), !dbg !20
+    %2 = load i32, i32* @global3, align 4, !dbg !20
+    call void @llvm.dbg.value(metadata i32 %2, metadata !19, metadata !DIExpression(DW_OP_LLVM_fragment, 64, 32)), !dbg !20
+    tail call void @ext3(i32 %0, i32 %1, i32 %2) #3, !dbg !20
+    ret i32 %0, !dbg !21
+  }
+
+  declare void @ext3(i32, i32, i32)
+
   ; Function Attrs: nounwind readnone speculatable
   declare void @llvm.dbg.value(metadata, metadata, metadata) #1
 
   !15 = !DISubrange(count: 3)
   !16 = !DILocation(line: 8, scope: !8)
   !17 = !DILocation(line: 9, scope: !8)
+  !18 = distinct !DISubprogram(name: "test2", scope: !2, file: !2, line: 7, type: !9, scopeLine: 7, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !1, retainedNodes: !11)
+  !19 = !DILocalVariable(name: "local", scope: !18, file: !2, line: 8, type: !13)
+  !20 = !DILocation(line: 15, scope: !18)
+  !21 = !DILocation(line: 16, scope: !18)
 
 ...
 ---
@@ -91,6 +116,40 @@ body:             |
 # CHECK:         callq   ext2
 # CHECK-NEXT: .Ltmp2:
 
+...
+---
+name:            test2
+tracksRegLiveness: true
+body:             |
+  bb.0.entry:
+    liveins: $rbx
+
+    frame-setup PUSH64r killed $rbx, implicit-def $rsp, implicit $rsp, debug-location !20
+    renamable $ebx = MOV32rm $rip, 1, $noreg, @global1, $noreg, debug-location !20 :: (dereferenceable load 4 from @global1)
+    DBG_VALUE $ebx, $noreg, !19, !DIExpression(DW_OP_LLVM_fragment, 0, 32), debug-location !20
+    renamable $esi = MOV32rm $rip, 1, $noreg, @global2, $noreg, debug-location !20 :: (dereferenceable load 4 from @global2)
+    DBG_VALUE $esi, $noreg, !19, !DIExpression(DW_OP_LLVM_fragment, 32, 32), debug-location !20
+    renamable $edx = MOV32rm $rip, 1, $noreg, @global3, $noreg, debug-location !20 :: (dereferenceable load 4 from @global3)
+    DBG_VALUE $edx, $noreg, !19, !DIExpression(DW_OP_LLVM_fragment, 64, 32), debug-location !20
+    $edi = MOV32rr $ebx, debug-location !20
+    CALL64pcrel32 @ext3, csr_64, implicit $rsp, implicit $ssp, implicit killed $edi, implicit $esi, implicit $edx, implicit-def $rsp, implicit-def $ssp, debug-location !20
+    $eax = MOV32rr killed $ebx, debug-location !20
+    $rbx = frame-destroy POP64r implicit-def $rsp, implicit $rsp, debug-location !20
+    RETQ killed $eax, debug-location !21
+
+...
+
+# CHECK: .Ltmp4:
+# CHECK-NEXT: #DEBUG_VALUE: test2:local <- [DW_OP_LLVM_fragment 0 32] $ebx
+# CHECK: .Ltmp5:
+# CHECK-NEXT: #DEBUG_VALUE: test2:local <- [DW_OP_LLVM_fragment 32 32] $esi
+# CHECK: .Ltmp6:
+# CHECK-NEXT: #DEBUG_VALUE: test2:local <- [DW_OP_LLVM_fragment 64 32] $edx
+# CHECK:         callq   ext3
+# CHECK-NEXT: .Ltmp7:
+# CHECK:         popq    %rbx
+# CHECK-NEXT: .Ltmp8:
+
 #### Location list for test1:local.
 
 # Verify that a location list entry, which does not contain the fragment that
@@ -134,3 +193,46 @@ body:             |
 # CHECK-NEXT: .quad   0
 # CHECK-NEXT: .quad   0
 
+#### Location list for test2:local.
+
+# Verify that the debug values that are described by $edi and $edx are
+# considered clobbered by the call to ext3(), leaving a location list entry
+# after the call where the first 32 bits are still described by $ebx.
+# That location list entry is valid until the restore of the register.
+
+# CHECK: .Ldebug_loc1:
+# CHECK-NEXT: .quad   .Ltmp4-.Lfunc_begin0
+# CHECK-NEXT: .quad   .Ltmp5-.Lfunc_begin0
+# CHECK-NEXT: .short  3                       # Loc expr size
+# CHECK-NEXT: .byte   83                      # super-register DW_OP_reg3
+# CHECK-NEXT: .byte   147                     # DW_OP_piece
+# CHECK-NEXT: .byte   4                       # 4
+# CHECK-NEXT: .quad   .Ltmp5-.Lfunc_begin0
+# CHECK-NEXT: .quad   .Ltmp6-.Lfunc_begin0
+# CHECK-NEXT: .short  6                       # Loc expr size
+# CHECK-NEXT: .byte   83                      # super-register DW_OP_reg3
+# CHECK-NEXT: .byte   147                     # DW_OP_piece
+# CHECK-NEXT: .byte   4                       # 4
+# CHECK-NEXT: .byte   84                      # super-register DW_OP_reg4
+# CHECK-NEXT: .byte   147                     # DW_OP_piece
+# CHECK-NEXT: .byte   4                       # 4
+# CHECK-NEXT: .quad   .Ltmp6-.Lfunc_begin0
+# CHECK-NEXT: .quad   .Ltmp7-.Lfunc_begin0
+# CHECK-NEXT: .short  9                       # Loc expr size
+# CHECK-NEXT: .byte   83                      # super-register DW_OP_reg3
+# CHECK-NEXT: .byte   147                     # DW_OP_piece
+# CHECK-NEXT: .byte   4                       # 4
+# CHECK-NEXT: .byte   84                      # super-register DW_OP_reg4
+# CHECK-NEXT: .byte   147                     # DW_OP_piece
+# CHECK-NEXT: .byte   4                       # 4
+# CHECK-NEXT: .byte   81                      # super-register DW_OP_reg1
+# CHECK-NEXT: .byte   147                     # DW_OP_piece
+# CHECK-NEXT: .byte   4                       # 4
+# CHECK-NEXT: .quad   .Ltmp7-.Lfunc_begin0
+# CHECK-NEXT: .quad   .Ltmp8-.Lfunc_begin0
+# CHECK-NEXT: .short  3                       # Loc expr size
+# CHECK-NEXT: .byte   83                      # super-register DW_OP_reg3
+# CHECK-NEXT: .byte   147                     # DW_OP_piece
+# CHECK-NEXT: .byte   4                       # 4
+# CHECK-NEXT: .quad   0
+# CHECK-NEXT: .quad   0