[PATCH] D12781: PGO IR-level instrumentation infrastructure

[Xinliang David Li via llvm-commits]

Point taken :)

1) The test cases need to be greatly simplified -- same control flow with
original tests but with bare minimal other code
2) use text profile data.

thanks,

David


On Fri, Nov 20, 2015 at 1:07 PM, Justin Bogner <mail at justinbogner.com>
wrote:

> Sean Silva <chisophugis at gmail.com> writes:
> > These test files look like they are just a dump of IR generated from a
> > C/C++, which is extremely verbose and has a lot of inessential
> > details. I also don't like checking in binary profdata files. Overall
> > these tests seem extremely brittle. I also don't understand why the
> > test files have names like "for" or "goto" or "ifelse". Those concepts
> > don't exist in the IR. Surely the tests should have names like
> > "criticaledge" etc.
> >
> > It seems like this testing might be more readable as C++ unit
> > tests. Using the new pass manager this should be easy to wire up. I
> > think the hardest part would be to stub out IndexedInstrProfReader.
>
> Unit tests might be appropriate, but even as lit tests these can and
> should be massively simplified. The tests here do a lot of work that
> isn't relevant to what's being tested - there are loads and stores to
> various things which really have to do with the profile metadata.
>
> Hand rolled IR that is representative of interesting LLVM IR contructs
> would be much clearer about what's being tested, since it wouldn't have
> irrelevant details. This combined with using the .proftext format so
> that the input profiles are human readable and editable should simplify
> these tests greatly, without losing any coverage of the feature.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151120/d50819fb/attachment.html>