[DwarfDebug] Improve single location detection in validThroughout (2/4)
authorOCHyams <orlando.hyams@sony.com>
Thu, 27 Aug 2020 08:40:50 +0000 (09:40 +0100)
committerOCHyams <orlando.hyams@sony.com>
Thu, 27 Aug 2020 10:52:29 +0000 (11:52 +0100)
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

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/single-location-2.mir [new file with mode: 0644]
llvm/test/DebugInfo/X86/trim-var-locs.mir

index 5a1085e..bca6065 100644 (file)
@@ -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 @@ public:
   }
 
   /// 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(); }
index 4ff0fde..b488979 100644 (file)
@@ -110,6 +110,9 @@ protected:
   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 @@ public:
 
   /// 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
index ab31381..df1a961 100644 (file)
@@ -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
index a46de83..9693248 100644 (file)
@@ -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) {
index 0730fff..404fb2f 100644 (file)
@@ -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;
       }
index d85f2d2..2c2fb5e 100644 (file)
@@ -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'
index 5b9ecf0..4362f8e 100644 (file)
@@ -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 (file)
index 0000000..e3f0ec9
--- /dev/null
@@ -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
+...
index 04ab56d..9c1de25 100644 (file)
@@ -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)