[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