[PATCH] D124302: [llvm][misexpect] Replace assertion with early return

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added a comment.

In D124302#3469029 <https://reviews.llvm.org/D124302#3469029>, @paulkirth wrote:

> One concern I have here, since someone reported this to me is that perhaps some of MisExpect's assumptions regarding when branch weight metadata will be attached is erroneous.
>
> The key assumptions we make are that for frontend instrumentation that any branch weights that have been attached when LowerExpectIntrinsics runs will originate from profiling. The other key assumption we make is that when branch weights are assigned for IR instrumentation that LowerExpectIntrinsic is the only place that can add these weights before they are added by the IR or sample profiles. If that's not the case, then we need to discuss a way to address that shortcoming.
>
> That concern doesn't change the need for this fix, but may require some more in depth discussion on discourse about how to address this in a comprehensive manner.

The failing test case was with SamplePGO. I went back and looked at the original patch and realized that it doesn't seem to have any test cases for SamplePGO, even though it is being called there. It probably makes sense to look more closely at the IR in that case to see how this code is getting the 0 "expected" edge weights. Rather than this change, how about disabling misexpect verification for sample PGO as a short term fix, then doing some investigation and adding a fix along with sample PGO tests when re-enabling it there?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124302