[PATCH] D158668: Add getLikelyBranchWeight helper function

Paul Kirth via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Aug 23 15:15:19 PDT 2023


paulkirth added a comment.

My initial reaction to this was that we should keep the `--unlikely-branch-weights` flag available, even if we decide to move forward with the other changes. but given that you haven't had to update many tests, I think its probably fine to go this way. Probably just an FYI on discourse is sufficient to notify potential downstream consumers.

As for exposing the Likely/unlikely weights ... I'm a bit conflicted. On one hand, I would love a simple way to query the current weights for MisExpect diagnostics, and some of the other things we've been prototyping like suggesting places where annotations would be profitable. On the other hand, I dislike exposing some internal detail of a pass.

That said, it seems like these have more value when exposed, so I think it woudl be fine, as long as we can prevent frontends from using the APIs, like you mentioned in https://reviews.llvm.org/D158642#4611025.

I'm not sure how to separate the APIs  the way we want, though. Most of the APIs in "ProfDataUtils.h" are desirable to be public, which means `include` is a good place for them, but `getLikelyBranchWeight` may not fit well in that regard... Maybe we need an internal header in `lib`? or perhaps I'm thinking bout this too much...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158668



More information about the cfe-commits mailing list