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

[Chandler Carruth via llvm-commits]

chandlerc added a comment.

Mostly nits...


================
Comment at: lib/Analysis/InlineCost.cpp:1322
@@ -1321,29 +1321,3 @@
 
-INITIALIZE_PASS_BEGIN(InlineCostAnalysis, "inline-cost", "Inline Cost Analysis",
-                      true, true)
-INITIALIZE_PASS_DEPENDENCY(TargetTransformInfoWrapperPass)
-INITIALIZE_PASS_DEPENDENCY(AssumptionCacheTracker)
-INITIALIZE_PASS_END(InlineCostAnalysis, "inline-cost", "Inline Cost Analysis",
-                    true, true)
-
-char InlineCostAnalysis::ID = 0;
-
-InlineCostAnalysis::InlineCostAnalysis() : CallGraphSCCPass(ID) {}
-
-InlineCostAnalysis::~InlineCostAnalysis() {}
-
-void InlineCostAnalysis::getAnalysisUsage(AnalysisUsage &AU) const {
-  AU.setPreservesAll();
-  AU.addRequired<AssumptionCacheTracker>();
-  AU.addRequired<TargetTransformInfoWrapperPass>();
-  CallGraphSCCPass::getAnalysisUsage(AU);
-}
-
-bool InlineCostAnalysis::runOnSCC(CallGraphSCC &SCC) {
-  TTIWP = &getAnalysis<TargetTransformInfoWrapperPass>();
-  ACT = &getAnalysis<AssumptionCacheTracker>();
-  return false;
-}
-
-InlineCost InlineCostAnalysis::getInlineCost(CallSite CS, int Threshold) {
-  return getInlineCost(CS, CS.getCalledFunction(), Threshold);
+namespace llvm {
+InlineCost getInlineCost(CallSite CS, int Threshold, TargetTransformInfo &TTI,
----------------
Rather than putting all of this in the llvm namespace, you can just use the qualified function name "llvm::getInlineCost" in the definition.

================
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);
----------------
Why add this code?

================
Comment at: lib/Transforms/IPO/InlineSimple.cpp:65
@@ -60,1 +64,3 @@
+private:
+  TargetTransformInfoWrapperPass *TTIWP;
 };
----------------
You can actually make TTI the member instead of the 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);
----------------
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.

================
Comment at: lib/Transforms/IPO/Inliner.cpp:477-478
@@ -474,3 +476,4 @@
   CallGraph &CG = getAnalysis<CallGraphWrapperPass>().getCallGraph();
-  AssumptionCacheTracker *ACT = &getAnalysis<AssumptionCacheTracker>();
+  if (!ACT)
+    ACT = &getAnalysis<AssumptionCacheTracker>();
   auto &TLI = getAnalysis<TargetLibraryInfoWrapperPass>().getTLI();
----------------
Same comment as above, just overwrite the pointer each time.


Repository:
  rL LLVM

http://reviews.llvm.org/D15701