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

[Easwaran Raman via llvm-commits]

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 @@
+private:
+  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.


Repository:
  rL LLVM

http://reviews.llvm.org/D15701