[PATCH] D24638: [thinlto] Basic thinlto fdo heuristic

[Teresa Johnson via llvm-commits]

tejohnson added inline comments.

================
Comment at: llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp:291
@@ +290,3 @@
+        Edge.second.Hotness == CalleeInfo::HotnessType::Hot
+            ? Threshold * ImportHotMultiplier
+            : Threshold;
----------------
mehdi_amini wrote:
> This does not seem correct to me. The multiplier should be apply only on the initial "seed" of the call chain, not at every chain.
The multiplier is applied to each call independently based on whether it is marked hot. E.g. if we have:
A -> B (hot call)
and then B has 2 calls:
B -> C1 (hot call)
and
B -> C2 (cold call)

I assume you are referring to A as the "seed" of the call chain? We want to treat the two different calls from B differently, it doesn't matter that the call from A -> B is hot.

================
Comment at: llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp:368
@@ -358,3 +367,3 @@
     DEBUG(dbgs() << "Initalize import for " << GVSummary.first << "\n");
     computeImportForFunction(*FuncSummary, Index, ImportInstrLimit,
                              DefinedGVSummaries, Worklist, ImportList,
----------------
mehdi_amini wrote:
> Here the `ImportInstrLimit` can benefit of the multiplier.
No since the calls from FuncSummary will be treated appropriately if they are hot.


Repository:
  rL LLVM

https://reviews.llvm.org/D24638