[llvm] d97f776 - Revert "[NPM][Inliner] Factor ImportedFunctionStats in the InlineAdvisor"

Mircea Trofin via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 20 11:19:45 PST 2021


Author: Mircea Trofin
Date: 2021-01-20T11:19:34-08:00
New Revision: d97f776be5f8cd3cd446fe73827cd355f6bab4e1

URL: https://github.com/llvm/llvm-project/commit/d97f776be5f8cd3cd446fe73827cd355f6bab4e1
DIFF: https://github.com/llvm/llvm-project/commit/d97f776be5f8cd3cd446fe73827cd355f6bab4e1.diff

LOG: Revert "[NPM][Inliner] Factor ImportedFunctionStats in the InlineAdvisor"

This reverts commit e8aec763a57e211420dfceb2a8dc6b88574924f3.

Added: 
    

Modified: 
    llvm/include/llvm/Analysis/InlineAdvisor.h
    llvm/include/llvm/Analysis/MLInlineAdvisor.h
    llvm/include/llvm/Analysis/ReplayInlineAdvisor.h
    llvm/include/llvm/Transforms/IPO/Inliner.h
    llvm/include/llvm/Transforms/Utils/ImportedFunctionsInliningStatistics.h
    llvm/lib/Analysis/InlineAdvisor.cpp
    llvm/lib/Analysis/MLInlineAdvisor.cpp
    llvm/lib/Analysis/ReplayInlineAdvisor.cpp
    llvm/lib/Transforms/IPO/Inliner.cpp
    llvm/lib/Transforms/IPO/SampleProfile.cpp
    llvm/lib/Transforms/Utils/ImportedFunctionsInliningStatistics.cpp
    llvm/test/Transforms/Inline/inline_stats.ll

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/Analysis/InlineAdvisor.h b/llvm/include/llvm/Analysis/InlineAdvisor.h
index 5946db3103f0..295e677126d6 100644
--- a/llvm/include/llvm/Analysis/InlineAdvisor.h
+++ b/llvm/include/llvm/Analysis/InlineAdvisor.h
@@ -12,7 +12,6 @@
 #include "llvm/Analysis/InlineCost.h"
 #include "llvm/Config/llvm-config.h"
 #include "llvm/IR/PassManager.h"
-#include "llvm/Transforms/Utils/ImportedFunctionsInliningStatistics.h"
 #include <memory>
 #include <unordered_set>
 
@@ -66,7 +65,10 @@ class InlineAdvice {
   /// behavior by implementing the corresponding record*Impl.
   ///
   /// Call after inlining succeeded, and did not result in deleting the callee.
-  void recordInlining();
+  void recordInlining() {
+    markRecorded();
+    recordInliningImpl();
+  }
 
   /// Call after inlining succeeded, and resulted in deleting the callee.
   void recordInliningWithCalleeDeleted();
@@ -112,7 +114,6 @@ class InlineAdvice {
     assert(!Recorded && "Recording should happen exactly once");
     Recorded = true;
   }
-  void recordInlineStatsIfNeeded();
 
   bool Recorded = false;
 };
