[llvm-dwarfdump][Statistics] Unify coverage statistic computation

Summary:
The patch removes OffsetToFirstDefinition in the 'scope bytes total'
statistic computation. Thus it unifies the way the scope and the coverage
buckets are computed. The rationals behind that are the following:

1. OffsetToFirstDefinition was used to calculate the variable's life range.
However, there is no simple way to do it accurately, so the scope calculated
this way might be misleading. See D69027 for more details on the subject.
2. Both 'scope bytes total' and coverage buckets seem to be intended
to represent the same data in different ways. Otherwise, the statistics
might be controversial and confusing.

Note that the approach gives up a thorough evaluation of debug information
completeness (i.e. coverage buckets by themselves doesn't tell how good
the debug information is). Only changes in coverage over time make
a 'physical' sense.

Reviewers: djtodoro, aprantl, vsk, dblaikie, avl

Subscribers: llvm-commits

Tags: #llvm

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

Author: chbessonova

llvm/test/tools/llvm-dwarfdump/X86/locstats.ll

@@ -5,7 +5,7 @@
 ; CHECK: "formal params scope bytes total":20
 ; CHECK: "formal params scope bytes covered":20
 ; CHECK: "formal params entry value scope bytes covered":5
-; CHECK: "vars scope bytes total":78
+; CHECK: "vars scope bytes total":90
 ; CHECK: "vars scope bytes covered":60
 ; CHECK: "vars entry value scope bytes covered":0
 ; CHECK: "total variables procesed by location statistics":6

llvm/test/tools/llvm-dwarfdump/X86/statistics-base-address.s

@@ -5,7 +5,7 @@
 # RUN: llvm-mc -triple x86_64-pc-linux %s -filetype=obj -o %t
 # RUN: llvm-dwarfdump --statistics %t | FileCheck %s
 
-# CHECK: "vars scope bytes total":8
+# CHECK: "vars scope bytes total":12
 # CHECK: "vars scope bytes covered":8
 
         .text

llvm/test/tools/llvm-dwarfdump/X86/statistics-dwo.test

@@ -1,5 +1,5 @@
 # Test of the llmv-dwarfdump --statistics with split dwarf (dwo files)
-# (version 3).
+# (version >= 3).
 #
 # Create a directory in which to put all files, so .o file can find .dwo file.
 RUN: rm -rf %t && mkdir -p %t
@@ -69,7 +69,7 @@ RUN: llvm-dwarfdump --statistics statistics-fib.split-dwarf.o | FileCheck %s
 # }
 #
 
-CHECK: "version":3
+CHECK: "version":4
 CHECK: "source functions":3
 CHECK: "source functions with location":3
 CHECK: "inlined functions":7
@@ -80,7 +80,7 @@ CHECK: "source variables":30
 # Ideally the value below would be 33 but currently it's not.
 CHECK: "variables with location":22
 CHECK: "call site entries":7
-CHECK: "scope bytes total":2702
+CHECK: "scope bytes total":2817
 CHECK: "scope bytes covered":1160
 CHECK: "total function size":594
 CHECK: "total inlined function size":345

llvm/test/tools/llvm-dwarfdump/X86/statistics-v3.test

@@ -1,4 +1,4 @@
-# Test of the llmv-dwarfdump --statistics newly added stats (version 3).
+# Test of the llmv-dwarfdump --statistics newly added stats (version >= 3).
 #
 RUN: llvm-mc -triple x86_64-unknown-linux-gnu %S/Inputs/statistics-fib.s -filetype=obj -o %t-statistics-fib.o
 RUN: llvm-dwarfdump --statistics %t-statistics-fib.o | FileCheck %s
@@ -64,7 +64,7 @@ RUN: llvm-dwarfdump --statistics %t-statistics-fib.o | FileCheck %s
 # }
 #
 
-CHECK: "version":3
+CHECK: "version":4
 CHECK: "source functions":3
 CHECK: "source functions with location":3
 CHECK: "inlined functions":8
