[PATCH] D71733: [NFC][InlineCost] Factor cost modeling out of CallAnalyzer traversal.

[Easwaran Raman via Phabricator via llvm-commits]

eraman added inline comments.


================
Comment at: llvm/lib/Analysis/InlineCost.cpp:473
+        InlineConstants::IndirectCallThreshold;
+    InlineCostCallAnalyzer CA(TTI, GetAssumptionCache, GetBFI, PSI, ORE, F,
+                              Call, IndirectCallParams, false);
----------------
mtrofin wrote:
> eraman wrote:
> > Let's say you are using a different flavor of InlineCostCallAnalyzer (one which  has different implemnetation of the virtual onXYZ methods), you should be instantiating an instance of that class here and call analyze here.
> Maybe, but I'd prefer addressing that problem when the scenario arises, and avoid the added complexity meanwhile, wdyt?
I still think this is better handled now, but if you think otherwise, please add a comment. 


================
Comment at: llvm/lib/Analysis/InlineCost.cpp:2016
   // functions (and hence DT and LI will hopefully be cheap).
   if (Caller->hasMinSize()) {
     DominatorTree DT(F);
----------------
mtrofin wrote:
> eraman wrote:
> > Shouldn't this entire block be moved to InlineCostCallAnalyzer as this isn't performing symbolic evaluation or legality check?
> Possibly, but all that prep work - getting a DominatorTree, LoopInfo, going over loops, making sure we ignore those that aren't executed - would be necessary if we want to account the number of loops somehow. 
Well, all this prep work is for cost computation right? Again, since the goal is to keep only symbolic analysis + legality check  here,  why keep these here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71733