[PATCH] D58849: llvm-dwarfdump: Add new variable, parameter and inlining statistics; also function source location statistics.

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: test/tools/llvm-dwarfdump/X86/Inputs/statistics-fib.s:1
+	.text
+	.file	"fib.c"
----------------
aprantl wrote:
> Do we need this as assembler or would it be more compact if we checked in llvm IR instead and compile it with llc on the fly?
This was a suggestion by @echristo and myself - one we made admittedly without looking at the existing testing for statistics, but I still agree with having seen the existing tests.

The statistics tests are standout as the only tests for llvm-dwarfdump that don't use either assembly or object files (& their coverage is a bit limited because of it - note the existing tests don't test any of the instruction counts, because they could vary as LLVM changes happen).

My take on it is that testing llvm-dwarfdump shouldn't also involve/be dependent on the behavior of LLVM's DWARF emission - because it means more points of failure reflected in a single test, so the test is less actionable when it does fail (is this a bug in the test, a bug in LLVM's DWARF emission, or a mismatch between the two and the test expectations need to be updated because of it)

(In the abstract ideal, something like yaml2obj would have high level abstractions for DWARF making it easy to hand write specific inputs (potentially even corrupt inputs) to various tool tests)




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

https://reviews.llvm.org/D58849