[PATCH] D71733: [NFC][InlineCost] Factor cost modeling out of CallAnalyzer traversal.
Easwaran Raman via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Jan 8 13:46:18 PST 2020
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
More information about the llvm-commits
mailing list