From: David Stenberg Date: Wed, 10 Apr 2019 11:28:28 +0000 (+0000) Subject: [DebugInfo] Track multiple registers in DbgEntityHistoryCalculator X-Git-Tag: llvmorg-10-init~8106 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=b96943b6a00d76e4bcf5b6d459d346cd2fb9e296;p=platform%2Fupstream%2Fllvm.git [DebugInfo] Track multiple registers in DbgEntityHistoryCalculator 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 --- diff --git a/llvm/include/llvm/CodeGen/DbgEntityHistoryCalculator.h b/llvm/include/llvm/CodeGen/DbgEntityHistoryCalculator.h index 717abc5..7eec75b 100644 --- a/llvm/include/llvm/CodeGen/DbgEntityHistoryCalculator.h +++ b/llvm/include/llvm/CodeGen/DbgEntityHistoryCalculator.h @@ -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]; diff --git a/llvm/lib/CodeGen/AsmPrinter/DbgEntityHistoryCalculator.cpp b/llvm/lib/CodeGen/AsmPrinter/DbgEntityHistoryCalculator.cpp index 5dec591..c006f3c 100644 --- a/llvm/lib/CodeGen/AsmPrinter/DbgEntityHistoryCalculator.cpp +++ b/llvm/lib/CodeGen/AsmPrinter/DbgEntityHistoryCalculator.cpp @@ -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 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 TrackedRegs; + // If we have created a new debug value entry, close all preceding // live entries that overlap. SmallVector 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 diff --git a/llvm/test/DebugInfo/COFF/pieces.ll b/llvm/test/DebugInfo/COFF/pieces.ll index e04255d..b218f07 100644 --- a/llvm/test/DebugInfo/COFF/pieces.ll +++ b/llvm/test/DebugInfo/COFF/pieces.ll @@ -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" diff --git a/llvm/test/DebugInfo/MIR/X86/clobbered-fragments.mir b/llvm/test/DebugInfo/MIR/X86/clobbered-fragments.mir index e43463e..3d4decd 100644 --- a/llvm/test/DebugInfo/MIR/X86/clobbered-fragments.mir +++ b/llvm/test/DebugInfo/MIR/X86/clobbered-fragments.mir @@ -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. --- | @@ -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 @@ -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) ... --- @@ -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