SILPassManager: After a new function is pushed on the stack don't restart the pipeline on the old function.

Instead the pipeline is continued on the old function. This happens when a pass pushes a new, e.g. specialized function, on the function stack.
There is no need to repeat passes which already did run on a function.
It saves a little of compile time and I didn't see any significant impact on code size or performance.
It also simplifies the pass manager.

Author: eeckstein

include/swift/SILOptimizer/PassManager/PassManager.h

@@ -42,8 +42,21 @@ class SILPassManager {
   /// A list of registered analysis.
   llvm::SmallVector<SILAnalysis *, 16> Analysis;
 
+  /// An entry in the FunctionWorkList.
+  struct WorklistEntry {
+    WorklistEntry(SILFunction *F) : F(F) { }
+
+    SILFunction *F;
+
+    /// The current position in the transform-list.
+    unsigned PipelineIdx = 0;
+
+    /// How many times the pipeline was restarted for the function.
+    unsigned NumRestarts = 0;
+  };
+
   /// The worklist of functions to be processed by function passes.
-  std::vector<SILFunction *> FunctionWorklist;
+  std::vector<WorklistEntry> FunctionWorklist;
 
   // Name of the current optimization stage for diagnostics.
   std::string StageName;
@@ -211,9 +224,8 @@ class SILPassManager {
   /// the module.
   void runModulePass(SILModuleTransform *SMT);
 
-  /// Run the passes in \p FuncTransforms on the function \p F.
-  void runPassesOnFunction(PassList FuncTransforms, SILFunction *F,
-                           bool runToCompletion);
+  /// Run the pass \p SFT on the function \p F.
+  void runPassOnFunction(SILFunctionTransform *SFT, SILFunction *F);
 
   /// Run the passes in \p FuncTransforms. Return true
   /// if the pass manager requested to stop the execution

lib/SILOptimizer/PassManager/PassManager.cpp

@@ -49,10 +49,6 @@ llvm::cl::opt<unsigned> SILNumOptPassesToRun(
     "sil-opt-pass-count", llvm::cl::init(UINT_MAX),
     llvm::cl::desc("Stop optimizing after <N> optimization passes"));
 
-llvm::cl::opt<unsigned> SILFunctionPassPipelineLimit("sil-pipeline-limit",
-                                                     llvm::cl::init(10),
-                                                     llvm::cl::desc(""));
-
 llvm::cl::opt<std::string> SILBreakOnFun(
     "sil-break-on-function", llvm::cl::init(""),
     llvm::cl::desc(
@@ -274,110 +270,92 @@ static bool breakBeforeRunning(StringRef fnName, StringRef passName) {
   return fnName == SILBreakOnFun && passName == SILBreakOnPass;
 }
 
-void SILPassManager::runPassesOnFunction(PassList FuncTransforms,
-                                         SILFunction *F,
-                                         bool runToCompletion) {
-
-  const SILOptions &Options = getOptions();
-
-  CompletedPasses &completedPasses = CompletedPassesMap[F];
+void SILPassManager::runPassOnFunction(SILFunctionTransform *SFT,
+                                       SILFunction *F) {
 
   assert(analysesUnlocked() && "Expected all analyses to be unlocked!");
 
-  for (auto SFT : FuncTransforms) {
-    PrettyStackTraceSILFunctionTransform X(SFT, NumPassesRun);
-    DebugPrintEnabler DebugPrint(NumPassesRun);
-
-    SFT->injectPassManager(this);
-    SFT->injectFunction(F);
-
-    // If nothing changed since the last run of this pass, we can skip this
-    // pass.
-    if (completedPasses.test((size_t)SFT->getPassKind()) &&
-        !SILDisableSkippingPasses) {
-      if (SILPrintPassName)
-        llvm::dbgs() << "(Skip) Stage: " << StageName
-                     << " Pass: " << SFT->getName()
-                     << ", Function: " << F->getName() << "\n";
-      continue;
-    }
-
-    if (isDisabled(SFT)) {
-      if (SILPrintPassName)
-        llvm::dbgs() << "(Disabled) Stage: " << StageName
-                     << " Pass: " << SFT->getName()
-                     << ", Function: " << F->getName() << "\n";
-      continue;
-    }
+  PrettyStackTraceSILFunctionTransform X(SFT, NumPassesRun);
+  DebugPrintEnabler DebugPrint(NumPassesRun);
 
-    CurrentPassHasInvalidated = false;
+  SFT->injectPassManager(this);
+  SFT->injectFunction(F);
 
+  // If nothing changed since the last run of this pass, we can skip this
+  // pass.
+  CompletedPasses &completedPasses = CompletedPassesMap[F];
+  if (completedPasses.test((size_t)SFT->getPassKind()) &&
+      !SILDisableSkippingPasses) {
     if (SILPrintPassName)
-      llvm::dbgs() << "#" << NumPassesRun << " Stage: " << StageName
+      llvm::dbgs() << "  (Skip) Stage: " << StageName
                    << " Pass: " << SFT->getName()
                    << ", Function: " << F->getName() << "\n";
+    return;
+  }
 
-    if (doPrintBefore(SFT, F)) {
-      llvm::dbgs() << "*** SIL function before " << StageName << " "
-                   << SFT->getName() << " (" << NumOptimizationIterations
-                   << ") ***\n";
-      F->dump(Options.EmitVerboseSIL);
-    }
+  if (isDisabled(SFT)) {
+    if (SILPrintPassName)
+      llvm::dbgs() << "  (Disabled) Stage: " << StageName
+                   << " Pass: " << SFT->getName()
+                   << ", Function: " << F->getName() << "\n";
+    return;
+  }
 
-    llvm::sys::TimeValue StartTime = llvm::sys::TimeValue::now();
-    Mod->registerDeleteNotificationHandler(SFT);
-    if (breakBeforeRunning(F->getName(), SFT->getName()))
-      LLVM_BUILTIN_DEBUGTRAP;
-    SFT->run();
-    assert(analysesUnlocked() && "Expected all analyses to be unlocked!");
-    Mod->removeDeleteNotificationHandler(SFT);
-
-    // Did running the transform result in new functions being added
-    // to the top of our worklist?
-    bool newFunctionsAdded = (F != FunctionWorklist.back());
-
-    if (SILPrintPassTime) {
-      auto Delta =
-          llvm::sys::TimeValue::now().nanoseconds() - StartTime.nanoseconds();
-      llvm::dbgs() << Delta << " (" << SFT->getName() << "," << F->getName()
-                   << ")\n";
-    }
+  CurrentPassHasInvalidated = false;
 
-    // If this pass invalidated anything, print and verify.
-    if (doPrintAfter(SFT, F, CurrentPassHasInvalidated && SILPrintAll)) {
-      llvm::dbgs() << "*** SIL function after " << StageName << " "
-                   << SFT->getName() << " (" << NumOptimizationIterations
-                   << ") ***\n";
-      F->dump(Options.EmitVerboseSIL);
-    }
+  if (SILPrintPassName)
+    llvm::dbgs() << "  #" << NumPassesRun << " Stage: " << StageName
+                 << " Pass: " << SFT->getName()
+                 << ", Function: " << F->getName() << "\n";
 
-    // Remember if this pass didn't change anything.
-    if (!CurrentPassHasInvalidated)
-      completedPasses.set((size_t)SFT->getPassKind());
+  if (doPrintBefore(SFT, F)) {
+    llvm::dbgs() << "*** SIL function before " << StageName << " "
+                 << SFT->getName() << " (" << NumOptimizationIterations
+                 << ") ***\n";
+    F->dump(getOptions().EmitVerboseSIL);
+  }
 
-    if (Options.VerifyAll &&
-        (CurrentPassHasInvalidated || SILVerifyWithoutInvalidation)) {
-      F->verify();
-      verifyAnalyses(F);
-    }
+  llvm::sys::TimeValue StartTime = llvm::sys::TimeValue::now();
+  Mod->registerDeleteNotificationHandler(SFT);
+  if (breakBeforeRunning(F->getName(), SFT->getName()))
+    LLVM_BUILTIN_DEBUGTRAP;
+  SFT->run();
+  assert(analysesUnlocked() && "Expected all analyses to be unlocked!");
+  Mod->removeDeleteNotificationHandler(SFT);
 
-    ++NumPassesRun;
+  if (SILPrintPassTime) {
+    auto Delta =
+        llvm::sys::TimeValue::now().nanoseconds() - StartTime.nanoseconds();
+    llvm::dbgs() << Delta << " (" << SFT->getName() << "," << F->getName()
+                 << ")\n";
+  }
 
-    if (!continueTransforming())
-      return;
+  // If this pass invalidated anything, print and verify.
+  if (doPrintAfter(SFT, F, CurrentPassHasInvalidated && SILPrintAll)) {
+    llvm::dbgs() << "*** SIL function after " << StageName << " "
+                 << SFT->getName() << " (" << NumOptimizationIterations
+                 << ") ***\n";
+    F->dump(getOptions().EmitVerboseSIL);
+  }
 
-    if (runToCompletion)
-      continue;
+  // Remember if this pass didn't change anything.
+  if (!CurrentPassHasInvalidated)
+    completedPasses.set((size_t)SFT->getPassKind());
 
-    // If running the transform resulted in new functions on the top
-    // of the worklist, we'll return so that we can begin processing
-    // those new functions.
-    if (shouldRestartPipeline() || newFunctionsAdded)
-      return;
+  if (getOptions().VerifyAll &&
+      (CurrentPassHasInvalidated || SILVerifyWithoutInvalidation)) {
+    F->verify();
+    verifyAnalyses(F);
   }
+
+  ++NumPassesRun;
 }
 
 void SILPassManager::runFunctionPasses(PassList FuncTransforms) {
+
+  if (FuncTransforms.empty())
+    return;
+
   BasicCalleeAnalysis *BCA = getAnalysis<BasicCalleeAnalysis>();
   BottomUpFunctionOrder BottomUpOrder(*Mod, BCA);
   auto BottomUpFunctions = BottomUpOrder.getFunctions();
@@ -395,82 +373,41 @@ void SILPassManager::runFunctionPasses(PassList FuncTransforms) {
       FunctionWorklist.push_back(*I);
   }
 
-  // Used to track how many times a given function has been
-  // (partially) optimized by the function pass pipeline in this
-  // invocation.
-  llvm::DenseMap<SILFunction *, unsigned> CountOptimized;
-
-  // Count of how many iterations we've had since any function was
-  // popped off the function worklist. This is used to ensure progress
-  // and eliminate the chance of going into an infinite loop in cases
-  // where (for example) we have recursive type-based specialization
+  // The maximum number of times the pass pipeline can be restarted for a
+  // function. This is used to ensure we are not going into an infinite loop in
+  // cases where (for example) we have recursive type-based specialization
   // happening.
-  unsigned IterationsWithoutProgress = 0;
+  const unsigned MaxNumRestarts = 20;
 
-  // The maximum number of functions we'll optimize without popping
-  // any off the worklist. This is expected to non-zero.
-  const unsigned MaxIterationsWithoutProgress = 20;
+  if (SILPrintPassName)
+    llvm::dbgs() << "Start function passes at stage: " << StageName << "\n";
 
-  // Pop functions off the worklist, and run all function transforms
-  // on each of them.
+  // Run all transforms for all functions, starting at the tail of the worklist.
   while (!FunctionWorklist.empty() && continueTransforming()) {
-    auto *F = FunctionWorklist.back();
+    unsigned TailIdx = FunctionWorklist.size() - 1;
+    unsigned PipelineIdx = FunctionWorklist[TailIdx].PipelineIdx;
+    SILFunction *F = FunctionWorklist[TailIdx].F;
 
-    // If we've done many iterations without progress, pop the current
-    // function and any other function we've run any optimizations on
-    // on from the stack and then continue.
-    if (IterationsWithoutProgress == (MaxIterationsWithoutProgress - 1)) {
-      // Pop the current (potentially not-yet-optimized) function off.
+    if (PipelineIdx >= FuncTransforms.size()) {
+      // All passes did already run for the function. Pop it off the worklist.
       FunctionWorklist.pop_back();
-      IterationsWithoutProgress = 0;
-
-      // Pop any remaining functions that have been optimized (at
-      // least through some portion of the pipeline).
-      while (!FunctionWorklist.empty() &&
-             CountOptimized[FunctionWorklist.back()] > 0)
-        FunctionWorklist.pop_back();
-
       continue;
     }
-
-    if (CountOptimized[F] > SILFunctionPassPipelineLimit) {
-      DEBUG(llvm::dbgs() << "*** Hit limit optimizing: " << F->getName()
-                         << '\n');
-      FunctionWorklist.pop_back();
-      IterationsWithoutProgress = 0;
-      continue;
-    }
-
-    assert(
-        !shouldRestartPipeline() &&
+    assert(!shouldRestartPipeline() &&
         "Did not expect function pipeline set up to restart from beginning!");
 
-    assert(CountOptimized[F] <= SILFunctionPassPipelineLimit &&
-           "Function optimization count exceeds limit!");
-    auto runToCompletion = CountOptimized[F] == SILFunctionPassPipelineLimit;
-
-    runPassesOnFunction(FuncTransforms, F, runToCompletion);
-    ++CountOptimized[F];
-    ++IterationsWithoutProgress;
-
-    if (runToCompletion) {
-      FunctionWorklist.pop_back();
-      IterationsWithoutProgress = 0;
-      clearRestartPipeline();
-      continue;
-    }
-
+    runPassOnFunction(FuncTransforms[PipelineIdx], F);
 
-    // If running the function transforms did not result in new
-    // functions being added to the top of the worklist, then we're
-    // done with this function and can pop it off and continue.
-    // Otherwise, we'll return to this function and reoptimize after
-    // processing the new functions that were added.
-    if (F == FunctionWorklist.back() && !shouldRestartPipeline()) {
-      FunctionWorklist.pop_back();
-      IterationsWithoutProgress = 0;
+    // Note: Don't get entry reference prior to runPassOnFunction().
+    // A pass can push a new function to the worklist which may cause a
+    // reallocation of the buffer and that would invalidate the reference.
+    WorklistEntry &Entry = FunctionWorklist[TailIdx];
+    if (shouldRestartPipeline() && Entry.NumRestarts < MaxNumRestarts) {
+      ++Entry.NumRestarts;
+      Entry.PipelineIdx = 0;
+    } else {
+      ++Entry.PipelineIdx;
     }
-
     clearRestartPipeline();
   }
 }

lib/SILOptimizer/Transforms/FunctionSignatureOpts.cpp

@@ -869,6 +869,13 @@ class FunctionSignatureOpts : public SILFunctionTransform {
       // Make sure the PM knows about this function. This will also help us
       // with self-recursion.
       notifyPassManagerOfFunction(FST.getOptimizedFunction());
+
+      // We have to restart the pipeline for this thunk in order to run the
+      // inliner (and other opts) again. This is important if the new
+      // specialized function (which is called from this thunk) is
+      // function-signature-optimized again and also becomes an
+      // always-inline-thunk.
+      restartPassPipeline();
     }
   }
 

test/SILOptimizer/inline_devirtualize_specialize.sil

@@ -1,4 +1,4 @@
-// RUN: %target-sil-opt -enable-sil-verify-all %s -inline -devirtualizer -generic-specializer -dce | %FileCheck %s
+// RUN: %target-sil-opt -enable-sil-verify-all %s -devirtualizer -generic-specializer -inline -dce | %FileCheck %s
 
 sil_stage canonical