[llvm] 0b5a805 - [DwarfDebug] Improve single location detection in validThroughout (2/4)
via llvm-commits
llvm-commits at lists.llvm.org
Thu Aug 27 04:13:58 PDT 2020
Author: OCHyams
Date: 2020-08-27T11:52:29+01:00
New Revision: 0b5a8050ea39355a3876cc6bba9383d91e224e1f
URL: https://github.com/llvm/llvm-project/commit/0b5a8050ea39355a3876cc6bba9383d91e224e1f
DIFF: https://github.com/llvm/llvm-project/commit/0b5a8050ea39355a3876cc6bba9383d91e224e1f.diff
LOG: [DwarfDebug] Improve single location detection in validThroughout (2/4)
With this patch we're now accounting for two more cases which should be
considered 'valid throughout': First, where RangeEnd is ScopeEnd. Second, where
RangeEnd comes before ScopeEnd when including meta instructions, but are both
preceded by the same non-meta instruction.
CTMark shows a geomean binary size reduction of 1.5% for RelWithDebInfo builds.
`llvm-locstats` (using D85636) shows a very small variable location coverage
change in 2 of 10 binaries, but it is in the order of 10s of bytes which lines
up with my expectations.
I've added a test which checks both of these new cases. The first check in the
test isn't strictly necessary for this patch. But I'm not sure that it is
explicitly tested anywhere else, and is useful for the final patch in the
series.
Reviewed By: aprantl
Differential Revision: https://reviews.llvm.org/D86151
Added:
llvm/test/DebugInfo/X86/single-location-2.mir
Modified:
llvm/include/llvm/CodeGen/DbgEntityHistoryCalculator.h
llvm/include/llvm/CodeGen/DebugHandlerBase.h
llvm/lib/CodeGen/AsmPrinter/DbgEntityHistoryCalculator.cpp
llvm/lib/CodeGen/AsmPrinter/DebugHandlerBase.cpp
llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
llvm/test/DebugInfo/MIR/AArch64/implicit-def-dead-scope.mir
llvm/test/DebugInfo/MIR/X86/callsite-stack-value.mir
llvm/test/DebugInfo/X86/trim-var-locs.mir
Removed:
################################################################################
diff --git a/llvm/include/llvm/CodeGen/DbgEntityHistoryCalculator.h b/llvm/include/llvm/CodeGen/DbgEntityHistoryCalculator.h
index 5a1085ea3a37..bca6065b1643 100644
--- a/llvm/include/llvm/CodeGen/DbgEntityHistoryCalculator.h
+++ b/llvm/include/llvm/CodeGen/DbgEntityHistoryCalculator.h
@@ -24,6 +24,24 @@ class MachineFunction;
class MachineInstr;
class TargetRegisterInfo;
+/// Record instruction ordering so we can query their relative positions within
+/// a function. Meta instructions are given the same ordinal as the preceding
+/// non-meta instruction. Class state is invalid if MF is modified after
+/// calling initialize.
+class InstructionOrdering {
+public:
+ void initialize(const MachineFunction &MF);
+ void clear() { InstNumberMap.clear(); }
+
+ /// Check if instruction \p A comes before \p B, where \p A and \p B both
+ /// belong to the MachineFunction passed to initialize().
+ bool isBefore(const MachineInstr *A, const MachineInstr *B) const;
+
+private:
+ /// Each instruction is assigned an order number.
+ DenseMap<const MachineInstr *, unsigned> InstNumberMap;
+};
+
/// For each user variable, keep a list of instruction ranges where this
/// variable is accessible. The variables are listed in order of appearance.
class DbgValueHistoryMap {
@@ -93,7 +111,8 @@ class DbgValueHistoryMap {
}
/// Drop location ranges which exist entirely outside each variable's scope.
- void trimLocationRanges(const MachineFunction &MF, LexicalScopes &LScopes);
+ void trimLocationRanges(const MachineFunction &MF, LexicalScopes &LScopes,
+ const InstructionOrdering &Ordering);
bool empty() const { return VarEntries.empty(); }
void clear() { VarEntries.clear(); }
EntriesMap::const_iterator begin() const { return VarEntries.begin(); }
diff --git a/llvm/include/llvm/CodeGen/DebugHandlerBase.h b/llvm/include/llvm/CodeGen/DebugHandlerBase.h
index 4ff0fdea36ae..b488979f458c 100644
--- a/llvm/include/llvm/CodeGen/DebugHandlerBase.h
+++ b/llvm/include/llvm/CodeGen/DebugHandlerBase.h
@@ -110,6 +110,9 @@ class DebugHandlerBase : public AsmPrinterHandler {
virtual void endFunctionImpl(const MachineFunction *MF) = 0;
virtual void skippedNonDebugFunction() {}
+private:
+ InstructionOrdering InstOrdering;
+
// AsmPrinterHandler overrides.
public:
void beginInstruction(const MachineInstr *MI) override;
@@ -129,8 +132,10 @@ class DebugHandlerBase : public AsmPrinterHandler {
/// If this type is derived from a base type then return base type size.
static uint64_t getBaseTypeSize(const DIType *Ty);
+
+ const InstructionOrdering &getInstOrdering() const { return InstOrdering; }
};
-}
+} // namespace llvm
#endif
diff --git a/llvm/lib/CodeGen/AsmPrinter/DbgEntityHistoryCalculator.cpp b/llvm/lib/CodeGen/AsmPrinter/DbgEntityHistoryCalculator.cpp
index ab31381de407..df1a9617b87d 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DbgEntityHistoryCalculator.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DbgEntityHistoryCalculator.cpp
@@ -53,24 +53,6 @@ static Register isDescribedByReg(const MachineInstr &MI) {
: Register();
}
-/// Record instruction ordering so we can query their relative positions within
-/// a function. Meta instructions are given the same ordinal as the preceding
-/// non-meta instruction. Class state is invalid if MF is modified after
-/// calling initialize.
-class InstructionOrdering {
-public:
- void initialize(const MachineFunction &MF);
- void clear() { InstNumberMap.clear(); }
-
- /// Check if instruction \p A comes before \p B, where \p A and \p B both
- /// belong to the MachineFunction passed to initialize().
- bool isBefore(const MachineInstr *A, const MachineInstr *B) const;
-
-private:
- /// Each instruction is assigned an order number.
- DenseMap<const MachineInstr *, unsigned> InstNumberMap;
-};
-
void InstructionOrdering::initialize(const MachineFunction &MF) {
// We give meta instructions the same ordinal as the preceding instruction
// because this class is written for the task of comparing positions of
@@ -161,11 +143,9 @@ intersects(const MachineInstr *StartMI, const MachineInstr *EndMI,
return None;
}
-void DbgValueHistoryMap::trimLocationRanges(const MachineFunction &MF,
- LexicalScopes &LScopes) {
- InstructionOrdering Ordering;
- Ordering.initialize(MF);
-
+void DbgValueHistoryMap::trimLocationRanges(
+ const MachineFunction &MF, LexicalScopes &LScopes,
+ const InstructionOrdering &Ordering) {
// The indices of the entries we're going to remove for each variable.
SmallVector<EntryIndex, 4> ToRemove;
// Entry reference count for each variable. Clobbers left with no references
diff --git a/llvm/lib/CodeGen/AsmPrinter/DebugHandlerBase.cpp b/llvm/lib/CodeGen/AsmPrinter/DebugHandlerBase.cpp
index a46de83e555c..9693248de70f 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DebugHandlerBase.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DebugHandlerBase.cpp
@@ -196,8 +196,9 @@ void DebugHandlerBase::beginFunction(const MachineFunction *MF) {
assert(DbgLabels.empty() && "DbgLabels map wasn't cleaned!");
calculateDbgEntityHistory(MF, Asm->MF->getSubtarget().getRegisterInfo(),
DbgValues, DbgLabels);
+ InstOrdering.initialize(*MF);
if (TrimVarLocs)
- DbgValues.trimLocationRanges(*MF, LScopes);
+ DbgValues.trimLocationRanges(*MF, LScopes, InstOrdering);
LLVM_DEBUG(DbgValues.dump());
// Request labels for the full history.
@@ -333,6 +334,7 @@ void DebugHandlerBase::endFunction(const MachineFunction *MF) {
DbgLabels.clear();
LabelsBeforeInsn.clear();
LabelsAfterInsn.clear();
+ InstOrdering.clear();
}
void DebugHandlerBase::beginBasicBlock(const MachineBasicBlock &MBB) {
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
index 0730fff3bcaf..404fb2f8a806 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
@@ -1518,7 +1518,8 @@ void DwarfDebug::collectVariableInfoFromMFTable(
/// either open or otherwise rolls off the end of the scope.
static bool validThroughout(LexicalScopes &LScopes,
const MachineInstr *DbgValue,
- const MachineInstr *RangeEnd) {
+ const MachineInstr *RangeEnd,
+ const InstructionOrdering &Ordering) {
assert(DbgValue->getDebugLoc() && "DBG_VALUE without a debug location");
auto MBB = DbgValue->getParent();
auto DL = DbgValue->getDebugLoc();
@@ -1586,9 +1587,8 @@ static bool validThroughout(LexicalScopes &LScopes,
// The location range and variable's enclosing scope are both contained within
// MBB, test if location terminates before end of scope.
- for (auto I = RangeEnd->getIterator(); I != MBB->end(); ++I)
- if (&*I == LScopeEnd)
- return false;
+ if (Ordering.isBefore(RangeEnd, LScopeEnd))
+ return false;
// There's a single location which starts at the scope start, and ends at or
// after the scope end.
@@ -1734,7 +1734,7 @@ bool DwarfDebug::buildLocationList(
return false;
if (VeryLargeBlocks.count(StartDebugMI->getParent()))
return false;
- return validThroughout(LScopes, StartDebugMI, EndMI);
+ return validThroughout(LScopes, StartDebugMI, EndMI, getInstOrdering());
}
DbgEntity *DwarfDebug::createConcreteEntity(DwarfCompileUnit &TheCU,
@@ -1810,7 +1810,7 @@ void DwarfDebug::collectEntityInfo(DwarfCompileUnit &TheCU,
const auto *End =
SingleValueWithClobber ? HistoryMapEntries[1].getInstr() : nullptr;
if (VeryLargeBlocks.count(MInsn->getParent()) == 0 &&
- validThroughout(LScopes, MInsn, End)) {
+ validThroughout(LScopes, MInsn, End, getInstOrdering())) {
RegVar->initializeDbgValue(MInsn);
continue;
}
diff --git a/llvm/test/DebugInfo/MIR/AArch64/implicit-def-dead-scope.mir b/llvm/test/DebugInfo/MIR/AArch64/implicit-def-dead-scope.mir
index d85f2d25391d..2c2fb5e5de9e 100644
--- a/llvm/test/DebugInfo/MIR/AArch64/implicit-def-dead-scope.mir
+++ b/llvm/test/DebugInfo/MIR/AArch64/implicit-def-dead-scope.mir
@@ -5,9 +5,8 @@
# encountering an IMPLICIT_DEF in its own lexical scope.
# CHECK: .debug_info contents:
-# CHECK: DW_TAG_formal_parameter
-# CHECK: DW_AT_location [DW_FORM_sec_offset]
-# CHECK-NEXT: DW_OP_lit0, DW_OP_stack_value
+# CHECK: DW_TAG_formal_parameter [14]
+# CHECK-NEXT: DW_AT_const_value [DW_FORM_udata] (0)
# CHECK-NEXT: DW_AT_abstract_origin {{.*}} "name"
--- |
; ModuleID = 't.ll'
diff --git a/llvm/test/DebugInfo/MIR/X86/callsite-stack-value.mir b/llvm/test/DebugInfo/MIR/X86/callsite-stack-value.mir
index 5b9ecf08150b..4362f8e66b21 100644
--- a/llvm/test/DebugInfo/MIR/X86/callsite-stack-value.mir
+++ b/llvm/test/DebugInfo/MIR/X86/callsite-stack-value.mir
@@ -2,8 +2,7 @@
# RUN: -emit-call-site-info | llvm-dwarfdump - | FileCheck %s -implicit-check-not=call_site_parameter
# CHECK: DW_TAG_formal_parameter
-# CHECK-NEXT: DW_AT_location
-# CHECK-NEXT: DW_OP_reg17
+# CHECK-NEXT: DW_AT_location (DW_OP_reg17 XMM0)
# struct S {
# float w;
diff --git a/llvm/test/DebugInfo/X86/single-location-2.mir b/llvm/test/DebugInfo/X86/single-location-2.mir
new file mode 100644
index 000000000000..e3f0ec979e22
--- /dev/null
+++ b/llvm/test/DebugInfo/X86/single-location-2.mir
@@ -0,0 +1,92 @@
+# RUN: llc %s --start-after=livedebugvalues -filetype=obj -o - \
+# RUN: | llvm-dwarfdump - -name local* -regex \
+# RUN: | FileCheck %s
+#
+## This tests certain single location detection functionality. The Test MIR
+## is hand written. Test directives and comments inline.
+
+--- |
+ target triple = "x86_64-unknown-linux-gnu"
+ define dso_local i32 @fun() local_unnamed_addr !dbg !7 {
+ entry:
+ ret i32 0
+ }
+
+ !llvm.dbg.cu = !{!0}
+ !llvm.module.flags = !{!3, !4, !5}
+ !llvm.ident = !{!6}
+
+ !0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 11.0.0", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, splitDebugInlining: false, nameTableKind: None)
+ !1 = !DIFile(filename: "example.c", directory: "/")
+ !3 = !{i32 7, !"Dwarf Version", i32 4}
+ !4 = !{i32 2, !"Debug Info Version", i32 3}
+ !5 = !{i32 1, !"wchar_size", i32 4}
+ !6 = !{!"clang version 11.0.0"}
+ !8 = !DISubroutineType(types: !9)
+ !9 = !{!10}
+ !10 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+ !22 = !DISubroutineType(types: !23)
+ !23 = !{!10, !10}
+ ; --- Important metadata ---
+ !7 = distinct !DISubprogram(name: "fun", scope: !1, file: !1, line: 2, type: !8, scopeLine: 2, flags: DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0)
+ !24 = distinct !DILexicalBlock(scope: !7, file: !1, line: 9, column: 3)
+ !14 = distinct !DILexicalBlock(scope: !7, file: !1, line: 4, column: 3)
+ !12 = !DILocalVariable(name: "locala", scope: !7, file: !1, line: 1, type: !10)
+ !13 = !DILocalVariable(name: "localb", scope: !14, file: !1, line: 2, type: !10)
+ !25 = !DILocalVariable(name: "localc", scope: !24, file: !1, line: 3, type: !10)
+ !27 = !DILocalVariable(name: "tmp", scope: !14, file: !1, line: 2, type: !10)
+ !15 = !DILocation(line: 1, column: 0, scope: !7)
+ !18 = !DILocation(line: 2, column: 1, scope: !14)
+ !26 = !DILocation(line: 3, column: 1, scope: !24)
+...
+---
+name: fun
+body: |
+ bb.0.entry:
+ ;; This is the scope and variable structure:
+ ;; int fun() { // scope fun !7
+ ;; int locala; // scope fun !7, var locala !12, debug-location !15
+ ;; { int localb; // scope fun:block !14, var localb !13, debug-location !18
+ ;; int tmp; } // scope fun:block !14, var localb !27, debug-location !18
+ ;; { int localc; } // scope fun:block !24, var localc !25, debug-location !26
+ ;; }
+ ;;
+ ;; (1) Check that frame-setup instructions are not counted against
+ ;; locations being valid throughout the function call.
+ ;
+ ; CHECK: DW_TAG_variable
+ ; CHECK-NEXT: DW_AT_location (DW_OP_reg5 RDI)
+ ; CHECK-NEXT: DW_AT_name ("locala")
+ $rbp = frame-setup MOV64rr $rsp
+ DBG_VALUE $edi, $noreg, !12, !DIExpression(), debug-location !15
+ $eax = MOV32ri 0, debug-location !15
+
+ ;; (2) The scope block ends with a meta instruction. A location range ends
+ ;; with the final non-meta instruction in the scope. Check that
+ ;; location is considered valid throughout.
+ ;
+ ; CHECK: DW_TAG_variable
+ ; CHECK-NEXT: DW_AT_location (DW_OP_reg2 RCX)
+ ; CHECK-NEXT: DW_AT_name ("localb")
+ ;
+ ;; start scope, start location range
+ DBG_VALUE $ecx, $noreg, !13, !DIExpression(), debug-location !18
+ ;; end location range
+ $ecx = MOV32ri 1, debug-location !18
+ ;; end scope
+ DBG_VALUE $noreg, $noreg, !27, !DIExpression(), debug-location !18
+
+ ;; (3) The final instruction in the scope closes a location range. Check
+ ;; that location is considered valid throughout.
+ ;
+ ; CHECK: DW_TAG_variable
+ ; CHECK-NEXT: DW_AT_location (DW_OP_reg4 RSI)
+ ; CHECK-NEXT: DW_AT_name ("localc")
+ ;
+ ;; start scope, start location range
+ DBG_VALUE $esi, $noreg, !25, !DIExpression(), debug-location !26
+ ;; end scope, end location range
+ $esi = MOV32ri 2, debug-location !26
+
+ RETQ debug-location !15
+...
diff --git a/llvm/test/DebugInfo/X86/trim-var-locs.mir b/llvm/test/DebugInfo/X86/trim-var-locs.mir
index 04ab56d302ae..9c1de2593fa5 100644
--- a/llvm/test/DebugInfo/X86/trim-var-locs.mir
+++ b/llvm/test/DebugInfo/X86/trim-var-locs.mir
@@ -81,8 +81,7 @@ body: |
; block is not trimmed.
;
; CHECK: DW_TAG_variable
- ; CHECK-NEXT: DW_AT_location
- ; CHECK-NEXT: DW_OP_reg5 RDI
+ ; CHECK-NEXT: DW_AT_location (DW_OP_reg5 RDI)
; CHECK-NEXT: DW_AT_name ("localb")
;
; localb range 2 clobber in scope fun !7 (outside block !14)
More information about the llvm-commits
mailing list