@@ -140,7 +141,7 @@ class DefaultInlineAdvice : public InlineAdvice {
 class InlineAdvisor {
 public:
   InlineAdvisor(InlineAdvisor &&) = delete;
-  virtual ~InlineAdvisor();
+  virtual ~InlineAdvisor() { freeDeletedFunctions(); }
 
   /// Get an InlineAdvice containing a recommendation on whether to
   /// inline or not. \p CB is assumed to be a direct call. \p FAM is assumed to
@@ -162,14 +163,12 @@ class InlineAdvisor {
   virtual void onPassExit() {}
 
 protected:
-  InlineAdvisor(Module &M, FunctionAnalysisManager &FAM);
+  InlineAdvisor(FunctionAnalysisManager &FAM) : FAM(FAM) {}
   virtual std::unique_ptr<InlineAdvice> getAdviceImpl(CallBase &CB) = 0;
   virtual std::unique_ptr<InlineAdvice> getMandatoryAdvice(CallBase &CB,
                                                            bool Advice);
 
-  Module &M;
   FunctionAnalysisManager &FAM;
-  std::unique_ptr<ImportedFunctionsInliningStatistics> ImportedFunctionsStats;
 
   /// We may want to defer deleting functions to after the inlining for a whole
   /// module has finished. This allows us to reliably use function pointers as
@@ -203,9 +202,8 @@ class InlineAdvisor {
 /// reusable as-is for inliner pass test scenarios, as well as for regular use.
 class DefaultInlineAdvisor : public InlineAdvisor {
 public:
-  DefaultInlineAdvisor(Module &M, FunctionAnalysisManager &FAM,
-                       InlineParams Params)
-      : InlineAdvisor(M, FAM), Params(Params) {}
+  DefaultInlineAdvisor(FunctionAnalysisManager &FAM, InlineParams Params)
+      : InlineAdvisor(FAM), Params(Params) {}
 
 private:
   std::unique_ptr<InlineAdvice> getAdviceImpl(CallBase &CB) override;

diff  --git a/llvm/include/llvm/Analysis/MLInlineAdvisor.h b/llvm/include/llvm/Analysis/MLInlineAdvisor.h
index 54edbb823263..1afccf84ee48 100644
--- a/llvm/include/llvm/Analysis/MLInlineAdvisor.h
+++ b/llvm/include/llvm/Analysis/MLInlineAdvisor.h
@@ -50,6 +50,7 @@ class MLInlineAdvisor : public InlineAdvisor {
   virtual std::unique_ptr<MLInlineAdvice>
   getAdviceFromModel(CallBase &CB, OptimizationRemarkEmitter &ORE);
 
+  Module &M;
   std::unique_ptr<MLModelRunner> ModelRunner;
 
 private:

diff  --git a/llvm/include/llvm/Analysis/ReplayInlineAdvisor.h b/llvm/include/llvm/Analysis/ReplayInlineAdvisor.h
index 9ef572f7ab87..7e2b0957436d 100644
--- a/llvm/include/llvm/Analysis/ReplayInlineAdvisor.h
+++ b/llvm/include/llvm/Analysis/ReplayInlineAdvisor.h
@@ -24,9 +24,8 @@ class OptimizationRemarkEmitter;
 /// previous build to guide current inlining. This is useful for inliner tuning.
 class ReplayInlineAdvisor : public InlineAdvisor {
 public:
-  ReplayInlineAdvisor(Module &M, FunctionAnalysisManager &FAM,
-                      LLVMContext &Context, StringRef RemarksFile,
-                      bool EmitRemarks);
+  ReplayInlineAdvisor(FunctionAnalysisManager &FAM, LLVMContext &Context,
+                      StringRef RemarksFile, bool EmitRemarks);
   std::unique_ptr<InlineAdvice> getAdviceImpl(CallBase &CB) override;
   bool areReplayRemarksLoaded() const { return HasReplayRemarks; }
 

diff  --git a/llvm/include/llvm/Transforms/IPO/Inliner.h b/llvm/include/llvm/Transforms/IPO/Inliner.h
index b6cf747fe888..b6e793a8a380 100644
--- a/llvm/include/llvm/Transforms/IPO/Inliner.h
+++ b/llvm/include/llvm/Transforms/IPO/Inliner.h
@@ -97,6 +97,7 @@ struct LegacyInlinerBase : public CallGraphSCCPass {
 class InlinerPass : public PassInfoMixin<InlinerPass> {
 public:
   InlinerPass(bool OnlyMandatory = false) : OnlyMandatory(OnlyMandatory) {}
+  ~InlinerPass();
   InlinerPass(InlinerPass &&Arg) = default;
 
   PreservedAnalyses run(LazyCallGraph::SCC &C, CGSCCAnalysisManager &AM,
@@ -105,6 +106,7 @@ class InlinerPass : public PassInfoMixin<InlinerPass> {
 private:
   InlineAdvisor &getAdvisor(const ModuleAnalysisManagerCGSCCProxy::Result &MAM,
                             FunctionAnalysisManager &FAM, Module &M);
+  std::unique_ptr<ImportedFunctionsInliningStatistics> ImportedFunctionsStats;
   std::unique_ptr<DefaultInlineAdvisor> OwnedDefaultAdvisor;
   const bool OnlyMandatory;
 };

diff  --git a/llvm/include/llvm/Transforms/Utils/ImportedFunctionsInliningStatistics.h b/llvm/include/llvm/Transforms/Utils/ImportedFunctionsInliningStatistics.h
index d02bcd0e335b..033ea05b77fa 100644
--- a/llvm/include/llvm/Transforms/Utils/ImportedFunctionsInliningStatistics.h
+++ b/llvm/include/llvm/Transforms/Utils/ImportedFunctionsInliningStatistics.h
@@ -101,12 +101,6 @@ class ImportedFunctionsInliningStatistics {
   StringRef ModuleName;
 };
 
-enum class InlinerFunctionImportStatsOpts {
-  No = 0,
-  Basic = 1,
-  Verbose = 2,
-};
-
 } // llvm
 
 #endif // LLVM_TRANSFORMS_UTILS_IMPORTEDFUNCTIONSINLININGSTATISTICS_H

diff  --git a/llvm/lib/Analysis/InlineAdvisor.cpp b/llvm/lib/Analysis/InlineAdvisor.cpp
index 02708482f3df..dd7a57c73a17 100644
--- a/llvm/lib/Analysis/InlineAdvisor.cpp
+++ b/llvm/lib/Analysis/InlineAdvisor.cpp
@@ -48,8 +48,6 @@ static cl::opt<int>
                         cl::desc("Scale to limit the cost of inline deferral"),
                         cl::init(2), cl::Hidden);
 
-extern cl::opt<InlinerFunctionImportStatsOpts> InlinerFunctionImportStats;
-
 void DefaultInlineAdvice::recordUnsuccessfulInliningImpl(
     const InlineResult &Result) {
   using namespace ore;
@@ -132,20 +130,8 @@ void InlineAdvisor::freeDeletedFunctions() {
   DeletedFunctions.clear();
 }
 
-void InlineAdvice::recordInlineStatsIfNeeded() {
-  if (Advisor->ImportedFunctionsStats)
-    Advisor->ImportedFunctionsStats->recordInline(*Caller, *Callee);
-}
-
-void InlineAdvice::recordInlining() {
-  markRecorded();
-  recordInlineStatsIfNeeded();
-  recordInliningImpl();
-}
-
 void InlineAdvice::recordInliningWithCalleeDeleted() {
   markRecorded();
-  recordInlineStatsIfNeeded();
   Advisor->markFunctionAsDeleted(Callee);
   recordInliningWithCalleeDeletedImpl();
 }
@@ -157,7 +143,7 @@ bool InlineAdvisorAnalysis::Result::tryCreate(InlineParams Params,
   auto &FAM = MAM.getResult<FunctionAnalysisManagerModuleProxy>(M).getManager();
   switch (Mode) {
   case InliningAdvisorMode::Default:
-    Advisor.reset(new DefaultInlineAdvisor(M, FAM, Params));
+    Advisor.reset(new DefaultInlineAdvisor(FAM, Params));
     break;
   case InliningAdvisorMode::Development:
 #ifdef LLVM_HAVE_TF_API
@@ -442,25 +428,6 @@ void llvm::emitInlinedInto(OptimizationRemarkEmitter &ORE, DebugLoc DLoc,
   });
 }
 
-InlineAdvisor::InlineAdvisor(Module &M, FunctionAnalysisManager &FAM)
-    : M(M), FAM(FAM) {
-  if (InlinerFunctionImportStats != InlinerFunctionImportStatsOpts::No) {
-    ImportedFunctionsStats =
-        std::make_unique<ImportedFunctionsInliningStatistics>();
-    ImportedFunctionsStats->setModuleInfo(M);
-  }
-}
-
-InlineAdvisor::~InlineAdvisor() {
-  if (ImportedFunctionsStats) {
-    assert(InlinerFunctionImportStats != InlinerFunctionImportStatsOpts::No);
-    ImportedFunctionsStats->dump(InlinerFunctionImportStats ==
-                                 InlinerFunctionImportStatsOpts::Verbose);
-  }
-
-  freeDeletedFunctions();
-}
-
 std::unique_ptr<InlineAdvice> InlineAdvisor::getMandatoryAdvice(CallBase &CB,
                                                                 bool Advice) {
   return std::make_unique<InlineAdvice>(this, CB, getCallerORE(CB), Advice);

diff  --git a/llvm/lib/Analysis/MLInlineAdvisor.cpp b/llvm/lib/Analysis/MLInlineAdvisor.cpp
index 89f4ff427dff..f75b7f45682a 100644
--- a/llvm/lib/Analysis/MLInlineAdvisor.cpp
+++ b/llvm/lib/Analysis/MLInlineAdvisor.cpp
@@ -66,8 +66,8 @@ CallBase *getInlinableCS(Instruction &I) {
 MLInlineAdvisor::MLInlineAdvisor(Module &M, ModuleAnalysisManager &MAM,
                                  std::unique_ptr<MLModelRunner> Runner)
     : InlineAdvisor(
-          M, MAM.getResult<FunctionAnalysisManagerModuleProxy>(M).getManager()),
-      ModelRunner(std::move(Runner)), CG(new CallGraph(M)),
+          MAM.getResult<FunctionAnalysisManagerModuleProxy>(M).getManager()),
+      M(M), ModelRunner(std::move(Runner)), CG(new CallGraph(M)),
       InitialIRSize(getModuleIRSize()), CurrentIRSize(InitialIRSize) {
   assert(ModelRunner);
 

diff  --git a/llvm/lib/Analysis/ReplayInlineAdvisor.cpp b/llvm/lib/Analysis/ReplayInlineAdvisor.cpp
index d6595baeb6d6..c28a1a3aceef 100644
--- a/llvm/lib/Analysis/ReplayInlineAdvisor.cpp
+++ b/llvm/lib/Analysis/ReplayInlineAdvisor.cpp
@@ -22,12 +22,11 @@ using namespace llvm;
 
 #define DEBUG_TYPE "inline-replay"
 
-ReplayInlineAdvisor::ReplayInlineAdvisor(Module &M,
-                                         FunctionAnalysisManager &FAM,
+ReplayInlineAdvisor::ReplayInlineAdvisor(FunctionAnalysisManager &FAM,
                                          LLVMContext &Context,
                                          StringRef RemarksFile,
                                          bool EmitRemarks)
-    : InlineAdvisor(M, FAM), HasReplayRemarks(false), EmitRemarks(EmitRemarks) {
+    : InlineAdvisor(FAM), HasReplayRemarks(false), EmitRemarks(EmitRemarks) {
   auto BufferOrErr = MemoryBuffer::getFileOrSTDIN(RemarksFile);
   std::error_code EC = BufferOrErr.getError();
   if (EC) {

diff  --git a/llvm/lib/Transforms/IPO/Inliner.cpp b/llvm/lib/Transforms/IPO/Inliner.cpp
index 7f11a3aad38a..8be4e93e09da 100644
--- a/llvm/lib/Transforms/IPO/Inliner.cpp
+++ b/llvm/lib/Transforms/IPO/Inliner.cpp
@@ -90,7 +90,24 @@ static cl::opt<bool>
     DisableInlinedAllocaMerging("disable-inlined-alloca-merging",
                                 cl::init(false), cl::Hidden);
 
-extern cl::opt<InlinerFunctionImportStatsOpts> InlinerFunctionImportStats;
+namespace {
+
+enum class InlinerFunctionImportStatsOpts {
+  No = 0,
+  Basic = 1,
+  Verbose = 2,
+};
+
+} // end anonymous namespace
+
+static cl::opt<InlinerFunctionImportStatsOpts> InlinerFunctionImportStats(
+    "inliner-function-import-stats",
+    cl::init(InlinerFunctionImportStatsOpts::No),
+    cl::values(clEnumValN(InlinerFunctionImportStatsOpts::Basic, "basic",
+                          "basic statistics"),
+               clEnumValN(InlinerFunctionImportStatsOpts::Verbose, "verbose",
+                          "printing of statistics for each inlined function")),
+    cl::Hidden, cl::desc("Enable inliner stats for imported functions"));
 
 LegacyInlinerBase::LegacyInlinerBase(char &ID) : CallGraphSCCPass(ID) {}
 
@@ -630,12 +647,17 @@ bool LegacyInlinerBase::removeDeadFunctions(CallGraph &CG,
   return true;
 }
 
+InlinerPass::~InlinerPass() {
+  if (ImportedFunctionsStats) {
+    assert(InlinerFunctionImportStats != InlinerFunctionImportStatsOpts::No);
+    ImportedFunctionsStats->dump(InlinerFunctionImportStats ==
+                                 InlinerFunctionImportStatsOpts::Verbose);
+  }
+}
+
 InlineAdvisor &
 InlinerPass::getAdvisor(const ModuleAnalysisManagerCGSCCProxy::Result &MAM,
                         FunctionAnalysisManager &FAM, Module &M) {
-  if (OwnedDefaultAdvisor)
-    return *OwnedDefaultAdvisor;
-
   auto *IAA = MAM.getCachedResult<InlineAdvisorAnalysis>(M);
   if (!IAA) {
     // It should still be possible to run the inliner as a stand-alone SCC pass,
@@ -647,7 +669,7 @@ InlinerPass::getAdvisor(const ModuleAnalysisManagerCGSCCProxy::Result &MAM,
     // The one we would get from the MAM can be invalidated as a result of the
     // inliner's activity.
     OwnedDefaultAdvisor =
-        std::make_unique<DefaultInlineAdvisor>(M, FAM, getInlineParams());
+        std::make_unique<DefaultInlineAdvisor>(FAM, getInlineParams());
     return *OwnedDefaultAdvisor;
   }
   assert(IAA->getAdvisor() &&
@@ -676,6 +698,13 @@ PreservedAnalyses InlinerPass::run(LazyCallGraph::SCC &InitialC,
 
   auto AdvisorOnExit = make_scope_exit([&] { Advisor.onPassExit(); });
 
+  if (!ImportedFunctionsStats &&
+      InlinerFunctionImportStats != InlinerFunctionImportStatsOpts::No) {
+    ImportedFunctionsStats =
+        std::make_unique<ImportedFunctionsInliningStatistics>();
+    ImportedFunctionsStats->setModuleInfo(M);
+  }
+
   // We use a single common worklist for calls across the entire SCC. We
   // process these in-order and append new calls introduced during inlining to
   // the end.
@@ -850,6 +879,9 @@ PreservedAnalyses InlinerPass::run(LazyCallGraph::SCC &InitialC,
         }
       }
 
+      if (InlinerFunctionImportStats != InlinerFunctionImportStatsOpts::No)
+        ImportedFunctionsStats->recordInline(F, Callee);
+
       // Merge the attributes based on the inlining.
       AttributeFuncs::mergeAttributesForInlining(F, Callee);
 

diff  --git a/llvm/lib/Transforms/IPO/SampleProfile.cpp b/llvm/lib/Transforms/IPO/SampleProfile.cpp
index 9dc7173bf529..ef1ec9ca7b7a 100644
--- a/llvm/lib/Transforms/IPO/SampleProfile.cpp
+++ b/llvm/lib/Transforms/IPO/SampleProfile.cpp
@@ -1962,7 +1962,7 @@ bool SampleProfileLoader::doInitialization(Module &M,
 
   if (FAM && !ProfileInlineReplayFile.empty()) {
     ExternalInlineAdvisor = std::make_unique<ReplayInlineAdvisor>(
-        M, *FAM, Ctx, ProfileInlineReplayFile, /*EmitRemarks=*/false);
+        *FAM, Ctx, ProfileInlineReplayFile, /*EmitRemarks=*/false);
     if (!ExternalInlineAdvisor->areReplayRemarksLoaded())
       ExternalInlineAdvisor.reset();
   }

diff  --git a/llvm/lib/Transforms/Utils/ImportedFunctionsInliningStatistics.cpp b/llvm/lib/Transforms/Utils/ImportedFunctionsInliningStatistics.cpp
index e19a1cd1db98..ea93f99d69e3 100644
--- a/llvm/lib/Transforms/Utils/ImportedFunctionsInliningStatistics.cpp
+++ b/llvm/lib/Transforms/Utils/ImportedFunctionsInliningStatistics.cpp
@@ -13,7 +13,6 @@
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/Module.h"
-#include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/raw_ostream.h"
 #include <algorithm>
@@ -21,15 +20,6 @@
 #include <sstream>
 using namespace llvm;
 
-cl::opt<InlinerFunctionImportStatsOpts> InlinerFunctionImportStats(
-    "inliner-function-import-stats",
-    cl::init(InlinerFunctionImportStatsOpts::No),
-    cl::values(clEnumValN(InlinerFunctionImportStatsOpts::Basic, "basic",
-                          "basic statistics"),
-               clEnumValN(InlinerFunctionImportStatsOpts::Verbose, "verbose",
-                          "printing of statistics for each inlined function")),
-    cl::Hidden, cl::desc("Enable inliner stats for imported functions"));
-
 ImportedFunctionsInliningStatistics::InlineGraphNode &
 ImportedFunctionsInliningStatistics::createInlineGraphNode(const Function &F) {
 

diff  --git a/llvm/test/Transforms/Inline/inline_stats.ll b/llvm/test/Transforms/Inline/inline_stats.ll
index 25933b143ca1..1553da04c7b6 100644
--- a/llvm/test/Transforms/Inline/inline_stats.ll
+++ b/llvm/test/Transforms/Inline/inline_stats.ll
@@ -9,9 +9,6 @@
 ; RUN: opt -S -passes=inliner-wrapper-no-mandatory-first -inliner-function-import-stats=basic < %s 2>&1 | FileCheck %s --check-prefixes=CHECK-BASIC,CHECK
 ; RUN: opt -S -passes=inliner-wrapper-no-mandatory-first -inliner-function-import-stats=verbose < %s 2>&1 | FileCheck %s --check-prefixes="CHECK-VERBOSE",CHECK
 
-; RUN: opt -S -passes=inliner-wrapper -inliner-function-import-stats=basic < %s 2>&1 | FileCheck %s --check-prefixes=CHECK-BASIC,CHECK
-; RUN: opt -S -passes=inliner-wrapper -inliner-function-import-stats=verbose < %s 2>&1 | FileCheck %s --check-prefixes=CHECK-VERBOSE,CHECK
-
 ; CHECK: ------- Dumping inliner stats for [<stdin>] -------
 ; CHECK-BASIC-NOT: -- List of inlined functions:
 ; CHECK-BASIC-NOT: -- Inlined not imported function


        


More information about the llvm-commits mailing list