[PATCH] D150649: [FuncSpec] Enable specialization of literal constants.

[Alexandros Lamprineas via Phabricator via llvm-commits]

labrinea marked 2 inline comments as done.
labrinea added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/FunctionSpecialization.cpp:495-507
+    // Check the function entry frequency only once. We sink this code here to
+    // postpone BFI until we know for sure there are Specialization candidates,
+    // otherwise we are adding unnecessary overhead.
+    if (!HasCheckedEntryFreq) {
+      // Reject cold functions (for some definition of 'cold').
+      uint64_t EntryFreq = (GetBFI)(*F).getEntryFreq();
+      if (!ForceSpecialization && EntryFreq < MinEntryFreq)
----------------
ChuanqiXu wrote:
> Can't we move this out of the loop simply?
It's explained in the comments. If we hoist this code we are eagerly asking for the BlockFrequencyAnalysis to run even if no specializations are found. I've checked and moving it regresses compilation times for benchmarks with no specializations.


================
Comment at: llvm/lib/Transforms/IPO/FunctionSpecialization.cpp:527
         Score += getSpecializationBonus(A.Formal, A.Actual, KnownConstants);
+      Score /= SpecCost;
 
----------------
ChuanqiXu wrote:
> What's the reason that we use `/` instead of `-` here?
Because we want the Bonus to be at least `MinScore` times higher than SpecCost. The delta was too aggressive heuristic. A ratio seems more sensible.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150649/new/

https://reviews.llvm.org/D150649