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

Easwaran Raman via llvm-commits llvm-commits at lists.llvm.org
Mon Feb 1 15:24:02 PST 2016


eraman added a comment.

I've added two more test cases.


================
Comment at: include/llvm/Analysis/InlineCost.h:44
@@ +43,3 @@
+/// This class mimics block frequency analysis on CGSCC level. Block frequency
+/// info is computed on demand and cached unless they are invalidated.
+class BlockFrequencyAnalysis {
----------------
davidxl wrote:
> suggested comment change: ... on demand, cached (for callees), and incrementally updated (for callers). The caller BFI is invalidated after inlining is done for (the caller).
This class itself is agnostic about callers and callees. All it does is cache the BFI and invalidate them when required - and that's what the comment says.

================
Comment at: lib/Analysis/InlineCost.cpp:621
@@ +620,3 @@
+  BasicBlock *CallBB = Call->getParent();
+  if (HasPGOCounts) {
+    Optional<uint64_t> CallSiteCount = llvm::getBlockCount(CallBB, BFA);
----------------
davidxl wrote:
> This guard is wrong :1) it should check caller entry count, not callee entry count. 2) hasPGOCounts also depends on other conditions.
> 
> In fact, this condition can be removed.
Yes, this check is not needed and I've removed it.


http://reviews.llvm.org/D16381





More information about the llvm-commits mailing list