[PATCH] D12781: PGO IR-level instrumentation infrastructure

Xinliang David Li via llvm-commits llvm-commits at lists.llvm.org
Thu Nov 19 18:59:04 PST 2015


I thought about the size of the test case too. Initially I had the same
concern, but in second look, it does not seem to be brittle -- because it
just checks whether the key branches are properly annotated with the right
profile count or not, or profile counter update instruction is inserted or
not.

I think adding C++ unit tests are independent tasks which can be done once
the driver part of the changes land.

David



On Thu, Nov 19, 2015 at 6:40 PM, Sean Silva <chisophugis at gmail.com> wrote:

> silvas added a comment.
>
> 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.
>
>
> http://reviews.llvm.org/D12781
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151119/d47fc5fa/attachment.html>


More information about the llvm-commits mailing list