[PATCH] D150375: [FuncSpec] Replace LoopInfo with BlockFrequencyInfo.

[Mingming Liu via Phabricator via llvm-commits]

mingmingl added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/FunctionSpecialization.cpp:600-606
+  uint64_t Weight = BFI.getBlockFreq(I->getParent()).getFrequency() /
+                    BFI.getEntryFreq();
+  if (!Weight)
+    return 0;
 
-  // Increase the cost if it is inside the loop.
-  unsigned LoopDepth = LI.getLoopDepth(I->getParent());
-  Bonus *= std::pow((double)AvgLoopIters, LoopDepth);
+  Cost Bonus = Weight *
+      TTI.getInstructionCost(U, TargetTransformInfo::TCK_SizeAndLatency);
----------------
(Hi! I notice this and was trying to get some generated function specialized so would like to share some thoughts :)  )

Function spec cost is calculated as ` Metrics.NumInsts * InlineConstants::getInstrCost()` (i.e., without TTI per instruction cost).
I wonder if it would make more sense to make per-instruction cost calculation consistent.

Also it seems `Weight` could give a big bonus for functions in a hot inner-loop with PGO, wonder how it affects code size in PGO case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150375