Skip to content

Commit

Permalink
[DebugInfo] Track multiple registers in DbgEntityHistoryCalculator
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@dstenb
dstenb committed Apr 10, 2019
1 parent 5ffec6d commit b96943b
Show file tree
Hide file tree
Showing 4 changed files with 144 additions and 33 deletions.
4 changes: 0 additions & 4 deletions llvm/include/llvm/CodeGen/DbgEntityHistoryCalculator.h
View file
Expand Up @@ -83,10 +83,6 @@ class DbgValueHistoryMap {
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];
Expand Down
69 changes: 41 additions & 28 deletions llvm/lib/CodeGen/AsmPrinter/DbgEntityHistoryCalculator.cpp
View file
Expand Up @@ -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;
}
Expand All @@ -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;
Expand Down Expand Up @@ -135,33 +127,39 @@ 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.
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;
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion llvm/test/DebugInfo/COFF/pieces.ll
View file
Expand Up @@ -104,7 +104,7 @@
; 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"


Expand Down
102 changes: 102 additions & 0 deletions llvm/test/DebugInfo/MIR/X86/clobbered-fragments.mir
View file
Expand Up @@ -14,6 +14,12 @@
# 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.

--- |
Expand All @@ -37,6 +43,21 @@

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

Expand Down Expand Up @@ -64,6 +85,10 @@
!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)

...
---
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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

0 comments on commit b96943b

Please sign in to comment.