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

[Easwaran Raman via llvm-commits]

eraman marked an inline comment as done.

================
Comment at: include/llvm/Analysis/InlineCost.h:152
@@ -132,1 +151,3 @@
+/// \brief Return estimated count of the block \p BB.
+Optional<uint64_t> getBlockCount(BasicBlock *BB, BlockFrequencyAnalysis *BFA);
 }
----------------
davidxl wrote:
> ok -- ProfileCommon.h will be a better place in the future. For now let's keep it here -- but I think we should have a top level wrapper to get count for callsite here.
Do you want the wrapper in InlineCost.cpp or in ProfileCommon.h (later)? As of now, there are two calls to this and one of them can not pass CS (this is in Inliner.cpp during BFI updates) since the call instruction in CS has been removed.

I agree that getProfileCount(CS) is a useful API, but I think that should be somewhere like ProfileCommon

================
Comment at: test/Transforms/Inline/function-count-update.ll:5
@@ +4,3 @@
+; This tests that the function count of a callee gets correctly updated after it
+; has been inlined into two back-to-back callsites in a single callee. The callee
+; has an alwaysinline attribute and so gets inlined both with the regular inliner
----------------
davidxl wrote:
> in a single callee or in a single BB?
> 
> The test intends to verify that the profile update of the second inlined callsite is correct after block split -- so having the callee identical in the two sites are not essential to the test.
This is a typo - I meant to write "in the same caller".  I have fixed the comments. Yes, for the purpose of this test, we don't need the callee to be the same. I have made the caller call 2 different functions. I also don't think making the calls in a block other than the entry makes any difference, but since it doesn't hurt, I've made that change as well.


http://reviews.llvm.org/D16381