[PATCH] D12781: PGO IR-level instrumentation infrastructure

[Justin Bogner via llvm-commits]

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.