[PATCH] D15701: Refactor inline costs analysis by removing the InlineCostAnalysis class

Easwaran Raman via llvm-commits llvm-commits at lists.llvm.org
Tue Dec 22 10:56:32 PST 2015

eraman marked an inline comment as done.

Comment at: lib/Transforms/IPO/InlineSimple.cpp:55-56
@@ -54,2 +54,4 @@
   InlineCost getInlineCost(CallSite CS) override {
-    return ICA->getInlineCost(CS, getInlineThreshold(CS));
+    Function *Callee = CS.getCalledFunction();
+    assert(Callee);
+    TargetTransformInfo &TTI = TTIWP->getTTI(*Callee);
chandlerc wrote:
> Why add this code?
Originally, there was a if(Callee) guard before calling the getTTI, but the Callee can never be NULL here. I agree that assert(Callee) is not very useful and will remove this.

Comment at: lib/Transforms/IPO/InlineSimple.cpp:65
@@ -60,1 +64,3 @@
+  TargetTransformInfoWrapperPass *TTIWP;
chandlerc wrote:
> You can actually make TTI the member instead of the wrapper pass.
I'm confused. Isn't the returned TTI dependent on the function? If I make TTI a member, how do I get the TTI for the callee in getInlineCost without calling getAnalysis on TTI wrapper pass ?

Comment at: lib/Transforms/IPO/InlineSimple.cpp:104-105
@@ -97,3 +103,4 @@
 bool SimpleInliner::runOnSCC(CallGraphSCC &SCC) {
-  ICA = &getAnalysis<InlineCostAnalysis>();
+  if (!TTIWP)
+    TTIWP = &getAnalysis<TargetTransformInfoWrapperPass>();
   return Inliner::runOnSCC(SCC);
chandlerc wrote:
> You want to replace this on every run rather than checking it on each run. Look for other places where we cache a TTI pointer. You should also leave it uninitialized in the constructor so we get an MSan error if we reach code without the run call happening first.
I didn't think about the MSan interaction and it now makes sense to remove the initialization and hence can't do the if (!TTIWP)... . I am curious if there is any other reason to avoid this pattern.



More information about the llvm-commits mailing list