[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/include/llvm/Transforms/Scalar/SimplifyCFG.h

@@ -17,26 +17,25 @@
 
 #include "llvm/IR/Function.h"
 #include "llvm/IR/PassManager.h"
+#include "llvm/Transforms/Utils/Local.h"
 
 namespace llvm {
 
-/// \brief A pass to simplify and canonicalize the CFG of a function.
+/// A pass to simplify and canonicalize the CFG of a function.
 ///
-/// This pass iteratively simplifies the entire CFG of a function, removing
-/// unnecessary control flows and bringing it into the canonical form expected
-/// by the rest of the mid-level optimizer.
+/// This pass iteratively simplifies the entire CFG of a function. It may change
+/// or remove control flow to put the CFG into a canonical form expected by
+/// other passes of the mid-level optimizer. Depending on the specified options,
+/// it may further optimize control-flow to create non-canonical forms.
 class SimplifyCFGPass : public PassInfoMixin<SimplifyCFGPass> {
-  int BonusInstThreshold;
-  bool LateSimplifyCFG;
+  SimplifyCFGOptions Options;
 
 public:
-  /// \brief Construct a pass with the default thresholds
-  /// and switch optimizations.
+  /// Construct a pass with default options.
   SimplifyCFGPass();
 
-  /// \brief Construct a pass with a specific bonus threshold
-  /// and optional switch optimizations.
-  SimplifyCFGPass(int BonusInstThreshold, bool LateSimplifyCFG);
+  /// Construct a pass with optional optimizations.
+  SimplifyCFGPass(const SimplifyCFGOptions &PassOptions);
 
   /// \brief Run the pass over the function.
   PreservedAnalyses run(Function &F, FunctionAnalysisManager &AM);

llvm/include/llvm/Transforms/Utils/Local.h

@@ -51,6 +51,22 @@ class LazyValueInfo;
 
 template<typename T> class SmallVectorImpl;
 
+/// A set of parameters used to control the transforms in the SimplifyCFG pass.
+/// Options may change depending on the position in the optimization pipeline.
+/// For example, canonical form that includes switches and branches may later be
+/// replaced by lookup tables and selects.
+struct SimplifyCFGOptions {
+  int BonusInstThreshold;
+  bool ConvertSwitchToLookupTable;
+  bool NeedCanonicalLoop;
+
+  SimplifyCFGOptions(int BonusThreshold = 1, bool SwitchToLookup = false,
+                     bool CanonicalLoops = true)
+      : BonusInstThreshold(BonusThreshold),
+        ConvertSwitchToLookupTable(SwitchToLookup),
+        NeedCanonicalLoop(CanonicalLoops) {}
+};
+
 //===----------------------------------------------------------------------===//
 //  Local constant propagation.
 //
@@ -135,17 +151,16 @@ bool TryToSimplifyUncondBranchFromEmptyBlock(BasicBlock *BB);
 /// values, but instcombine orders them so it usually won't matter.
 bool EliminateDuplicatePHINodes(BasicBlock *BB);
 
-/// This function is used to do simplification of a CFG.  For
-/// example, it adjusts branches to branches to eliminate the extra hop, it
-/// eliminates unreachable basic blocks, and does other "peephole" optimization
-/// of the CFG.  It returns true if a modification was made, possibly deleting
-/// the basic block that was pointed to. LoopHeaders is an optional input
-/// parameter, providing the set of loop header that SimplifyCFG should not
-/// eliminate.
+/// This function is used to do simplification of a CFG.  For example, it
+/// adjusts branches to branches to eliminate the extra hop, it eliminates
+/// unreachable basic blocks, and does other peephole optimization of the CFG.
+/// It returns true if a modification was made, possibly deleting the basic
+/// block that was pointed to. LoopHeaders is an optional input parameter
+/// providing the set of loop headers that SimplifyCFG should not eliminate.
 bool SimplifyCFG(BasicBlock *BB, const TargetTransformInfo &TTI,
-                 unsigned BonusInstThreshold, AssumptionCache *AC = nullptr,
-                 SmallPtrSetImpl<BasicBlock *> *LoopHeaders = nullptr,
-                 bool LateSimplifyCFG = false);
+                 AssumptionCache *AC = nullptr,
+                 const SimplifyCFGOptions &Options = {},
+                 SmallPtrSetImpl<BasicBlock *> *LoopHeaders = nullptr);
 
 /// This function is used to flatten a CFG. For example, it uses parallel-and
 /// and parallel-or mode to collapse if-conditions and merge if-regions with

llvm/lib/CodeGen/DwarfEHPrepare.cpp

@@ -172,7 +172,7 @@ size_t DwarfEHPrepare::pruneUnreachableResumes(
       BasicBlock *BB = RI->getParent();
       new UnreachableInst(Ctx, RI);
       RI->eraseFromParent();
-      SimplifyCFG(BB, TTI, 1);
+      SimplifyCFG(BB, TTI);
     }
   }
   Resumes.resize(ResumesLeft);

llvm/lib/Target/AMDGPU/AMDGPUUnifyDivergentExitNodes.cpp

@@ -142,7 +142,7 @@ static BasicBlock *unifyReturnBlockSet(Function &F,
 
   for (BasicBlock *BB : ReturningBlocks) {
     // Cleanup possible branch to unconditional branch to the return.
-    SimplifyCFG(BB, TTI, 2);
+    SimplifyCFG(BB, TTI, nullptr, {2});
   }
 
   return NewRetBlock;

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,28 +174,25 @@ 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);
 
   return true;
 }
 
 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<TargetIRAnalysis>(F);
   auto &AC = AM.getResult<AssumptionAnalysis>(F);
 
-  if (!simplifyFunctionCFG(F, TTI, &AC, BonusInstThreshold, LateSimplifyCFG))
+  if (!simplifyFunctionCFG(F, TTI, &AC, Options))
     return PreservedAnalyses::all();
   PreservedAnalyses PA;
   PA.preserve<GlobalsAA>();
@@ -206,16 +201,17 @@ PreservedAnalyses SimplifyCFGPass::run(Function &F,
 
 namespace {
 struct BaseCFGSimplifyPass : public FunctionPass {
-  unsigned BonusInstThreshold;
   std::function<bool(const Function &)> PredicateFtor;
-  bool LateSimplifyCFG;
+  int BonusInstThreshold;
+  bool ConvertSwitchToLookupTable;
+  bool KeepCanonicalLoops;
 
-  BaseCFGSimplifyPass(int T, bool LateSimplifyCFG,
-                      std::function<bool(const Function &)> Ftor,
-                      char &ID)
+  BaseCFGSimplifyPass(int T, bool ConvertSwitch, bool KeepLoops,
+                      std::function<bool(const 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<AssumptionCacheTracker>().getAssumptionCache(F);
     const TargetTransformInfo &TTI =
         getAnalysis<TargetTransformInfoWrapperPass>().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<bool(const 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<bool(const Function &)> Ftor = nullptr)
-                      : BaseCFGSimplifyPass(T, true, Ftor, ID) {
+                      : BaseCFGSimplifyPass(T, true, false, Ftor, ID) {
     initializeLateCFGSimplifyPassPass(*PassRegistry::getPassRegistry());
   }
 };