[llvm] [llvm][profdata][NFC] Support 64-bit weights in ProfDataUtils (PR #86607)

Paul Kirth via llvm-commits llvm-commits at lists.llvm.org
Tue Mar 26 11:04:32 PDT 2024


ilovepi wrote:

> can you explain when 32-bit vs 64-bit weights are used?

I think we generally work w/ 32-bit weights, but in some cases, we use 64-bit to make sure that summing or scaling don't overflow.  @MatzeB mentioned some of this in his comment. My motivation here is to avoid having bespoke handling of branch weight extraction. To a large extent due to to https://github.com/llvm/llvm-project/pull/86609. 

> Be careful here! I think there is a bunch of code that is summing up weights and stores intermediate results in a `uint64_t`. This is all fine when the weights are `uint32_t` but risks (silent!) overflow when the weights use the full `uint64_t`. I haven't looked around much but could find one random example in `SimplifyCFG` already:
> 
> https://github.com/llvm/llvm-project/blob/c8d70e94c4b69e809142054e75b9725ed70418af/llvm/lib/Transforms/Utils/SimplifyCFG.cpp?plain=1#L1221

Right, I don't want to change the defaults, I just want to provide better utilities, so people aren't manually walking the MD_prof metadata. When we add the proposed optional field in #86609, the offsets won't be fixed, and IMO, its better to provide the necessary utilities and point people towards those.

That's one of the reasons I left `extractBranchWeights()` APIs as they are now, only using `uint32_t`, which is the main way people should be extracting weights. I think the 32/64 APIs are a rather niche use case, but if passes need to operate on 64-bit weights, as in SimplifyCFG, then we should support that in ProfdataUtils.

> I am assuming that this also serves as preparation to increasing the bit size of the weight annotations?

That isn't one of my goals.

https://github.com/llvm/llvm-project/pull/86607


More information about the llvm-commits mailing list