[PATCH] D16381: Adjust inlining thresholds based on callsite hotness

[David Li via llvm-commits]

davidxl added a comment.

When the invalidation happens, the updated entry count information gets discarded it seems. This is wrong. The data should be propagated back to the IR. The information can be useful later for decisions such as function layout and optimize for size etc.

Some test case is also needed to test that the entry counts can be properly updated during inlining.


================
Comment at: lib/Transforms/IPO/InlineSimple.cpp:140
@@ +139,3 @@
+/// \brief Update the frequency of a block that is cloned into the caller.
+/// This is invoked when OrigBB from the callee is cloned into NewBB in the
+/// caller.
----------------
\p OrigBB   \p NewBB

================
Comment at: lib/Transforms/IPO/InlineSimple.cpp:155
@@ +154,3 @@
+  uint64_t CalleeEntryFreq = CalleeBFI->getEntryFreq();
+  BlockFrequency Freq = CalleeBFI->getBlockFreq(OrigBB);
+  CallerBFI->setBlockFreq(NewBB, (double)(Freq.getFrequency()) /
----------------
Freq --> OrigBBFreq  with comment it is OrigBB's frequency in callee

================
Comment at: lib/Transforms/IPO/InlineSimple.cpp:160
@@ +159,3 @@
+
+/// \brief Update Callee's entry count after it got inlined at a callsite.
+void SimpleInliner::UpdateEntryCount(BasicBlock *CallBB, Function *Callee) {
----------------
\p Callee ... after it got inlined at callsite in block \p CallBB

================
Comment at: lib/Transforms/IPO/InlineSimple.cpp:175
@@ +174,3 @@
+  uint64_t CallerEntryFreq = CallerBFI->getEntryFreq();
+  uint64_t CallSiteCount = CallerCount.getValue() *
+                           (CallSiteFreq + CallerEntryFreq / 2) /
----------------
computing callsite count belongs to a common helper function -- which can be used elsewhere too.

================
Comment at: lib/Transforms/IPO/InlineSimple.cpp:178
@@ +177,3 @@
+                           CallerEntryFreq;
+  // Since CallSiteCount is an estimate, it could exceed the original callee
+  // count and has to be set to 0.
----------------
It is an estimate because the limitation of frequency propagation algorithm (unable to handle zero BP etc).

It is helpful to add a debug message here to indicate this condition.


http://reviews.llvm.org/D16381