[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.s:1-13
+# RUN: llvm-mc %s --filetype=obj -triple=x86_64-unknown-linux-gnu -o - \
+# RUN:   | llvm-dwarfdump --statistics - \
+# RUN:   | FileCheck %s
+#
+# Check that coverage for variable locations which do not cover the parent
+# scope is tracked separately in "sum_all_variables(#bytes in any scope)".
+#
----------------
Probably worth simplifying this test - it seems really long if it's only testing this property. (& if possible, including source code/steps to generate it - though probably better/easier to hand-craft DWARF for a case like this, since any case where LLVM produces locations wider than their scope is buggy & if that can't be tickled in a small example, maybe worth some hand-writing/tweaking of the DWARF or IR)


================
Comment at: llvm/tools/llvm-dwarfdump/Statistics.cpp:300-305
+          // 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
+          // multiple simultaneous locations, which would make this calculation
+          // misleading because we will count the overlapped areas
+          // twice. However, clang does not currently emit DWARF like this.
----------------
FWIW, I hope it does emit DWARF like this in the not-too-distant future. (specifically, if the variable's location is described using an entry_value, it might be good to use that entry_value for the entire scope of the function, even if the variable's value is available through other means as well - it'd be a more efficient encoding than having two separate ranges that use entry_value split because there's a range that uses an immediate value in between)


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

https://reviews.llvm.org/D85636