[PATCH] D85636: [llvm-dwarfdump] Fix misleading scope byte coverage statistics

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: llvm/test/tools/llvm-dwarfdump/X86/stats-scope-bytes-covered.yaml:17-23
+##     DW_TAG_lexical_block
+##       DW_AT_low_pc	(0x0000000000000005)
+##       DW_AT_high_pc	(0x000000000000000a)
+##
+##       DW_TAG_variable
+##         DW_AT_location	(0x00000000:
+##            [0x0000000000000000, 0x0000000000000005): DW_OP_reg5 RDI)
----------------
Might be worth making this test case a bit more interesting by having /some/ overlap (and some non-overlap) regions.


================
Comment at: llvm/tools/llvm-dwarfdump/Statistics.cpp:296-312
         for (auto Entry : *Loc) {
-          uint64_t BytesEntryCovered = Entry.Range->HighPC - Entry.Range->LowPC;
-          BytesCovered += BytesEntryCovered;
+          TotalBytesCovered += Entry.Range->HighPC - Entry.Range->LowPC;
+          uint64_t ScopeBytesCoveredByEntry = 0;
+          // Calculate how many bytes of the parent scope this entry covers.
+          // FIXME: In section 2.6.2 of the DWARFv5 spec it says that "The
+          // address ranges defined by the bounded location descriptions of a
+          // location list may overlap". So in theory a variable can have
----------------
Little concerned about the N^2 complexity of this approach, and wondering if an alternative approach might also address the limitation regarding overlapping locations.

Start with a range of intervals (without overlaps) for the enclosing scope. Then intersect any location ranges with that collection of intervals and store those in another non-overlapping interval range.

A small data structure that lets you build a non-overlapping list of intervals (merging intervals, etc), keeping them sorted - then it'll be easier to find other overlapping regions.

eg:
```
Intervals Scope;
for (address ranges in scope)
  Scope.insert(R);
Intervals Loc;
for (address ranges in location) {
  R' = Scope.intersection(R);
  Loc.insert(R'); 
}
scope bytes covered = Loc.total_coverage/size/something
```

But, yeah, maybe the implementation of that's more complicated than is justified for this for now.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85636/new/

https://reviews.llvm.org/D85636