[PATCH] D16381: Infrastructure to allow use of PGO in inliner

[Jun Bum Lim via llvm-commits]

junbuml added inline comments.

================
Comment at: lib/Analysis/InlineCost.cpp:618-619
@@ -598,1 +617,4 @@
   }
+  Instruction *Call = CS.getInstruction();
+  BasicBlock *CallBB = Call->getParent();
+  Optional<uint64_t> CallSiteCount = llvm::getBlockCount(CallBB, BFA);
----------------
Call and CallBB seems to be used only in llvm::getBlockCount(CallBB, BFA);



================
Comment at: lib/Analysis/InlineCost.cpp:1564
@@ +1563,3 @@
+  uint64_t FunctionEntryFreq = BFI->getEntryFreq();
+  uint64_t BBCount = EntryCount.getValue() * BBFreq / FunctionEntryFreq;
+  return BBCount;
----------------
Can we guarantee FunctionEntryFreq is always non-zero if EntryCount is non-zero ?

================
Comment at: lib/Transforms/IPO/InlineSimple.cpp:159
@@ +158,3 @@
+  BlockFrequency OrigBBFreq = CalleeBFI->getBlockFreq(OrigBB);
+  CallerBFI->setBlockFreq(NewBB, (double)(OrigBBFreq.getFrequency()) /
+                                     CalleeEntryFreq * CallSiteFreq);
----------------
Should we have (double) here ? 

================
Comment at: lib/Transforms/IPO/InlineSimple.cpp:160
@@ +159,3 @@
+  CallerBFI->setBlockFreq(NewBB, (double)(OrigBBFreq.getFrequency()) /
+                                     CalleeEntryFreq * CallSiteFreq);
+}
----------------
why not checking CalleeEntryFreq * CallSiteFreq is non-zero.

================
Comment at: lib/Transforms/IPO/InlineSimple.cpp:196
@@ +195,3 @@
+void SimpleInliner::InvalidateFunctionAnalysis(Function *F) {
+  if (F) {
+    BFA->invalidateBlockFrequencyInfo(F);
----------------
No need to have braces.



http://reviews.llvm.org/D16381