From 0f9b4773c1c68886e63161cd78f4d576db632eb8 Mon Sep 17 00:00:00 2001 From: Sanjay Patel Date: Wed, 27 Sep 2017 14:54:16 +0000 Subject: [SimplifyCFG] add a struct to house optional folds (PR34603) This was intended to be no-functional-change, but it's not - there's a test diff. So I thought I should stop here and post it as-is to see if this looks like what was expected based on the discussion in PR34603: https://bugs.llvm.org/show_bug.cgi?id=34603 Notes: 1. The test improvement occurs because the existing 'LateSimplifyCFG' marker is not carried through the recursive calls to 'SimplifyCFG()->SimplifyCFGOpt().run()->SimplifyCFG()'. The parameter isn't passed down, so we pick up the default value from the function signature after the first level. I assumed that was a bug, so I've passed 'Options' down in all of the 'SimplifyCFG' calls. 2. I split 'LateSimplifyCFG' into 2 bits: ConvertSwitchToLookupTable and KeepCanonicalLoops. This would theoretically allow us to differentiate the transforms controlled by those params independently. 3. We could stash the optional AssumptionCache pointer and 'LoopHeaders' pointer in the struct too. I just stopped here to minimize the diffs. 4. Similarly, I stopped short of messing with the pass manager layer. I have another question that could wait for the follow-up: why is the new pass manager creating the pass with LateSimplifyCFG set to true no matter where in the pipeline it's creating SimplifyCFG passes? // Create an early function pass manager to cleanup the output of the // frontend. EarlyFPM.addPass(SimplifyCFGPass()); --> /// \brief Construct a pass with the default thresholds /// and switch optimizations. SimplifyCFGPass::SimplifyCFGPass() : BonusInstThreshold(UserBonusInstThreshold), LateSimplifyCFG(true) {} <-- switches get converted to lookup tables and loops may not be in canonical form If this is unintended, then it's possible that the current behavior of dropping the 'LateSimplifyCFG' setting via recursion was masking this bug. Differential Revision: https://reviews.llvm.org/D38138 llvm-svn: 314308 --- llvm/lib/Transforms/Scalar/SimplifyCFGPass.cpp | 48 ++++++++++++-------------- 1 file changed, 23 insertions(+), 25 deletions(-) (limited to 'llvm/lib/Transforms/Scalar/SimplifyCFGPass.cpp') diff --git a/llvm/lib/Transforms/Scalar/SimplifyCFGPass.cpp b/llvm/lib/Transforms/Scalar/SimplifyCFGPass.cpp index 8754c714c5b..3ef119ec05d 100644 --- a/llvm/lib/Transforms/Scalar/SimplifyCFGPass.cpp +++ b/llvm/lib/Transforms/Scalar/SimplifyCFGPass.cpp @@ -130,8 +130,7 @@ static bool mergeEmptyReturnBlocks(Function &F) { /// iterating until no more changes are made. static bool iterativelySimplifyCFG(Function &F, const TargetTransformInfo &TTI, AssumptionCache *AC, - unsigned BonusInstThreshold, - bool LateSimplifyCFG) { + const SimplifyCFGOptions &Options) { bool Changed = false; bool LocalChange = true; @@ -146,7 +145,7 @@ static bool iterativelySimplifyCFG(Function &F, const TargetTransformInfo &TTI, // Loop over all of the basic blocks and remove them if they are unneeded. for (Function::iterator BBIt = F.begin(); BBIt != F.end(); ) { - if (SimplifyCFG(&*BBIt++, TTI, BonusInstThreshold, AC, &LoopHeaders, LateSimplifyCFG)) { + if (SimplifyCFG(&*BBIt++, TTI, AC, Options, &LoopHeaders)) { LocalChange = true; ++NumSimpl; } @@ -157,12 +156,11 @@ static bool iterativelySimplifyCFG(Function &F, const TargetTransformInfo &TTI, } static bool simplifyFunctionCFG(Function &F, const TargetTransformInfo &TTI, - AssumptionCache *AC, int BonusInstThreshold, - bool LateSimplifyCFG) { + AssumptionCache *AC, + const SimplifyCFGOptions &Options) { bool EverChanged = removeUnreachableBlocks(F); EverChanged |= mergeEmptyReturnBlocks(F); - EverChanged |= iterativelySimplifyCFG(F, TTI, AC, BonusInstThreshold, - LateSimplifyCFG); + EverChanged |= iterativelySimplifyCFG(F, TTI, AC, Options); // If neither pass changed anything, we're done. if (!EverChanged) return false; @@ -176,8 +174,7 @@ static bool simplifyFunctionCFG(Function &F, const TargetTransformInfo &TTI, return true; do { - EverChanged = iterativelySimplifyCFG(F, TTI, AC, BonusInstThreshold, - LateSimplifyCFG); + EverChanged = iterativelySimplifyCFG(F, TTI, AC, Options); EverChanged |= removeUnreachableBlocks(F); } while (EverChanged); @@ -185,19 +182,17 @@ static bool simplifyFunctionCFG(Function &F, const TargetTransformInfo &TTI, } SimplifyCFGPass::SimplifyCFGPass() - : BonusInstThreshold(UserBonusInstThreshold), - LateSimplifyCFG(true) {} + : Options(UserBonusInstThreshold, true, false) {} -SimplifyCFGPass::SimplifyCFGPass(int BonusInstThreshold, bool LateSimplifyCFG) - : BonusInstThreshold(BonusInstThreshold), - LateSimplifyCFG(LateSimplifyCFG) {} +SimplifyCFGPass::SimplifyCFGPass(const SimplifyCFGOptions &PassOptions) + : Options(PassOptions) {} PreservedAnalyses SimplifyCFGPass::run(Function &F, FunctionAnalysisManager &AM) { auto &TTI = AM.getResult(F); auto &AC = AM.getResult(F); - if (!simplifyFunctionCFG(F, TTI, &AC, BonusInstThreshold, LateSimplifyCFG)) + if (!simplifyFunctionCFG(F, TTI, &AC, Options)) return PreservedAnalyses::all(); PreservedAnalyses PA; PA.preserve(); @@ -206,16 +201,17 @@ PreservedAnalyses SimplifyCFGPass::run(Function &F, namespace { struct BaseCFGSimplifyPass : public FunctionPass { - unsigned BonusInstThreshold; std::function PredicateFtor; - bool LateSimplifyCFG; + int BonusInstThreshold; + bool ConvertSwitchToLookupTable; + bool KeepCanonicalLoops; - BaseCFGSimplifyPass(int T, bool LateSimplifyCFG, - std::function Ftor, - char &ID) + BaseCFGSimplifyPass(int T, bool ConvertSwitch, bool KeepLoops, + std::function Ftor, char &ID) : FunctionPass(ID), PredicateFtor(std::move(Ftor)), - LateSimplifyCFG(LateSimplifyCFG) { - BonusInstThreshold = (T == -1) ? UserBonusInstThreshold : unsigned(T); + ConvertSwitchToLookupTable(ConvertSwitch), + KeepCanonicalLoops(KeepLoops) { + BonusInstThreshold = (T == -1) ? UserBonusInstThreshold : T; } bool runOnFunction(Function &F) override { if (skipFunction(F) || (PredicateFtor && !PredicateFtor(F))) @@ -225,7 +221,9 @@ struct BaseCFGSimplifyPass : public FunctionPass { &getAnalysis().getAssumptionCache(F); const TargetTransformInfo &TTI = getAnalysis().getTTI(F); - return simplifyFunctionCFG(F, TTI, AC, BonusInstThreshold, LateSimplifyCFG); + return simplifyFunctionCFG( + F, TTI, AC, + {BonusInstThreshold, ConvertSwitchToLookupTable, KeepCanonicalLoops}); } void getAnalysisUsage(AnalysisUsage &AU) const override { @@ -240,7 +238,7 @@ struct CFGSimplifyPass : public BaseCFGSimplifyPass { CFGSimplifyPass(int T = -1, std::function Ftor = nullptr) - : BaseCFGSimplifyPass(T, false, Ftor, ID) { + : BaseCFGSimplifyPass(T, false, true, Ftor, ID) { initializeCFGSimplifyPassPass(*PassRegistry::getPassRegistry()); } }; @@ -250,7 +248,7 @@ struct LateCFGSimplifyPass : public BaseCFGSimplifyPass { LateCFGSimplifyPass(int T = -1, std::function Ftor = nullptr) - : BaseCFGSimplifyPass(T, true, Ftor, ID) { + : BaseCFGSimplifyPass(T, true, false, Ftor, ID) { initializeLateCFGSimplifyPassPass(*PassRegistry::getPassRegistry()); } }; -- cgit v1.2.3