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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to