@@ -75,7 +75,7 @@ CHECK: "source variables":33
 # Ideally the value below would be 33 but currently it's not.
 CHECK: "variables with location":24
 CHECK: "call site entries":8
-CHECK: "scope bytes total":2958
+CHECK: "scope bytes total":3072
 CHECK: "scope bytes covered":1188
 CHECK: "total function size":636
 CHECK: "total inlined function size":388

llvm/test/tools/llvm-dwarfdump/X86/statistics.ll

@@ -1,6 +1,6 @@
 ; RUN: llc -O0 %s -o - -filetype=obj \
 ; RUN:   | llvm-dwarfdump -statistics - | FileCheck %s
-; CHECK: "version":3
+; CHECK: "version":4
 
 ; int GlobalConst = 42;
 ; int Global;

llvm/tools/llvm-dwarfdump/Statistics.cpp

@@ -60,28 +60,26 @@ struct PerFunctionStats {
 struct GlobalStats {
   /// Total number of PC range bytes covered by DW_AT_locations.
   unsigned ScopeBytesCovered = 0;
-  /// Total number of PC range bytes in each variable's enclosing scope,
-  /// starting from the first definition of the variable.
-  unsigned ScopeBytesFromFirstDefinition = 0;
+  /// Total number of PC range bytes in each variable's enclosing scope.
+  unsigned ScopeBytes = 0;
   /// Total number of PC range bytes covered by DW_AT_locations with
   /// the debug entry values (DW_OP_entry_value).
   unsigned ScopeEntryValueBytesCovered = 0;
   /// Total number of PC range bytes covered by DW_AT_locations of
   /// formal parameters.
   unsigned ParamScopeBytesCovered = 0;
-  /// Total number of PC range bytes in each variable's enclosing scope,
-  /// starting from the first definition of the variable (only for parameters).
-  unsigned ParamScopeBytesFromFirstDefinition = 0;
+  /// Total number of PC range bytes in each variable's enclosing scope
+  /// (only for parameters).
+  unsigned ParamScopeBytes = 0;
   /// Total number of PC range bytes covered by DW_AT_locations with
   /// the debug entry values (DW_OP_entry_value) (only for parameters).
   unsigned ParamScopeEntryValueBytesCovered = 0;
   /// Total number of PC range bytes covered by DW_AT_locations (only for local
   /// variables).
   unsigned VarScopeBytesCovered = 0;
-  /// Total number of PC range bytes in each variable's enclosing scope,
-  /// starting from the first definition of the variable (only for local
-  /// variables).
-  unsigned VarScopeBytesFromFirstDefinition = 0;
+  /// Total number of PC range bytes in each variable's enclosing scope
+  /// (only for local variables).
+  unsigned VarScopeBytes = 0;
   /// Total number of PC range bytes covered by DW_AT_locations with
   /// the debug entry values (DW_OP_entry_value) (only for local variables).
   unsigned VarScopeEntryValueBytesCovered = 0;
@@ -133,19 +131,6 @@ struct LocationStats {
   unsigned NumVar = 0;
 };
 
-/// Extract the low pc from a Die.
-static uint64_t getLowPC(DWARFDie Die) {
-  auto RangesOrError = Die.getAddressRanges();
-  DWARFAddressRangesVector Ranges;
-  if (RangesOrError)
-    Ranges = RangesOrError.get();
-  else
-    llvm::consumeError(RangesOrError.takeError());
-  if (Ranges.size())
-    return Ranges[0].LowPC;
-  return dwarf::toAddress(Die.find(dwarf::DW_AT_low_pc), 0);
-}
-
 /// Collect debug location statistics for one DIE.
 static void collectLocStats(uint64_t BytesCovered, uint64_t BytesInScope,
                             std::vector<unsigned> &VarParamLocStats,
@@ -177,8 +162,8 @@ static void collectLocStats(uint64_t BytesCovered, uint64_t BytesInScope,
 
 /// Collect debug info quality metrics for one DIE.
 static void collectStatsForDie(DWARFDie Die, std::string FnPrefix,
-                               std::string VarPrefix, uint64_t ScopeLowPC,
-                               uint64_t BytesInScope, uint32_t InlineDepth,
+                               std::string VarPrefix, uint64_t BytesInScope,
+                               uint32_t InlineDepth,
                                StringMap<PerFunctionStats> &FnStatMap,
                                GlobalStats &GlobalStats,
                                LocationStats &LocStats) {
@@ -188,7 +173,6 @@ static void collectStatsForDie(DWARFDie Die, std::string FnPrefix,
   bool IsArtificial = false;
   uint64_t BytesCovered = 0;
   uint64_t BytesEntryValuesCovered = 0;
-  uint64_t OffsetToFirstDefinition = 0;
   auto &FnStats = FnStatMap[FnPrefix];
   bool IsParam = Die.getTag() == dwarf::DW_TAG_formal_parameter;
   bool IsLocalVar = Die.getTag() == dwarf::DW_TAG_variable;
@@ -262,16 +246,6 @@ static void collectStatsForDie(DWARFDie Die, std::string FnPrefix,
           if (IsEntryValue(Entry.Expr))
             BytesEntryValuesCovered += BytesEntryCovered;
         }
-        if (!Loc->empty()) {
-          uint64_t FirstDef = Loc->front().Range->LowPC;
-          // Ranges sometimes start before the lexical scope.
-          if (FirstDef >= ScopeLowPC)
-            OffsetToFirstDefinition = FirstDef - ScopeLowPC;
-          // Or even after it. Count that as a failure.
-          if (OffsetToFirstDefinition > BytesInScope)
-            OffsetToFirstDefinition = 0;
-        }
-        assert(BytesInScope);
       }
     }
   }
@@ -304,25 +278,21 @@ static void collectStatsForDie(DWARFDie Die, std::string FnPrefix,
   FnStats.VarsInFunction.insert(VarPrefix + VarName);
   if (BytesInScope) {
     FnStats.TotalVarWithLoc += (unsigned)HasLoc;
-    // Adjust for the fact the variables often start their lifetime in the
-    // middle of the scope.
-    BytesInScope -= OffsetToFirstDefinition;
     // Turns out we have a lot of ranges that extend past the lexical scope.
     GlobalStats.ScopeBytesCovered += std::min(BytesInScope, BytesCovered);
-    GlobalStats.ScopeBytesFromFirstDefinition += BytesInScope;
+    GlobalStats.ScopeBytes += BytesInScope;
     GlobalStats.ScopeEntryValueBytesCovered += BytesEntryValuesCovered;
     if (IsParam) {
       GlobalStats.ParamScopeBytesCovered +=
           std::min(BytesInScope, BytesCovered);
-      GlobalStats.ParamScopeBytesFromFirstDefinition += BytesInScope;
+      GlobalStats.ParamScopeBytes += BytesInScope;
       GlobalStats.ParamScopeEntryValueBytesCovered += BytesEntryValuesCovered;
     } else if (IsLocalVar) {
       GlobalStats.VarScopeBytesCovered += std::min(BytesInScope, BytesCovered);
-      GlobalStats.VarScopeBytesFromFirstDefinition += BytesInScope;
+      GlobalStats.VarScopeBytes += BytesInScope;
       GlobalStats.VarScopeEntryValueBytesCovered += BytesEntryValuesCovered;
     }
-    assert(GlobalStats.ScopeBytesCovered <=
-           GlobalStats.ScopeBytesFromFirstDefinition);
+    assert(GlobalStats.ScopeBytesCovered <= GlobalStats.ScopeBytes);
   } else if (Die.getTag() == dwarf::DW_TAG_member) {
     FnStats.ConstantMembers++;
   } else {
@@ -351,8 +321,8 @@ static void collectStatsForDie(DWARFDie Die, std::string FnPrefix,
 
 /// Recursively collect debug info quality metrics.
 static void collectStatsRecursive(DWARFDie Die, std::string FnPrefix,
-                                  std::string VarPrefix, uint64_t ScopeLowPC,
-                                  uint64_t BytesInScope, uint32_t InlineDepth,
+                                  std::string VarPrefix, uint64_t BytesInScope,
+                                  uint32_t InlineDepth,
                                   StringMap<PerFunctionStats> &FnStatMap,
                                   GlobalStats &GlobalStats,
                                   LocationStats &LocStats) {
@@ -387,7 +357,6 @@ static void collectStatsRecursive(DWARFDie Die, std::string FnPrefix,
     uint64_t BytesInThisScope = 0;
     for (auto Range : Ranges)
       BytesInThisScope += Range.HighPC - Range.LowPC;
-    ScopeLowPC = getLowPC(Die);
 
     // Count the function.
     if (!IsBlock) {
@@ -423,8 +392,8 @@ static void collectStatsRecursive(DWARFDie Die, std::string FnPrefix,
     }
   } else {
     // Not a scope, visit the Die itself. It could be a variable.
-    collectStatsForDie(Die, FnPrefix, VarPrefix, ScopeLowPC, BytesInScope,
-                       InlineDepth, FnStatMap, GlobalStats, LocStats);
+    collectStatsForDie(Die, FnPrefix, VarPrefix, BytesInScope, InlineDepth,
+                       FnStatMap, GlobalStats, LocStats);
   }
 
   // Set InlineDepth correctly for child recursion
@@ -441,9 +410,8 @@ static void collectStatsRecursive(DWARFDie Die, std::string FnPrefix,
     if (Child.getTag() == dwarf::DW_TAG_lexical_block)
       ChildVarPrefix += toHex(LexicalBlockIndex++) + '.';
 
-    collectStatsRecursive(Child, FnPrefix, ChildVarPrefix, ScopeLowPC,
-                          BytesInScope, InlineDepth, FnStatMap, GlobalStats,
-                          LocStats);
+    collectStatsRecursive(Child, FnPrefix, ChildVarPrefix, BytesInScope,
+                          InlineDepth, FnStatMap, GlobalStats, LocStats);
     Child = Child.getSibling();
   }
 }
@@ -496,13 +464,13 @@ bool collectStatsForObjectFile(ObjectFile &Obj, DWARFContext &DICtx,
   StringMap<PerFunctionStats> Statistics;
   for (const auto &CU : static_cast<DWARFContext *>(&DICtx)->compile_units())
     if (DWARFDie CUDie = CU->getNonSkeletonUnitDIE(false))
-      collectStatsRecursive(CUDie, "/", "g", 0, 0, 0, Statistics, GlobalStats,
+      collectStatsRecursive(CUDie, "/", "g", 0, 0, Statistics, GlobalStats,
                             LocStats);
 
   /// The version number should be increased every time the algorithm is changed
   /// (including bug fixes). New metrics may be added without increasing the
   /// version.
-  unsigned Version = 3;
+  unsigned Version = 4;
   unsigned VarParamTotal = 0;
   unsigned VarParamUnique = 0;
   unsigned VarParamWithLoc = 0;
@@ -562,19 +530,17 @@ bool collectStatsForObjectFile(ObjectFile &Obj, DWARFContext &DICtx,
   printDatum(OS, "call site entries", GlobalStats.CallSiteEntries);
   printDatum(OS, "call site DIEs", GlobalStats.CallSiteDIEs);
   printDatum(OS, "call site parameter DIEs", GlobalStats.CallSiteParamDIEs);
-  printDatum(OS, "scope bytes total",
-             GlobalStats.ScopeBytesFromFirstDefinition);
+  printDatum(OS, "scope bytes total", GlobalStats.ScopeBytes);
   printDatum(OS, "scope bytes covered", GlobalStats.ScopeBytesCovered);
   printDatum(OS, "entry value scope bytes covered",
              GlobalStats.ScopeEntryValueBytesCovered);
   printDatum(OS, "formal params scope bytes total",
-             GlobalStats.ParamScopeBytesFromFirstDefinition);
+             GlobalStats.ParamScopeBytes);
   printDatum(OS, "formal params scope bytes covered",
              GlobalStats.ParamScopeBytesCovered);
   printDatum(OS, "formal params entry value scope bytes covered",
              GlobalStats.ParamScopeEntryValueBytesCovered);
-  printDatum(OS, "vars scope bytes total",
-             GlobalStats.VarScopeBytesFromFirstDefinition);
+  printDatum(OS, "vars scope bytes total", GlobalStats.VarScopeBytes);
   printDatum(OS, "vars scope bytes covered", GlobalStats.VarScopeBytesCovered);
   printDatum(OS, "vars entry value scope bytes covered",
              GlobalStats.VarScopeEntryValueBytesCovered);
@@ -609,7 +575,7 @@ bool collectStatsForObjectFile(ObjectFile &Obj, DWARFContext &DICtx,
                    << "%\n";
       llvm::dbgs() << "PC Ranges covered: "
                    << (int)std::round((GlobalStats.ScopeBytesCovered * 100.0) /
-                                      GlobalStats.ScopeBytesFromFirstDefinition)
+                                      GlobalStats.ScopeBytes)
                    << "%\n");
   return true;
 }

llvm/utils/llvm-locstats/llvm-locstats.py

@@ -25,12 +25,12 @@ def locstats_output(
   variables_total_locstats,
   variables_with_loc,
   scope_bytes_covered,
-  scope_bytes_from_first_def,
+  scope_bytes,
   variables_coverage_map
   ):
 
   pc_ranges_covered = int(ceil(scope_bytes_covered * 100.0)
-              / scope_bytes_from_first_def)
+              / scope_bytes)
   variables_coverage_per_map = {}
   for cov_bucket in coverage_buckets():
     variables_coverage_per_map[cov_bucket] = \
@@ -99,7 +99,7 @@ def Main():
   variables_total_locstats = None
   variables_with_loc = None
   variables_scope_bytes_covered = None
-  variables_scope_bytes_from_first_def = None
+  variables_scope_bytes = None
   variables_scope_bytes_entry_values = None
   variables_coverage_map = {}
   binary = results.file_name
@@ -130,7 +130,7 @@ def Main():
       json_parsed['total vars procesed by location statistics']
     variables_scope_bytes_covered = \
       json_parsed['vars scope bytes covered']
-    variables_scope_bytes_from_first_def = \
+    variables_scope_bytes = \
       json_parsed['vars scope bytes total']
     if not results.ignore_debug_entry_values:
       for cov_bucket in coverage_buckets():
@@ -152,7 +152,7 @@ def Main():
       json_parsed['total params procesed by location statistics']
     variables_scope_bytes_covered = \
       json_parsed['formal params scope bytes covered']
-    variables_scope_bytes_from_first_def = \
+    variables_scope_bytes = \
       json_parsed['formal params scope bytes total']
     if not results.ignore_debug_entry_values:
       for cov_bucket in coverage_buckets():
@@ -177,7 +177,7 @@ def Main():
       json_parsed['total variables procesed by location statistics']
     variables_scope_bytes_covered = \
       json_parsed['scope bytes covered']
-    variables_scope_bytes_from_first_def = \
+    variables_scope_bytes = \
       json_parsed['scope bytes total']
     if not results.ignore_debug_entry_values:
       for cov_bucket in coverage_buckets():
@@ -200,7 +200,7 @@ def Main():
     variables_total_locstats,
     variables_with_loc,
     variables_scope_bytes_covered,
-    variables_scope_bytes_from_first_def,
+    variables_scope_bytes,
     variables_coverage_map
     )