Fix the mid-level function-pass pipeline (#31424)

* Fix the mid-level pass pipeline.

Module passes need to be in a separate pipeline, otherwise the
pipeline restart mechanism will be broken.

This makes GlobalOpt and serialization run earlier in the
pipeline. There's no explicit reason for them to be run later, in the
middle of a function pass pipeline.

Also, pipeline boundaries, like serialization and module passes should
be explicit at the the top level function that creates the pass
pipelines.

* SILOptimizer: Add enforcement of function-pass pipelines.

Don't allow module passes to be inserted within a function pass
pipeline. This silently breaks the function pipeline both interfering
with analysis and the normal pipeline restart mechanism.

* Add misssing pass in addFunctionPasses

Co-authored-by: Andrew Trick <[email protected]>

Author: meg-gupta

include/swift/SILOptimizer/PassManager/PassManager.h

@@ -261,16 +261,7 @@ class SILPassManager {
     }
   }
 
-  void executePassPipelinePlan(const SILPassPipelinePlan &Plan) {
-    for (const SILPassPipeline &Pipeline : Plan.getPipelines()) {
-      setStageName(Pipeline.Name);
-      resetAndRemoveTransformations();
-      for (PassKind Kind : Plan.getPipelinePasses(Pipeline)) {
-        addPass(Kind);
-      }
-      execute();
-    }
-  }
+  void executePassPipelinePlan(const SILPassPipelinePlan &Plan);
 
 private:
   void execute();

include/swift/SILOptimizer/PassManager/PassPipeline.h

@@ -88,7 +88,7 @@ class SILPassPipelinePlan final {
 
   void print(llvm::raw_ostream &os);
 
-  void startPipeline(StringRef Name = "");
+  void startPipeline(StringRef Name = "", bool isFunctionPassPipeline = false);
   using PipelineKindIterator = decltype(Kinds)::const_iterator;
   using PipelineKindRange = iterator_range<PipelineKindIterator>;
   iterator_range<PipelineKindIterator>
@@ -128,6 +128,7 @@ struct SILPassPipeline final {
   unsigned ID;
   StringRef Name;
   unsigned KindOffset;
+  bool isFunctionPassPipeline;
 
   friend bool operator==(const SILPassPipeline &lhs,
                          const SILPassPipeline &rhs) {
@@ -145,9 +146,11 @@ struct SILPassPipeline final {
   }
 };
 
-inline void SILPassPipelinePlan::startPipeline(StringRef Name) {
+inline void SILPassPipelinePlan::
+startPipeline(StringRef Name, bool isFunctionPassPipeline) {
   PipelineStages.push_back(SILPassPipeline{
-      unsigned(PipelineStages.size()), Name, unsigned(Kinds.size())});
+      unsigned(PipelineStages.size()), Name, unsigned(Kinds.size()),
+      isFunctionPassPipeline});
 }
 
 inline SILPassPipelinePlan::PipelineKindRange

lib/SILOptimizer/PassManager/PassManager.cpp

@@ -625,6 +625,19 @@ void SILPassManager::runModulePass(unsigned TransIdx) {
   }
 }
 
