From fd98ce10c7f709a9d62914fb10f597388e439060 Mon Sep 17 00:00:00 2001 From: Meghana Gupta Date: Wed, 25 Mar 2020 07:22:26 -0700 Subject: [PATCH] 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 --- .../SILOptimizer/Analysis/FunctionOrder.h | 27 ++++++----- lib/SILOptimizer/Analysis/FunctionOrder.cpp | 4 -- lib/SILOptimizer/PassManager/PassManager.cpp | 46 ++++++++++++++++++- .../UtilityPasses/FunctionOrderPrinter.cpp | 4 +- test/DebugInfo/inlined-generics-basic.swift | 2 +- 5 files changed, 62 insertions(+), 21 deletions(-) diff --git a/include/swift/SILOptimizer/Analysis/FunctionOrder.h b/include/swift/SILOptimizer/Analysis/FunctionOrder.h index 6863cee08b4b6..8a443a886cbc7 100644 --- a/include/swift/SILOptimizer/Analysis/FunctionOrder.h +++ b/include/swift/SILOptimizer/Analysis/FunctionOrder.h @@ -31,7 +31,6 @@ class BottomUpFunctionOrder { typedef TinyPtrVector SCC; private: - SILModule &M; llvm::SmallVector TheSCCs; llvm::SmallVector TheFunctions; @@ -44,24 +43,29 @@ class BottomUpFunctionOrder { llvm::SmallSetVector 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 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 getFunctions() { + /// Get a flattened view of all functions in all the SCCs in bottom-up order + ArrayRef 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 diff --git a/lib/SILOptimizer/Analysis/FunctionOrder.cpp b/lib/SILOptimizer/Analysis/FunctionOrder.cpp index 03cc277a0dcc5..e0bf1a17ef177 100644 --- a/lib/SILOptimizer/Analysis/FunctionOrder.cpp +++ b/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); -} diff --git a/lib/SILOptimizer/PassManager/PassManager.cpp b/lib/SILOptimizer/PassManager/PassManager.cpp index 369ad70e03d88..b9f1142591446 100644 --- a/lib/SILOptimizer/PassManager/PassManager.cpp +++ b/lib/SILOptimizer/PassManager/PassManager.cpp @@ -491,8 +491,9 @@ runFunctionPasses(unsigned FromTransIdx, unsigned ToTransIdx) { return; BasicCalleeAnalysis *BCA = getAnalysis(); - 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 NewBottomUpSet(NewFunctionsBottomUp.begin(), + NewFunctionsBottomUp.end()); + + // Remove all the functions in the new bottom up order from FunctionWorklist + llvm::DenseMap 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); + } } } diff --git a/lib/SILOptimizer/UtilityPasses/FunctionOrderPrinter.cpp b/lib/SILOptimizer/UtilityPasses/FunctionOrderPrinter.cpp index 59bd0b43a8fa7..da66869263cac 100644 --- a/lib/SILOptimizer/UtilityPasses/FunctionOrderPrinter.cpp +++ b/lib/SILOptimizer/UtilityPasses/FunctionOrderPrinter.cpp @@ -35,8 +35,8 @@ class FunctionOrderPrinterPass : public SILModuleTransform { /// The entry point to the transformation. void run() override { BCA = getAnalysis(); - auto &M = *getModule(); - BottomUpFunctionOrder Orderer(M, BCA); + BottomUpFunctionOrder Orderer(BCA); + Orderer.computeBottomUpOrder(getModule()); llvm::outs() << "Bottom up function order:\n"; auto SCCs = Orderer.getSCCs(); diff --git a/test/DebugInfo/inlined-generics-basic.swift b/test/DebugInfo/inlined-generics-basic.swift index 93f13e5d05955..5dddba06245a5 100644 --- a/test/DebugInfo/inlined-generics-basic.swift +++ b/test/DebugInfo/inlined-generics-basic.swift @@ -91,9 +91,9 @@ public class C { // 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",