[PATCH] D30062: Estimate speedup due to inlining and use that to adjust threshold.

[David Li via Phabricator via llvm-commits]

davidxl added inline comments.


================
Comment at: lib/Analysis/InlineCost.cpp:89
+/// only when the callsite is relatively hot compared to caller's entry.
+static cl::opt<unsigned> MinBFForSpeedupBonus(
+    "min-freq-for-speedup-bonus", cl::Hidden, cl::init(8), cl::ZeroOrMore,
----------------
Does this mean globally hot callsites may not get the speed up bonus if their relative frequency to caller entry is smaller than 8? while a less hot callsite may get both hot callsite bump and big speedup bump?


================
Comment at: lib/Analysis/InlineCost.cpp:246
                       function_ref<Constant *(ArrayRef<Constant *>)> Evaluate);
+  int getSpeedupBonus(CallSite &CS, int Threshold);
+  bool hasEstimatedSpeedup();
----------------
Document the method.


================
Comment at: lib/Analysis/InlineCost.cpp:247
+  int getSpeedupBonus(CallSite &CS, int Threshold);
+  bool hasEstimatedSpeedup();
 
----------------
hasLargeSpeedup ?


================
Comment at: lib/Analysis/InlineCost.cpp:508
+    if (SVLookup)
+      accumulateSavings();
 
----------------
should this be called inside the lambda?


================
Comment at: lib/Analysis/InlineCost.cpp:541
     SimplifiedValues[&I] = C;
+    if (Lookup)
+      accumulateSavings();
----------------
why does this need to be guarded here?


================
Comment at: lib/Analysis/InlineCost.cpp:989
+    // FIXME: Increase the savings associated with simplifying a callsite.
+    if (SVLookup)
+      accumulateSavings(InlineConstants::InstrCost +
----------------
Why is the guard needed here?


================
Comment at: lib/Analysis/InlineCost.cpp:1089
+    // up to the current callee's weighted cost and savings.
+    accumulateSavings(InlineConstants::InstrCost +
+                      InlineConstants::CallPenalty);
----------------
Perhaps at least add parameter passing to the savings?


https://reviews.llvm.org/D30062