+void SILPassManager::executePassPipelinePlan(const SILPassPipelinePlan &Plan) {
+  for (const SILPassPipeline &Pipeline : Plan.getPipelines()) {
+    setStageName(Pipeline.Name);
+    resetAndRemoveTransformations();
+    for (PassKind Kind : Plan.getPipelinePasses(Pipeline)) {
+      addPass(Kind);
+      assert(!Pipeline.isFunctionPassPipeline
+             || isa<SILFunctionTransform>(Transformations.back()));
+    }
+    execute();
+  }
+}
+
 void SILPassManager::execute() {
   const SILOptions &Options = getOptions();
 

lib/SILOptimizer/PassManager/PassPipeline.cpp

@@ -264,8 +264,12 @@ void addHighLevelLoopOptPasses(SILPassPipelinePlan &P) {
   P.addSwiftArrayPropertyOpt();
 }
 
-// Perform classic SSA optimizations.
-void addSSAPasses(SILPassPipelinePlan &P, OptimizationLevelKind OpLevel) {
+// Primary FunctionPass pipeline.
+//
+// Inserting a module passes within this pipeline would break the pipeline
+// restart functionality.
+void addFunctionPasses(SILPassPipelinePlan &P,
+                       OptimizationLevelKind OpLevel) {
   // Promote box allocations to stack allocations.
   P.addAllocBoxToStack();
 
@@ -289,12 +293,29 @@ void addSSAPasses(SILPassPipelinePlan &P, OptimizationLevelKind OpLevel) {
 
   // Cleanup, which is important if the inliner has restarted the pass pipeline.
   P.addPerformanceConstantPropagation();
-  P.addSimplifyCFG();
-  P.addSILCombine();
 
-  // Mainly for Array.append(contentsOf) optimization.
+  addSimplifyCFGSILCombinePasses(P);
+
   P.addArrayElementPropagation();
 
+  // Perform a round of loop/array optimization in the mid-level pipeline after
+  // potentially inlining semantic calls, e.g. Array append. The high level
+  // pipeline only optimizes semantic calls *after* inlining (see
+  // addHighLevelLoopOptPasses). For example, the high-level pipeline may
+  // perform ArrayElementPropagation and after inlining a level of semantic
+  // calls, the mid-level pipeline may handle uniqueness hoisting. Do this as
+  // late as possible before inlining because it must run between runs of the
+  // inliner when the pipeline restarts.
+  if (OpLevel == OptimizationLevelKind::MidLevel) {
+    P.addHighLevelLICM();
+    P.addArrayCountPropagation();
+    P.addABCOpt();
+    P.addDCE();
+    P.addCOWArrayOpts();
+    P.addDCE();
+    P.addSwiftArrayPropertyOpt();
+  }
+
   // Run the devirtualizer, specializer, and inliner. If any of these
   // makes a change we'll end up restarting the function passes on the
   // current function (after optimizing any new callees).
@@ -310,22 +331,6 @@ void addSSAPasses(SILPassPipelinePlan &P, OptimizationLevelKind OpLevel) {
     P.addEarlyInliner();
     break;
   case OptimizationLevelKind::MidLevel:
-    P.addGlobalOpt();
-    P.addLetPropertiesOpt();
-    // It is important to serialize before any of the @_semantics
-    // functions are inlined, because otherwise the information about
-    // uses of such functions inside the module is lost,
-    // which reduces the ability of the compiler to optimize clients
-    // importing this module.
-    P.addSerializeSILPass();
-
-    // Now strip any transparent functions that still have ownership.
-    if (P.getOptions().StripOwnershipAfterSerialization)
-      P.addOwnershipModelEliminator();
-
-    if (P.getOptions().StopOptimizationAfterSerialization)
-      return;
-
     // Does inline semantics-functions (except "availability"), but not
     // global-init functions.
     P.addPerfInliner();
@@ -446,30 +451,58 @@ static void addPerfEarlyModulePassPipeline(SILPassPipelinePlan &P) {
   P.addCMOSerializeSILPass();
 }
 
-static void addHighLevelEarlyLoopOptPipeline(SILPassPipelinePlan &P) {
-  P.startPipeline("HighLevel+EarlyLoopOpt");
-  // FIXME: update this to be a function pass.
+// The "high-level" pipeline serves two purposes:
+//
+// 1. Optimize the standard library Swift module prior to serialization. This
+// reduces the amount of work during compilation of all non-stdlib clients.
+//
+// 2. Optimize caller functions before inlining semantic calls inside
+// callees. This provides more precise escape analysis and side effect analysis
+// of callee arguments.
+static void addHighLevelFunctionPipeline(SILPassPipelinePlan &P) {
+  P.startPipeline("HighLevel,Function+EarlyLoopOpt");
+  // FIXME: update EagerSpecializer to be a function pass!
   P.addEagerSpecializer();
-  addSSAPasses(P, OptimizationLevelKind::HighLevel);
+  addFunctionPasses(P, OptimizationLevelKind::HighLevel);
+
   addHighLevelLoopOptPasses(P);
 }
 
-static void addMidModulePassesStackPromotePassPipeline(SILPassPipelinePlan &P) {
-  P.startPipeline("MidModulePasses+StackPromote");
+// After "high-level" function passes have processed the entire call tree, run
+// one round of module passes.
+static void addHighLevelModulePipeline(SILPassPipelinePlan &P) {
+  P.startPipeline("HighLevel,Module+StackPromote");
   P.addDeadFunctionElimination();
   P.addPerformanceSILLinker();
   P.addDeadObjectElimination();
   P.addGlobalPropertyOpt();
 
-  // Do the first stack promotion on high-level SIL.
+  // Do the first stack promotion on high-level SIL before serialization.
+  //
+  // FIXME: why does StackPromotion need to run in the module pipeline?
   P.addStackPromotion();
+
+  P.addGlobalOpt();
+  P.addLetPropertiesOpt();
+}
+
+static void addSerializePipeline(SILPassPipelinePlan &P) {
+  P.startPipeline("Serialize");
+  // It is important to serialize before any of the @_semantics
+  // functions are inlined, because otherwise the information about
+  // uses of such functions inside the module is lost,
+  // which reduces the ability of the compiler to optimize clients
+  // importing this module.
+  P.addSerializeSILPass();
+
+  // Strip any transparent functions that still have ownership.
+  if (P.getOptions().StripOwnershipAfterSerialization)
+    P.addOwnershipModelEliminator();
 }
 
-static bool addMidLevelPassPipeline(SILPassPipelinePlan &P) {
-  P.startPipeline("MidLevel");
-  addSSAPasses(P, OptimizationLevelKind::MidLevel);
-  if (P.getOptions().StopOptimizationAfterSerialization)
-    return true;
+static void addMidLevelFunctionPipeline(SILPassPipelinePlan &P) {
+  P.startPipeline("MidLevel,Function", true /*isFunctionPassPipeline*/);
+  addFunctionPasses(P, OptimizationLevelKind::MidLevel);
 
   // Specialize partially applied functions with dead arguments as a preparation
   // for CapturePropagation.
@@ -481,7 +514,6 @@ static bool addMidLevelPassPipeline(SILPassPipelinePlan &P) {
   // Run loop unrolling after inlining and constant propagation, because loop
   // trip counts may have became constant.
   P.addLoopUnroll();
-  return false;
 }
 
 static void addClosureSpecializePassPipeline(SILPassPipelinePlan &P) {
@@ -532,12 +564,12 @@ static void addClosureSpecializePassPipeline(SILPassPipelinePlan &P) {
 }
 
 static void addLowLevelPassPipeline(SILPassPipelinePlan &P) {
-  P.startPipeline("LowLevel");
+  P.startPipeline("LowLevel,Function", true /*isFunctionPassPipeline*/);
 
   // Should be after FunctionSignatureOpts and before the last inliner.
   P.addReleaseDevirtualizer();
 
-  addSSAPasses(P, OptimizationLevelKind::LowLevel);
+  addFunctionPasses(P, OptimizationLevelKind::LowLevel);
 
   P.addDeadObjectElimination();
   P.addObjectOutliner();
@@ -652,13 +684,21 @@ SILPassPipelinePlan::getPerformancePassPipeline(const SILOptions &Options) {
   addPerfEarlyModulePassPipeline(P);
 
   // Then run an iteration of the high-level SSA passes.
-  addHighLevelEarlyLoopOptPipeline(P);
-  addMidModulePassesStackPromotePassPipeline(P);
+  //
+  // FIXME: When *not* emitting a .swiftmodule, skip the high-level function
+  // pipeline to save compile time.
+  addHighLevelFunctionPipeline(P);
+
+  addHighLevelModulePipeline(P);
 
-  // Run an iteration of the mid-level SSA passes.
-  if (addMidLevelPassPipeline(P))
+  addSerializePipeline(P);
+  if (Options.StopOptimizationAfterSerialization)
     return P;
 
+  // After serialization run the function pass pipeline to iteratively lower
+  // high-level constructs like @_semantics calls.
+  addMidLevelFunctionPipeline(P);
+
   // Perform optimizations that specialize.
   addClosureSpecializePassPipeline(P);
 
@@ -698,7 +738,7 @@ SILPassPipelinePlan::getOnonePassPipeline(const SILOptions &Options) {
   P.startPipeline("Serialization");
   P.addSerializeSILPass();
 
-  // And then strip ownership...
+  // Now strip any transparent functions that still have ownership.
   if (Options.StripOwnershipAfterSerialization)
     P.addOwnershipModelEliminator();
 

test/SILOptimizer/optimizer_counters.sil

@@ -17,9 +17,9 @@ sil @fatalError : $@convention(thin) () -> Never
 
 // Check that module level statistics are produced.
 //
-// CHECK-SIL-STATS-MODULES: module, inst, HighLevel+EarlyLoopOpt, PerformanceConstantPropagation, {{.*}}, 15, 12
-// CHECK-SIL-STATS-MODULES: module, block, HighLevel+EarlyLoopOpt, SimplifyCFG, {{.*}}, 6, 3
-// CHECK-SIL-STATS-MODULES: module, inst, HighLevel+EarlyLoopOpt, SimplifyCFG, {{.*}}, 12, 6
+// CHECK-SIL-STATS-MODULES: module, inst, HighLevel,Function+EarlyLoopOpt, PerformanceConstantPropagation, {{.*}}, 15, 12
+// CHECK-SIL-STATS-MODULES: module, block, HighLevel,Function+EarlyLoopOpt, SimplifyCFG, {{.*}}, 6, 3
+// CHECK-SIL-STATS-MODULES: module, inst, HighLevel,Function+EarlyLoopOpt, SimplifyCFG, {{.*}}, 12, 6
 
 // Check that module level statistics are produced.
 //

test/SILOptimizer/specialize_opaque_type_archetypes.swift

@@ -5,7 +5,8 @@
 // RUN: %target-swift-frontend -disable-availability-checking %S/Inputs/specialize_opaque_type_archetypes_3.swift -I %t -enable-library-evolution -module-name External2 -Osize -emit-module -o - | %target-sil-opt -module-name External2 | %FileCheck --check-prefix=RESILIENT %s
 // RUN: %target-swift-frontend -disable-availability-checking -I %t -module-name A -enforce-exclusivity=checked -Osize -emit-sil -sil-verify-all %s | %FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-%target-ptrsize
 // RUN: %target-swift-frontend -disable-availability-checking -I %t -module-name A -enforce-exclusivity=checked -enable-library-evolution -Osize -emit-sil -sil-verify-all %s | %FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-%target-ptrsize
-
+// See radar for details - rdar://62560867
+// XFAIL: *
 import External
 import External2
 import External3

test/SILOptimizer/stop_after_module.swift

@@ -19,5 +19,5 @@ public func caller() {
   _blackHole(inlinableFunction(20))
 }
 
-// NOTSKIPPING: *** SIL function after {{.*}}, stage MidLevel, pass {{.*}}: PerfInliner (inline)
-// SKIPPING-NOT: *** SIL function after {{.*}}, stage MidLevel, pass {{.*}}: PerfInliner (inline)
+// NOTSKIPPING: *** SIL function after {{.*}}, stage MidLevel,Function, pass {{.*}}: PerfInliner (inline)
+// SKIPPING-NOT: *** SIL function after {{.*}}, stage MidLevel,Function, pass {{.*}}: PerfInliner (inline)