[PATCH] D158889: [AsmPrinter][PGO] Adds optional dumping of branch probabilities for PGO metrics.

[Wenlei He via Phabricator via llvm-commits]

wenlei added a comment.

In D158889#4621393 <https://reviews.llvm.org/D158889#4621393>, @red1bluelost wrote:

> In D158889#4619737 <https://reviews.llvm.org/D158889#4619737>, @hoy wrote:
>
>> In D158889#4618519 <https://reviews.llvm.org/D158889#4618519>, @wenlei wrote:
>>
>>> We have something similar internally, but didn't upstream because we are not sure if the use case is too narrow to justify burdening the code base with all the related complexity.  cc @hoy
>>
>> Yeah, we internally serialize machine block execution counts to the binary.  Would it be helpful to write those counts into some bb section similar to the `llvm_bb_addr_map` section?
>
> This is interesting and similar but our metrics are only focusing on branch weights at the moment. We've focused on branch weights so far since they are a direct result of Profile Loading.
>
> At least for the current state of the metrics here and consumed in D158890 <https://reviews.llvm.org/D158890>, there is not a use for block counts but it could be a future extension of the metrics.

We're probably among the few people that will actually benefit from something like this, but honestly I'm still a bit unsure whether the use case is common enough to justify built-in support like this.

However, if we make something like this part of llvm, I suggest we at least make it as general purpose as possible. A few comments related to that:

1. Try to incorporate block counts/frequencies as well. Most of the researches on profile quality use a block overlap metric which relies on block counts rather than branch probabilities. Our internal version also uses block counts, as branch weights cannot represent the profile for branchless code.

2. Instead of coupling this with a specific consumer, Pin tool in your case, and in the next patch, I suggest we build general support to decode such metadata section, so tools like llvm-objdump can be used to inspect its payload.


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

https://reviews.llvm.org/D158889