[PATCH] D65060: [LICM] Make Loop ICM profile aware

Wed Jul 24 09:39:22 PDT 2019

davidxl added a comment.

It is also better to introduce a coldness factor parameter 'F', i.e, if the source block's count is less than  1/F of the header count, suppress the hoisting.

================
Comment at: llvm/lib/Transforms/Scalar/LICM.cpp:804
+  BasicBlock *SrcBlock = I.getParent();
+  if (BFI->getBlockFreq(SrcBlock) < BFI->getBlockFreq(DstBlock)) {
+    ORE->emit([&]() {
----------------
wenlei wrote:
> vsk wrote:
> > IIRC BFI returns a 0 frequency if information about a block is missing. If information for either block is missing, shouldn't this fall back to the non-PGO behavior?
> Hmm.. how can we differentiate between missing info vs. the block count is really just 0? My understanding is 1) if profile isn't available in general or for this function, BPI would use static info about the CFG to estimate weights (BranchProbabilityInfo::calculate), thus BFI automatically gets a "synthetic" static profile; 2) if information is missing because we don't have profile for that block, that's likely AutoFDO instead of Instr. PGO, and SampleProfileLoader::propagateWeights would try to infer if possible, but that's more of a profile accuracy issue. 
Without profile data, BFI can easily give you wrong results, so you do need real profile data to guide the decision, otherwise this can not be turned on by default.

To use real profile data, the right interface to use is ProfileSummaryInfo::getProfileCount -- this interface handles samplePGO and instrumentation based PGO properly.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65060