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

[Orlando Cazalet-Hyams via Phabricator via llvm-commits]

Orlando 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)".
+#
----------------
dblaikie wrote:
> 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)
I agree that the test is clunky, but I'm a bit stumped on what a good test would look like.

I would like to avoid testing from IR/MIR because, as you point out, the situation we are checking for is buggy output from clang. The output is prone to getting fixed and could cause a false failure in the future. That is why I chose to use assembly in the test, but it's not very human friendly, even with a smaller very contrived input MIR to generate the assembly.

yaml2obj seems like a promising solution but the yaml generated by obj2yaml is doesn't convey the purpose of the test either IMO.

So IMO a non-contrived (and more complex) yaml or .s test with the source in the comments is the way forward here, wdyt?



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

https://reviews.llvm.org/D85636