[PATCH] D12781: PGO IR-level instrumentation infrastructure

[Sean Silva via llvm-commits]

silvas added a comment.

The tests are looking great! A couple nits and a final question about the test.


================
Comment at: test/Transforms/PGOProfile/branch1.ll:31
@@ +30,2 @@
+
+; USE: ![[BW_ENTRY]] = !{!"branch_weights", i32 2, i32 1}
----------------
Can you use CHECK-DAG directive to move these next to the place where the `BW_*` capture occurs? That would improve locality and clarify what is being checked.

(same in the other files)

================
Comment at: test/Transforms/PGOProfile/checksum_mismatch.ll:4
@@ +3,3 @@
+
+; CHECK: Function control flow change detected (hash mismatch) single_bb
+; CHECK: No profile data available for function uncalled
----------------
Tiny nit: could you name all the tests which are just checking diagnostics to match `diag_*.ll`? (or whatever naming convention seems appropriate)

That will help to clearly identify them.

================
Comment at: test/Transforms/PGOProfile/loop2.ll:9
@@ +8,3 @@
+
+define i32 @test_nested_for(i32 %r, i32 %s, i32 %t) {
+entry:
----------------
What is the importance of testing a nested for loop? What part of the code are you trying to exercise that isn't covered by loop1.ll?


http://reviews.llvm.org/D12781