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

David Blaikie via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Nov 8 10:36:16 PST 2018


dblaikie added a comment.

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).

>   I guess I don't really care what the value is, but I would like to verify that "total function size" > "total inlined function size". Is there a way to do that?
>    
> 
>   CHECK: "total function size":[[FUNCSIZE:[0-9]+]]
>   CHECK: "total inlined function size":[[INLINESIZE:[0-9]+]]
> 
> 
> Is there a way to check FUNCSIZE > INLINESIZE?

Yeah, there's no way to do that, unfortunately - but I think (will wait for Adrian to weigh in here, as the original author/has more context for this test/features than I do) it'd make sense to put a specific triple on this test and test specific values for all the stats here.
If adding new features to test to this test case perturbs all the other statistics in a way that makes this test a pain to maintain (I doubt it will - probably cheap enough to change the test, eyeball all the new stat values, and just update these "golden" values when we have to) it may be better to split new features that would otherwise perturb things into new test files.


Repository:
  rL LLVM

https://reviews.llvm.org/D54217





More information about the llvm-commits mailing list