Update PassManager's function worklist for newly added SILFunctions

The PassManager should transform all functions in bottom up order.
This is necessary because when optimizations like inlining looks at the
callee function bodies to compute profitability, the callee functions
should have already undergone optimizations to get better profitability
estimates.

The PassManager builds its function worklist based on bottom up order
on initialization. However, newly created SILFunctions due to
specialization etc, are simply appended to the function worklist. This
can cause us to make bad inlining decisions due to inaccurate
profitability estimates. This change now updates the function worklist such
that, all the callees of the newly added SILFunction are proccessed
before it by the PassManager.

Fixes rdar://52202680

Author: meg-gupta

include/swift/SILOptimizer/Analysis/FunctionOrder.h

@@ -31,7 +31,6 @@ class BottomUpFunctionOrder {
   typedef TinyPtrVector<SILFunction *> SCC;
 
 private:
-  SILModule &M;
   llvm::SmallVector<SCC, 32> TheSCCs;
   llvm::SmallVector<SILFunction *, 32> TheFunctions;
 
@@ -44,24 +43,29 @@ class BottomUpFunctionOrder {
   llvm::SmallSetVector<SILFunction *, 4> DFSStack;
 
 public:
-  BottomUpFunctionOrder(SILModule &M, BasicCalleeAnalysis *BCA)
-      : M(M), BCA(BCA), NextDFSNum(0) {}
+  BottomUpFunctionOrder(BasicCalleeAnalysis *BCA)
+      : BCA(BCA), NextDFSNum(0) {}
+
+  /// DFS on 'F' to compute bottom up order
+  void computeBottomUpOrder(SILFunction *F) {
+     DFS(F);
+  }
+
+  /// DFS on all functions in the module to compute bottom up order
+  void computeBottomUpOrder(SILModule *M) {
+    for (auto &F : *M)
+      DFS(&F);
+  }
 
   /// Get the SCCs in bottom-up order.
   ArrayRef<SCC> getSCCs() {
-    if (!TheSCCs.empty())
-      return TheSCCs;
-
-    FindSCCs(M);
     return TheSCCs;
   }
 
-  /// Get a flattened view of all functions in all the SCCs in
-  /// bottom-up order
-  ArrayRef<SILFunction *> getFunctions() {
+  /// Get a flattened view of all functions in all the SCCs in bottom-up order
+  ArrayRef<SILFunction *> getBottomUpOrder() {
     if (!TheFunctions.empty())
       return TheFunctions;
-
     for (auto SCC : getSCCs())
       for (auto *F : SCC)
         TheFunctions.push_back(F);
@@ -71,7 +75,6 @@ class BottomUpFunctionOrder {
 
 private:
   void DFS(SILFunction *F);
-  void FindSCCs(SILModule &M);
 };
 
 } // end namespace swift

lib/SILOptimizer/Analysis/FunctionOrder.cpp

@@ -74,7 +74,3 @@ void BottomUpFunctionOrder::DFS(SILFunction *Start) {
   }
 }
 
-void BottomUpFunctionOrder::FindSCCs(SILModule &M) {
-  for (auto &F : M)
-    DFS(&F);
-}

lib/SILOptimizer/PassManager/PassManager.cpp

@@ -491,8 +491,9 @@ runFunctionPasses(unsigned FromTransIdx, unsigned ToTransIdx) {
     return;
 
   BasicCalleeAnalysis *BCA = getAnalysis<BasicCalleeAnalysis>();
-  BottomUpFunctionOrder BottomUpOrder(*Mod, BCA);
-  auto BottomUpFunctions = BottomUpOrder.getFunctions();
+  BottomUpFunctionOrder BottomUpOrder(BCA);
+  BottomUpOrder.computeBottomUpOrder(Mod);
+  auto BottomUpFunctions = BottomUpOrder.getBottomUpOrder();
 
   assert(FunctionWorklist.empty() && "Expected empty function worklist!");
 
@@ -545,6 +546,47 @@ runFunctionPasses(unsigned FromTransIdx, unsigned ToTransIdx) {
       ++Entry.PipelineIdx;
     }
     clearRestartPipeline();
+
+    if (TailIdx == (FunctionWorklist.size() - 1))  {
+      // No new functions to process
+      continue;
+    }
+
+    // Compute the bottom up order of the new functions and the callees in it
+    BottomUpFunctionOrder SubBottomUpOrder(BCA);
+    // Initialize BottomUpFunctionOrder with new functions
+    for (auto It = FunctionWorklist.begin() + TailIdx + 1;
+         It != FunctionWorklist.end(); It++) {
+      SubBottomUpOrder.computeBottomUpOrder(It->F);
+    }
+    auto NewFunctionsBottomUp = SubBottomUpOrder.getBottomUpOrder();
+    SmallPtrSet<SILFunction *, 8> NewBottomUpSet(NewFunctionsBottomUp.begin(),
+                                                 NewFunctionsBottomUp.end());
+
+    // Remove all the functions in the new bottom up order from FunctionWorklist
+    llvm::DenseMap<SILFunction *, WorklistEntry> FunctionsToReorder;
+    auto RemoveFn = [&FunctionsToReorder,
+                     &NewBottomUpSet](WorklistEntry Entry) {
+      if (NewBottomUpSet.find(Entry.F) == NewBottomUpSet.end()) {
+        return false;
+      }
+      FunctionsToReorder.insert(std::make_pair(Entry.F, Entry));
+      return true;
+    };
+    std::remove_if(FunctionWorklist.begin(), FunctionWorklist.end(), RemoveFn);
+    FunctionWorklist.erase((FunctionWorklist.begin() + FunctionWorklist.size() -
+                            FunctionsToReorder.size()),
+                           FunctionWorklist.end());
+
+    // Add back the functions in the new bottom up order to the FunctionWorklist
+    for (auto it = NewFunctionsBottomUp.rbegin();
+         it != NewFunctionsBottomUp.rend(); it++) {
+      auto Entry = FunctionsToReorder.find(*it);
+      if (Entry == FunctionsToReorder.end()) {
+        continue;
+      }
+      FunctionWorklist.push_back((*Entry).second);
+    }
   }
 }
 

lib/SILOptimizer/UtilityPasses/FunctionOrderPrinter.cpp

@@ -35,8 +35,8 @@ class FunctionOrderPrinterPass : public SILModuleTransform {
   /// The entry point to the transformation.
   void run() override {
     BCA = getAnalysis<BasicCalleeAnalysis>();
-    auto &M = *getModule();
-    BottomUpFunctionOrder Orderer(M, BCA);
+    BottomUpFunctionOrder Orderer(BCA);
+    Orderer.computeBottomUpOrder(getModule());
 
     llvm::outs() << "Bottom up function order:\n";
     auto SCCs = Orderer.getSCCs();

test/DebugInfo/inlined-generics-basic.swift

@@ -91,9 +91,9 @@ public class C<R> {
 // IR-LABEL: ret void
 
 // IR: ![[BOOL:[0-9]+]] = !DICompositeType({{.*}}name: "Bool"
-// IR: ![[LET_BOOL:[0-9]+]] = !DIDerivedType(tag: DW_TAG_const_type, baseType: ![[BOOL]])
 // IR: ![[INT:[0-9]+]] = !DICompositeType({{.*}}name: "Int"
 // IR: ![[LET_INT:[0-9]+]] = !DIDerivedType(tag: DW_TAG_const_type, baseType: ![[INT]])
+// IR: ![[LET_BOOL:[0-9]+]] = !DIDerivedType(tag: DW_TAG_const_type, baseType: ![[BOOL]])
 // IR: ![[TAU_0_0:[0-9]+]] = {{.*}}DW_TAG_structure_type, name: "$sxD",
 // IR: ![[LET_TAU_0_0:[0-9]+]] = !DIDerivedType(tag: DW_TAG_const_type, baseType: ![[TAU_0_0]])
 // IR: ![[TAU_1_0:[0-9]+]] = {{.*}}DW_TAG_structure_type, name: "$sqd__D",