[PATCH] D54217: Add total function byte size and inline function byte size to "llvm-dwarfdump --statistics"

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

In https://reviews.llvm.org/D54217#1291870, @dblaikie wrote:

> In https://reviews.llvm.org/D54217#1291854, @clayborg wrote:
>
> > In https://reviews.llvm.org/D54217#1291834, @dblaikie wrote:
> >
> > > In https://reviews.llvm.org/D54217#1291820, @clayborg wrote:
> > >
> > > > yeah, an easy code search indeed kicked up the tests. I will add one. Sorry about that
> > >
> > >
> > > No worries - still curious where you were looking for the tests initially, perhaps we can do something to make them more discoverable?
> >
> >
> > The only thing I could think of that could make it easier is to make a symlink from the tools/llvm-dwarfdump/test which points to ../../test/tools/llvm-dwarfdump
>
>
> Ah, fair enough - yeah, might just be a hurdle we can live with, then. The tests mostly match the directory hierarchy - but yeah, there are a few quirks.
>
> >> (another way I find tests: Break something nearby/related (eg: remove one of the existing statistics) & see what tests fail)
> >> 
> >> In https://reviews.llvm.org/D54217#1291828, @clayborg wrote:
> >> 
> >>> Looks like I can piggy back on llvm/test/tools/llvm-dwarfdump/X86/statistics.ll
> >>>
> >>> Does anyone know how to run only this test from command line?
> >> 
> >> 
> >> from the build directory I usually run "./bin/llvm-lit -v test/tools/llvm-dwarfdump/X86/statistics.ll" for example (yeah, even though that relative path doesn't exist in the build directory, lit knows to also try resolving it from the source dir, etc)
> > 
> > Thanks! Will be an easy tests to add. One other FileCheck question. Seems like I would want to check that "total function size" and "total inlined function size" exist. 
> >  Can I rely on the exact byte sizes from this .ll file on all systems?
>
> Only if the test has a specific triple. Which, by the looks of it, this test could benefit from. @aprantl - would that make sense? This test is only run when the X86 target is enabled anyway, so adding a specific triple wouldn't make it less portable - and would allow for specific statistic values to be tested, which seems like it'd be important for test coverage here (as it stands, byte count parts of this test look like it would still pass even if the statistics were miscomputed/all zero, etc).


@aprantl - any thoughts on the test coverage here. Currently there's no testing that these counters are correct/meaningful - just that they are printed out. Reckon it's worth putting a specific triple here & testing the values to ensure the computation is correct/doesn't regress?


https://reviews.llvm.org/D54217