[PATCH] D66324: clang-misexpect: Profile Guided Validation of Performance Annotations in LLVM

[Paul Kirth via Phabricator via cfe-commits]

paulkirth marked 2 inline comments as done.
paulkirth added inline comments.


================
Comment at: llvm/test/Transforms/LowerExpectIntrinsic/basic.ll:141
   %tobool = icmp ne i64 %expval, 0
-; CHECK: !prof !1
+; CHECK: !prof !2
 ; CHECK-NOT: @llvm.expect
----------------
lebedev.ri wrote:
> Here and elsewhere:
> is `!prof !1` no longer present?
The misexpect metadata tags take those slots.

`br i1 %tobool, label %if.then, label %if.end, !prof !0, !misexpect !1`

Unless my understanding of how the tags work is flawed, I think this is the expected behavior


================
Comment at: llvm/test/Transforms/LowerExpectIntrinsic/basic.ll:292-293
+; CHECK: !3 = !{!"misexpect", i64 1, i64 2000, i64 1}
+; CHECK: !4 = !{!"branch_weights", i32 1, i32 2000, i32 1}
+; CHECK: !5 = !{!"branch_weights", i32 2000, i32 1, i32 1}
----------------
lebedev.ri wrote:
> Should there be a FIXME, are some `misexpect` missing here?
This is happenstance unfortunately.

```
; CHECK: !4 = !{!"branch_weights", i32 1, i32 2000, i32 1}
; CHECK: !5 = !{!"branch_weights", i32 2000, i32 1, i32 1}
```

In both of these cases the expected index is still 0 or 1, so the metadata tags are deduplicated. 

If we want to check further, then we need to modify the test to select an index greater than 1.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66324/new/

https://reviews.llvm.org/D66324