[PATCH] D26370: Use max(BFI_Count, TotalProfCount) to get block's profile count.

Dehao Chen via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 8 16:08:05 PST 2016


danielcdh added inline comments.


================
Comment at: lib/Analysis/BlockFrequencyInfo.cpp:155
 Optional<uint64_t>
 BlockFrequencyInfo::getBlockProfileCount(const BasicBlock *BB) const {
   if (!BFI)
----------------
davidxl wrote:
> This does not look like the right place for the adjustment. Maybe provide a wrapper method in ProfileSummaryInfo::getBlockProfileCount ..
> 
> Besides, the adjustment does not work well for Instrumentation based PGO -- the branch weight may be scaled (though since it scales down, it does not matter in reality).
My concern is that if we add ProfileSummaryInfo::getBlockProfileCount, it will get confused with BlockFrequencyInfo::getBlockProfileCount.

Do you mean that we hide this function in BlockFrequencyInfo, and only expose it in ProfileSummaryInfo, and change all callsites to PSI? It will introduce dependency to PSI (though there are not many callsites to this function)


https://reviews.llvm.org/D26370





More information about the llvm-commits mailing list