<div dir="ltr">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.<div><br></div><div>I think adding C++ unit tests are independent tasks which can be done once the driver part of the changes land.</div><div><br></div><div>David</div><div><br></div><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Nov 19, 2015 at 6:40 PM, Sean Silva <span dir="ltr"><<a href="mailto:chisophugis@gmail.com" target="_blank">chisophugis@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">silvas added a comment.<br>
<br>
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.<br>
<br>
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.<br>
<br>
<br>
<a href="http://reviews.llvm.org/D12781" rel="noreferrer" target="_blank">http://reviews.llvm.org/D12781</a><br>
<br>
<br>
<br>
</blockquote></div><br></div>