[SimplifyCFG] use pass options and remove the latesimplifycfg pass

[spatel]

Summary

This is no-functional-change-intended, but there's enough churn here that I thought it would be best to do this as a preliminary patch for D38566.

This is repackaging the functionality of D30333 (defer switch-to-lookup-tables) and D35411 (defer folding unconditional branches) with pass parameters rather than a named "latesimplifycfg" pass. Now that we have individual options to control the functionality, we could decouple when these fire (but that's an independent patch if desired). The next planned step would be to add another option bit to disable the sinking transform mentioned in D38566.

As mentioned in D38138, I don't know if it's intentional or oversight that we don't have a "latesimplifycfg" equivalent invocation with the new pass manager pipeline. To make that explicit, I removed the default constructor used by that PM, so we'd be aware that we're always creating an "early" (ie, not fully functional) version of the simplifycfg pass.

[spatel]

Ping.

[spatel]

My description of the new pass manager behavior was wrong and inverted. It doesn't always create "-earlysimplifycfg" equivalent invocations. It always creates "-latesimplifycfg" equivalent invocations. I need to correct this patch to preserve that behavior (and add tests so it isn't accidentally broken).

It's also likely that we'll have another options bit in the near-term to correct the mistakes pointed out in D39011.

[spatel]

Patch updated:

1. We have another option intended to limit simplifycfg around switches early in the pipeline with rL316298.
2. Fixed the new pass manager code to continue to create latesimplifycfg-equivalent passes because this patch is NFCI. I created a phase ordering test for both PMs (it's based on the example from D35411) to show the bug and added it with rL316351.

[hfinkel]

Generally, I think this is the right thing to do. I don't like the API, however. We have SimplifyCFGOptions, which is good, but here essentially all users pass something like {1, false, false, true}, which defeats the point because I'll never remember which boolean is which. Maybe we could use a "builder API" for this, something like

SimplifyCFGOptions().enableForwardSwitchCondToPhi().enableConvertSwitchToLookupTable().disableNeedCanonicalLoop()

would be better (so that we can have names instead of just booleans and numbers)?

include/llvm/Transforms/Scalar/SimplifyCFG.h

35 ↗

(On Diff #119899)

This seems like overkill. We do have a current set of defaults (whatever we use for canonicalization), and defaulting to canonicalization seems reasonable to me. We can change the defaults if we change what our canonical form is.

[spatel]

Patch updated:

1. Keep default SimplifyCFGPass constructor and use it where appropriate (reminder: this patch is NFCI, so I preserved the non-default instantiations in the new PM with one FIXME comment).
2. Add methods to SimplifyCFGOptions to allow builder constructor pattern which provide easier-to-read instantiations of SimplifyCFGPass (please make sure I did that correctly because I've never tried that before!).

[bmakam]

include/llvm/Transforms/Scalar.h

263 ↗

(On Diff #120589)

We still pass boolean and values to createCFGSimplificationPass(1, false, false, true) Don't we need builder API here as well?

lib/Transforms/IPO/PassManagerBuilder.cpp

630 ↗

(On Diff #120589)

We still pass booleans and values here. See my comment above for builder API.

[spatel]

include/llvm/Transforms/Scalar.h

263 ↗

(On Diff #120589)

This is only used by the legacy PM unless I'm not seeing it correctly. Fixing this will require more futzing, possibly adding includes for the header or moving the struct definition from its current location. So I was hoping that we'd let that go given that the new PM seems relatively close to becoming default.

[spatel]

include/llvm/Transforms/Scalar.h

185–187 ↗

(On Diff #120589)

This is an example of the legacy PM API that I modeled the simplifyCFG creator wrapper on.

[hfinkel]

In D38631#909235, @spatel wrote:

Patch updated:

1. Keep default SimplifyCFGPass constructor and use it where appropriate (reminder: this patch is NFCI, so I preserved the non-default instantiations in the new PM with one FIXME comment).

Okay. I understand that you want to switch the defaults in a follow-up patch. Why, however, don't you make SimplifyCFGOptions() .forwardSwitchCondToPhi(true).convertSwitchToLookupTable(true).needCanonicalLoops(false) the default, and then change the default in a follow-up patch?

2. Add methods to SimplifyCFGOptions to allow builder constructor pattern which provide easier-to-read instantiations of SimplifyCFGPass (please make sure I did that correctly because I've never tried that before!).

include/llvm/Transforms/Scalar.h

263 ↗

(On Diff #120589)

Okay (although this is slightly suboptimal for the backends which I imagine will use the legacy pass manager longer).

[spatel]

Patch updated:
Use the "late" options in the default SimplifyCFGPass constructor. This makes the next (functional) patch smaller and eliminates all of the diffs from lib/Passes/PassBuilder.cpp in this one.

[hfinkel]

In D38631#909794, @spatel wrote:

Patch updated:
Use the "late" options in the default SimplifyCFGPass constructor. This makes the next (functional) patch smaller and eliminates all of the diffs from lib/Passes/PassBuilder.cpp in this one.

LGTM

llvm/trunk/include/llvm-c/Transforms/Scalar.h

 void LLVMAddBitTrackingDCEPass(LLVMPassManagerRef PM);
 
 /** See llvm::createAlignmentFromAssumptionsPass function. */
 void LLVMAddAlignmentFromAssumptionsPass(LLVMPassManagerRef PM);
 
 /** See llvm::createCFGSimplificationPass function. */
 void LLVMAddCFGSimplificationPass(LLVMPassManagerRef PM);
 
-/** See llvm::createLateCFGSimplificationPass function. */
-void LLVMAddLateCFGSimplificationPass(LLVMPassManagerRef PM);
-
 /** See llvm::createDeadStoreEliminationPass function. */
 void LLVMAddDeadStoreEliminationPass(LLVMPassManagerRef PM);
 
 /** See llvm::createScalarizerPass function. */
 void LLVMAddScalarizerPass(LLVMPassManagerRef PM);
 
 /** See llvm::createMergedLoadStoreMotionPass function. */
 void LLVMAddMergedLoadStoreMotionPass(LLVMPassManagerRef PM);

llvm/trunk/include/llvm/InitializePasses.h

 void initializeInstructionCombiningPassPass(PassRegistry&);
 void initializeInstructionSelectPass(PassRegistry&);
 void initializeInterleavedAccessPass(PassRegistry&);
 void initializeInternalizeLegacyPassPass(PassRegistry&);
 void initializeIntervalPartitionPass(PassRegistry&);
 void initializeJumpThreadingPass(PassRegistry&);
 void initializeLCSSAVerificationPassPass(PassRegistry&);
 void initializeLCSSAWrapperPassPass(PassRegistry&);
-void initializeLateCFGSimplifyPassPass(PassRegistry&);
 void initializeLazyBlockFrequencyInfoPassPass(PassRegistry&);
 void initializeLazyBranchProbabilityInfoPassPass(PassRegistry&);
 void initializeLazyMachineBlockFrequencyInfoPassPass(PassRegistry&);
 void initializeLazyValueInfoPrinterPass(PassRegistry&);
 void initializeLazyValueInfoWrapperPassPass(PassRegistry&);
 void initializeLegacyLICMPassPass(PassRegistry&);
 void initializeLegacyLoopSinkPassPass(PassRegistry&);
 void initializeLegalizerPass(PassRegistry&);

llvm/trunk/include/llvm/LinkAllPasses.h

ForcePassLinking() {
       (void) llvm::createSCEVAAWrapperPass();
       (void) llvm::createTypeBasedAAWrapperPass();
       (void) llvm::createScopedNoAliasAAWrapperPass();
       (void) llvm::createBoundsCheckingPass();
       (void) llvm::createBreakCriticalEdgesPass();
       (void) llvm::createCallGraphDOTPrinterPass();
       (void) llvm::createCallGraphViewerPass();
       (void) llvm::createCFGSimplificationPass();
-      (void) llvm::createLateCFGSimplificationPass();
       (void) llvm::createCFLAndersAAWrapperPass();
       (void) llvm::createCFLSteensAAWrapperPass();
       (void) llvm::createStructurizeCFGPass();
       (void) llvm::createLibCallsShrinkWrapPass();
       (void) llvm::createCalledValuePropagationPass();
       (void) llvm::createConstantMergePass();
       (void) llvm::createConstantPropagationPass();
       (void) llvm::createCostModelAnalysisPass();

llvm/trunk/include/llvm/Transforms/Scalar.h

 // preds always go to some succ. Thresholds other than minus one override the
 // internal BB duplication default threshold.
 //
 FunctionPass *createJumpThreadingPass(int Threshold = -1);
 
 //===----------------------------------------------------------------------===//
 //
 // CFGSimplification - Merge basic blocks, eliminate unreachable blocks,
-// simplify terminator instructions, etc...
+// simplify terminator instructions, convert switches to lookup tables, etc.
 //
 FunctionPass *createCFGSimplificationPass(
-    int Threshold = -1, std::function<bool(const Function &)> Ftor = nullptr);
-
-//===----------------------------------------------------------------------===//
-//
-// LateCFGSimplification - Like CFGSimplification, but may also
-// convert switches to lookup tables.
-//
-FunctionPass *createLateCFGSimplificationPass(
-    int Threshold = -1, std::function<bool(const Function &)> Ftor = nullptr);
+    unsigned Threshold = 1, bool ForwardSwitchCond = false,
+    bool ConvertSwitch = false, bool KeepLoops = true,
+    std::function<bool(const Function &)> Ftor = nullptr);
 
 //===----------------------------------------------------------------------===//
 //
 // FlattenCFG - flatten CFG, reduce number of conditional branches by using
 // parallel-and and parallel-or mode, etc...
 //
 FunctionPass *createFlattenCFGPass();
 

llvm/trunk/include/llvm/Transforms/Scalar/SimplifyCFG.h

 /// 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> {
   SimplifyCFGOptions Options;
 
 public:
-  /// Construct a pass with default options.
-  SimplifyCFGPass();
+  /// The default constructor sets the pass options to create optimal IR,
+  /// rather than canonical IR. That is, by default we do transformations that
+  /// are likely to improve performance but make analysis more difficult.
+  /// FIXME: This is inverted from what most instantiations of the pass should
+  /// be.
+  SimplifyCFGPass()
+      : SimplifyCFGPass(SimplifyCFGOptions()
+                            .forwardSwitchCondToPhi(true)
+                            .convertSwitchToLookupTable(true)
+                            .needCanonicalLoops(false)) {}
 
   /// Construct a pass with optional optimizations.
   SimplifyCFGPass(const SimplifyCFGOptions &PassOptions);
 
   /// \brief Run the pass over the function.
   PreservedAnalyses run(Function &F, FunctionAnalysisManager &AM);
 };
 
 }
 
 #endif

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

 /// replaced by lookup tables and selects.
 struct SimplifyCFGOptions {
   int BonusInstThreshold;
   bool ForwardSwitchCondToPhi;
   bool ConvertSwitchToLookupTable;
   bool NeedCanonicalLoop;
   AssumptionCache *AC;
 
-  SimplifyCFGOptions(int BonusThreshold = 1, bool ForwardSwitchCond = false,
+  SimplifyCFGOptions(unsigned BonusThreshold = 1,
+                     bool ForwardSwitchCond = false,
                      bool SwitchToLookup = false, bool CanonicalLoops = true,
                      AssumptionCache *AssumpCache = nullptr)
       : BonusInstThreshold(BonusThreshold),
         ForwardSwitchCondToPhi(ForwardSwitchCond),
         ConvertSwitchToLookupTable(SwitchToLookup),
         NeedCanonicalLoop(CanonicalLoops), AC(AssumpCache) {}
+
+  // Support 'builder' pattern to set members by name at construction time.
+  SimplifyCFGOptions &bonusInstThreshold(int I) {
+    BonusInstThreshold = I;
+    return *this;
+  }
+  SimplifyCFGOptions &forwardSwitchCondToPhi(bool B) {
+    ForwardSwitchCondToPhi = B;
+    return *this;
+  }
+  SimplifyCFGOptions &convertSwitchToLookupTable(bool B) {
+    ConvertSwitchToLookupTable = B;
+    return *this;
+  }
+  SimplifyCFGOptions &needCanonicalLoops(bool B) {
+    NeedCanonicalLoop = B;
+    return *this;
+  }
+  SimplifyCFGOptions &setAssumptionCache(AssumptionCache *Cache) {
+    AC = Cache;
+    return *this;
+  }
 };
 
 //===----------------------------------------------------------------------===//
 //  Local constant propagation.
 //
 
 /// If a terminator instruction is predicated on a constant value, convert it
 /// into an unconditional branch to the constant destination.

llvm/trunk/lib/LTO/LTOCodeGenerator.cpp

void LTOCodeGenerator::initializeLTOPasses() {
   initializeReversePostOrderFunctionAttrsLegacyPassPass(R);
   initializeGlobalsAAWrapperPassPass(R);
   initializeLegacyLICMPassPass(R);
   initializeMergedLoadStoreMotionLegacyPassPass(R);
   initializeGVNLegacyPassPass(R);
   initializeMemCpyOptLegacyPassPass(R);
   initializeDCELegacyPassPass(R);
   initializeCFGSimplifyPassPass(R);
-  initializeLateCFGSimplifyPassPass(R);
 }
 
 void LTOCodeGenerator::setAsmUndefinedRefs(LTOModule *Mod) {
   const std::vector<StringRef> &undefs = Mod->getAsmUndefinedRefs();
   for (int i = 0, e = undefs.size(); i != e; ++i)
     AsmUndefinedRefs[undefs[i]] = 1;
 }
 

llvm/trunk/lib/Target/AArch64/AArch64TargetMachine.cpp

void AArch64PassConfig::addIRPasses() {
   // Always expand atomic operations, we don't deal with atomicrmw or cmpxchg
   // ourselves.
   addPass(createAtomicExpandPass());
 
   // Cmpxchg instructions are often used with a subsequent comparison to
   // determine whether it succeeded. We can exploit existing control-flow in
   // ldrex/strex loops to simplify this, but it needs tidying up.
   if (TM->getOptLevel() != CodeGenOpt::None && EnableAtomicTidy)
-    addPass(createLateCFGSimplificationPass());
+    addPass(createCFGSimplificationPass(1, true, true, false));
 
   // Run LoopDataPrefetch
   //
   // Run this before LSR to remove the multiplies involved in computing the
   // pointer values N iterations ahead.
   if (TM->getOptLevel() != CodeGenOpt::None) {
     if (EnableLoopDataPrefetch)
       addPass(createLoopDataPrefetchPass());

llvm/trunk/lib/Target/ARM/ARMTargetMachine.cpp

if (TM->Options.ThreadModel == ThreadModel::Single)
     addPass(createLowerAtomicPass());
   else
     addPass(createAtomicExpandPass());
 
   // Cmpxchg instructions are often used with a subsequent comparison to
   // determine whether it succeeded. We can exploit existing control-flow in
   // ldrex/strex loops to simplify this, but it needs tidying up.
   if (TM->getOptLevel() != CodeGenOpt::None && EnableAtomicTidy)
-    addPass(createCFGSimplificationPass(-1, [this](const Function &F) {
+    addPass(createCFGSimplificationPass(
+        1, false, false, true, [this](const Function &F) {
           const auto &ST = this->TM->getSubtarget<ARMSubtarget>(F);
           return ST.hasAnyDataBarrier() && !ST.isThumb1Only();
         }));
 
   TargetPassConfig::addIRPasses();
 
   // Match interleaved memory accesses to ldN/stN intrinsics.
   if (TM->getOptLevel() != CodeGenOpt::None)
     addPass(createInterleavedAccessPass());
 }
 

llvm/trunk/lib/Transforms/IPO/PassManagerBuilder.cpp

void PassManagerBuilder::populateModulePassManager(
   if (RunSLPAfterLoopVectorization && SLPVectorize) {
     MPM.add(createSLPVectorizerPass()); // Vectorize parallel scalar chains.
     if (OptLevel > 1 && ExtraVectorizerPasses) {
       MPM.add(createEarlyCSEPass());
     }
   }
 
   addExtensionsToPM(EP_Peephole, MPM);
-  MPM.add(createLateCFGSimplificationPass()); // Switches to lookup tables
+  // Switches to lookup tables and other transforms that may not be considered
+  // canonical by other IR passes.
+  MPM.add(createCFGSimplificationPass(1, true, true, false));
   addInstructionCombiningPass(MPM);
 
   if (!DisableUnrollLoops) {
     MPM.add(createLoopUnrollPass(OptLevel));    // Unroll small loops
 
     // LoopUnroll may generate some redundency to cleanup.
     addInstructionCombiningPass(MPM);
 

llvm/trunk/lib/Transforms/Scalar/Scalar.cpp

void llvm::initializeScalarOpts(PassRegistry &Registry) {
   initializePartiallyInlineLibCallsLegacyPassPass(Registry);
   initializeReassociateLegacyPassPass(Registry);
   initializeRegToMemPass(Registry);
   initializeRewriteStatepointsForGCPass(Registry);
   initializeSCCPLegacyPassPass(Registry);
   initializeIPSCCPLegacyPassPass(Registry);
   initializeSROALegacyPassPass(Registry);
   initializeCFGSimplifyPassPass(Registry);
-  initializeLateCFGSimplifyPassPass(Registry);
   initializeStructurizeCFGPass(Registry);
   initializeSimpleLoopUnswitchLegacyPassPass(Registry);
   initializeSinkingLegacyPassPass(Registry);
   initializeTailCallElimPass(Registry);
   initializeSeparateConstOffsetFromGEPPass(Registry);
   initializeSpeculativeExecutionLegacyPassPass(Registry);
   initializeStraightLineStrengthReducePass(Registry);
   initializePlaceBackedgeSafepointsImplPass(Registry);
void LLVMAddBitTrackingDCEPass(LLVMPassManagerRef PM) {
   unwrap(PM)->add(createBitTrackingDCEPass());
 }
 
 void LLVMAddAlignmentFromAssumptionsPass(LLVMPassManagerRef PM) {
   unwrap(PM)->add(createAlignmentFromAssumptionsPass());
 }
 
 void LLVMAddCFGSimplificationPass(LLVMPassManagerRef PM) {
-  unwrap(PM)->add(createCFGSimplificationPass());
-}
-
-void LLVMAddLateCFGSimplificationPass(LLVMPassManagerRef PM) {
-  unwrap(PM)->add(createLateCFGSimplificationPass());
+  unwrap(PM)->add(createCFGSimplificationPass(1, false, false, true));
 }
 
 void LLVMAddDeadStoreEliminationPass(LLVMPassManagerRef PM) {
   unwrap(PM)->add(createDeadStoreEliminationPass());
 }
 
 void LLVMAddScalarizerPass(LLVMPassManagerRef PM) {
   unwrap(PM)->add(createScalarizerPass());

llvm/trunk/lib/Transforms/Scalar/SimplifyCFGPass.cpp

 #include "llvm/Transforms/Scalar.h"
 #include "llvm/Transforms/Scalar/SimplifyCFG.h"
 #include "llvm/Transforms/Utils/Local.h"
 #include <utility>
 using namespace llvm;
 
 #define DEBUG_TYPE "simplifycfg"
 
-static cl::opt<unsigned>
-UserBonusInstThreshold("bonus-inst-threshold", cl::Hidden, cl::init(1),
+static cl::opt<unsigned> UserBonusInstThreshold(
+    "bonus-inst-threshold", cl::Hidden, cl::init(1),
     cl::desc("Control the number of bonus instructions (default = 1)"));
 
+static cl::opt<bool> UserKeepLoops(
+    "keep-loops", cl::Hidden, cl::init(true),
+    cl::desc("Preserve canonical loop structure (default = true)"));
+
+static cl::opt<bool> UserSwitchToLookup(
+    "switch-to-lookup", cl::Hidden, cl::init(false),
+    cl::desc("Convert switches to lookup tables (default = false)"));
+
+static cl::opt<bool> UserForwardSwitchCond(
+    "forward-switch-cond", cl::Hidden, cl::init(false),
+    cl::desc("Forward switch condition to phi ops (default = false)"));
+
 STATISTIC(NumSimpl, "Number of blocks simplified");
 
 /// If we have more than one empty (other than phi node) return blocks,
 /// merge them together to promote recursive block merging.
 static bool mergeEmptyReturnBlocks(Function &F) {
   bool Changed = false;
 
   BasicBlock *RetBlock = nullptr;
static bool simplifyFunctionCFG(Function &F, const TargetTransformInfo &TTI,
   do {
     EverChanged = iterativelySimplifyCFG(F, TTI, Options);
     EverChanged |= removeUnreachableBlocks(F);
   } while (EverChanged);
 
   return true;
 }
 
-// FIXME: The new pass manager always creates a "late" simplifycfg pass using
-// this default constructor.
-SimplifyCFGPass::SimplifyCFGPass()
-    : Options(UserBonusInstThreshold, true, true, false) {}
-
-SimplifyCFGPass::SimplifyCFGPass(const SimplifyCFGOptions &PassOptions)
-    : Options(PassOptions) {}
+// Command-line settings override compile-time settings.
+SimplifyCFGPass::SimplifyCFGPass(const SimplifyCFGOptions &Opts) {
+  Options.BonusInstThreshold = UserBonusInstThreshold.getNumOccurrences()
+                                   ? UserBonusInstThreshold
+                                   : Opts.BonusInstThreshold;
+  Options.ForwardSwitchCondToPhi = UserForwardSwitchCond.getNumOccurrences()
+                                       ? UserForwardSwitchCond
+                                       : Opts.ForwardSwitchCondToPhi;
+  Options.ConvertSwitchToLookupTable = UserSwitchToLookup.getNumOccurrences()
+                                           ? UserSwitchToLookup
+                                           : Opts.ConvertSwitchToLookupTable;
+  Options.NeedCanonicalLoop = UserKeepLoops.getNumOccurrences()
+                                  ? UserKeepLoops
+                                  : Opts.NeedCanonicalLoop;
+}
 
 PreservedAnalyses SimplifyCFGPass::run(Function &F,
                                        FunctionAnalysisManager &AM) {
   auto &TTI = AM.getResult<TargetIRAnalysis>(F);
   Options.AC = &AM.getResult<AssumptionAnalysis>(F);
   if (!simplifyFunctionCFG(F, TTI, Options))
     return PreservedAnalyses::all();
   PreservedAnalyses PA;
   PA.preserve<GlobalsAA>();
   return PA;
 }
 
 namespace {
-struct BaseCFGSimplifyPass : public FunctionPass {
+struct CFGSimplifyPass : public FunctionPass {
+  static char ID;
+  SimplifyCFGOptions Options;
   std::function<bool(const Function &)> PredicateFtor;
-  int BonusInstThreshold;
-  bool ForwardSwitchCondToPhi;
-  bool ConvertSwitchToLookupTable;
-  bool KeepCanonicalLoops;
-
-  BaseCFGSimplifyPass(int T, bool ForwardSwitchCond, bool ConvertSwitch,
-                      bool KeepLoops,
-                      std::function<bool(const Function &)> Ftor, char &ID)
-      : FunctionPass(ID), PredicateFtor(std::move(Ftor)),
-        ForwardSwitchCondToPhi(ForwardSwitchCond),
-        ConvertSwitchToLookupTable(ConvertSwitch),
-        KeepCanonicalLoops(KeepLoops) {
-    BonusInstThreshold = (T == -1) ? UserBonusInstThreshold : T;
+
+  CFGSimplifyPass(unsigned Threshold = 1, bool ForwardSwitchCond = false,
+                  bool ConvertSwitch = false, bool KeepLoops = true,
+                  std::function<bool(const Function &)> Ftor = nullptr)
+      : FunctionPass(ID), PredicateFtor(std::move(Ftor)) {
+
+    initializeCFGSimplifyPassPass(*PassRegistry::getPassRegistry());
+
+    // Check for command-line overrides of options for debug/customization.
+    Options.BonusInstThreshold = UserBonusInstThreshold.getNumOccurrences()
+                                    ? UserBonusInstThreshold
+                                    : Threshold;
+
+    Options.ForwardSwitchCondToPhi = UserForwardSwitchCond.getNumOccurrences()
+                                         ? UserForwardSwitchCond
+                                         : ForwardSwitchCond;
+
+    Options.ConvertSwitchToLookupTable = UserSwitchToLookup.getNumOccurrences()
+                                             ? UserSwitchToLookup
+                                             : ConvertSwitch;
+
+    Options.NeedCanonicalLoop =
+        UserKeepLoops.getNumOccurrences() ? UserKeepLoops : KeepLoops;
   }
+
   bool runOnFunction(Function &F) override {
     if (skipFunction(F) || (PredicateFtor && !PredicateFtor(F)))
       return false;
 
-    AssumptionCache *AC =
-        &getAnalysis<AssumptionCacheTracker>().getAssumptionCache(F);
-    const TargetTransformInfo &TTI =
-        getAnalysis<TargetTransformInfoWrapperPass>().getTTI(F);
-    return simplifyFunctionCFG(F, TTI,
-                               {BonusInstThreshold, ForwardSwitchCondToPhi,
-                                ConvertSwitchToLookupTable, KeepCanonicalLoops,
-                                AC});
+    Options.AC = &getAnalysis<AssumptionCacheTracker>().getAssumptionCache(F);
+    auto &TTI = getAnalysis<TargetTransformInfoWrapperPass>().getTTI(F);
+    return simplifyFunctionCFG(F, TTI, Options);
   }
-
   void getAnalysisUsage(AnalysisUsage &AU) const override {
     AU.addRequired<AssumptionCacheTracker>();
     AU.addRequired<TargetTransformInfoWrapperPass>();
     AU.addPreserved<GlobalsAAWrapperPass>();
   }
 };
-
-struct CFGSimplifyPass : public BaseCFGSimplifyPass {
-  static char ID; // Pass identification, replacement for typeid
-
-  CFGSimplifyPass(int T = -1,
-                  std::function<bool(const Function &)> Ftor = nullptr)
-                  : BaseCFGSimplifyPass(T, false, false, true, Ftor, ID) {
-    initializeCFGSimplifyPassPass(*PassRegistry::getPassRegistry());
-  }
-};
-
-struct LateCFGSimplifyPass : public BaseCFGSimplifyPass {
-  static char ID; // Pass identification, replacement for typeid
-
-  LateCFGSimplifyPass(int T = -1,
-                      std::function<bool(const Function &)> Ftor = nullptr)
-                      : BaseCFGSimplifyPass(T, true, true, false, Ftor, ID) {
-    initializeLateCFGSimplifyPassPass(*PassRegistry::getPassRegistry());
-  }
-};
 }
 
 char CFGSimplifyPass::ID = 0;
 INITIALIZE_PASS_BEGIN(CFGSimplifyPass, "simplifycfg", "Simplify the CFG", false,
                       false)
 INITIALIZE_PASS_DEPENDENCY(TargetTransformInfoWrapperPass)
 INITIALIZE_PASS_DEPENDENCY(AssumptionCacheTracker)
 INITIALIZE_PASS_END(CFGSimplifyPass, "simplifycfg", "Simplify the CFG", false,
                     false)
 
-char LateCFGSimplifyPass::ID = 0;
-INITIALIZE_PASS_BEGIN(LateCFGSimplifyPass, "latesimplifycfg",
-                      "Simplify the CFG more aggressively", false, false)
-INITIALIZE_PASS_DEPENDENCY(TargetTransformInfoWrapperPass)
-INITIALIZE_PASS_DEPENDENCY(AssumptionCacheTracker)
-INITIALIZE_PASS_END(LateCFGSimplifyPass, "latesimplifycfg",
-                    "Simplify the CFG more aggressively", false, false)
-
 // Public interface to the CFGSimplification pass
 FunctionPass *
-llvm::createCFGSimplificationPass(int Threshold,
-    std::function<bool(const Function &)> Ftor) {
-  return new CFGSimplifyPass(Threshold, std::move(Ftor));
-}
-
-// Public interface to the LateCFGSimplification pass
-FunctionPass *
-llvm::createLateCFGSimplificationPass(int Threshold, 
+llvm::createCFGSimplificationPass(unsigned Threshold, bool ForwardSwitchCond,
+                                  bool ConvertSwitch, bool KeepLoops,
                                   std::function<bool(const Function &)> Ftor) {
-  return new LateCFGSimplifyPass(Threshold, std::move(Ftor));
+  return new CFGSimplifyPass(Threshold, ForwardSwitchCond, ConvertSwitch,
+                             KeepLoops, std::move(Ftor));
 }

llvm/trunk/test/CodeGen/AArch64/cmpxchg-idioms.ll

 ; CHECK: stlxr [[STATUS:w[0-9]+]], w20, [x19]
 ; CHECK: cbnz [[STATUS]], [[LOOP]]
 ; CHECK: orr [[STATUS]], wzr, #0x1
 ; CHECK: b [[PH:LBB[0-9]+_[0-9]+]]
 
 ; CHECK: [[FAILED]]:
 ; CHECK-NOT: cmp {{w[0-9]+}}, {{w[0-9]+}}
 
-; verify the preheader is simplified by latesimplifycfg.
+; verify the preheader is simplified by simplifycfg.
 ; CHECK: [[PH]]:
 ; CHECK: orr w22, wzr, #0x2
 ; CHECK-NOT: orr w22, wzr, #0x4
 ; CHECK-NOT: cmn w22, #4
 ; CHECK: b [[LOOP2:LBB[0-9]+_[0-9]+]]
 ; CHECK-NOT: b.ne [[LOOP2]]
 ; CHECK-NOT: b {{LBB[0-9]+_[0-9]+}}
 ; CHECK: bl _foo

llvm/trunk/test/Transforms/LoopVectorize/X86/float-induction-x86.ll

-; RUN: opt < %s  -O3 -latesimplifycfg -mcpu=core-avx2 -mtriple=x86_64-unknown-linux-gnu -S | FileCheck --check-prefix AUTO_VEC %s
+; RUN: opt < %s  -O3 -simplifycfg -keep-loops=false -mcpu=core-avx2 -mtriple=x86_64-unknown-linux-gnu -S | FileCheck --check-prefix AUTO_VEC %s
 
 ; This test checks auto-vectorization with FP induction variable.
 ; The FP operation is not "fast" and requires "fast-math" function attribute.
 
 ;void fp_iv_loop1(float * __restrict__ A, int N) {
 ;  float x = 1.0;
 ;  for (int i=0; i < N; ++i) {
 ;    A[i] = x;

llvm/trunk/test/Transforms/LoopVectorize/float-induction.ll

 ; RUN: opt < %s  -loop-vectorize -force-vector-interleave=1 -force-vector-width=4 -dce -instcombine -S | FileCheck --check-prefix VEC4_INTERL1 %s
 ; RUN: opt < %s  -loop-vectorize -force-vector-interleave=2 -force-vector-width=4 -dce -instcombine -S | FileCheck --check-prefix VEC4_INTERL2 %s
 ; RUN: opt < %s  -loop-vectorize -force-vector-interleave=2 -force-vector-width=1 -dce -instcombine -S | FileCheck --check-prefix VEC1_INTERL2 %s
-; RUN: opt < %s  -loop-vectorize -force-vector-interleave=1 -force-vector-width=2 -dce -simplifycfg -instcombine -latesimplifycfg -S | FileCheck --check-prefix VEC2_INTERL1_PRED_STORE %s
+; RUN: opt < %s  -loop-vectorize -force-vector-interleave=1 -force-vector-width=2 -dce -simplifycfg -instcombine -simplifycfg -keep-loops=false -S | FileCheck --check-prefix VEC2_INTERL1_PRED_STORE %s
 
 @fp_inc = common global float 0.000000e+00, align 4
 
 ;void fp_iv_loop1(float init, float * __restrict__ A, int N) {
 ;  float x = init;
 ;  for (int i=0; i < N; ++i) {
 ;    A[i] = x;
 ;    x -= fp_inc;

llvm/trunk/test/Transforms/SimplifyCFG/ARM/switch-to-lookup-table.ll

-; RUN: opt -S -latesimplifycfg -mtriple=arm -relocation-model=static    < %s | FileCheck %s --check-prefix=CHECK --check-prefix=ENABLE
-; RUN: opt -S -latesimplifycfg -mtriple=arm -relocation-model=pic       < %s | FileCheck %s --check-prefix=CHECK --check-prefix=ENABLE
-; RUN: opt -S -latesimplifycfg -mtriple=arm -relocation-model=ropi      < %s | FileCheck %s --check-prefix=CHECK --check-prefix=DISABLE
-; RUN: opt -S -latesimplifycfg -mtriple=arm -relocation-model=rwpi      < %s | FileCheck %s --check-prefix=CHECK --check-prefix=DISABLE
-; RUN: opt -S -latesimplifycfg -mtriple=arm -relocation-model=ropi-rwpi < %s | FileCheck %s --check-prefix=CHECK --check-prefix=DISABLE
+; RUN: opt -S -simplifycfg -switch-to-lookup -mtriple=arm -relocation-model=static    < %s | FileCheck %s --check-prefix=CHECK --check-prefix=ENABLE
+; RUN: opt -S -simplifycfg -switch-to-lookup -mtriple=arm -relocation-model=pic       < %s | FileCheck %s --check-prefix=CHECK --check-prefix=ENABLE
+; RUN: opt -S -simplifycfg -switch-to-lookup -mtriple=arm -relocation-model=ropi      < %s | FileCheck %s --check-prefix=CHECK --check-prefix=DISABLE
+; RUN: opt -S -simplifycfg -switch-to-lookup -mtriple=arm -relocation-model=rwpi      < %s | FileCheck %s --check-prefix=CHECK --check-prefix=DISABLE
+; RUN: opt -S -simplifycfg -switch-to-lookup -mtriple=arm -relocation-model=ropi-rwpi < %s | FileCheck %s --check-prefix=CHECK --check-prefix=DISABLE
 
 ; CHECK:       @{{.*}} = private unnamed_addr constant [3 x i32] [i32 1234, i32 5678, i32 15532]
 ; ENABLE:      @{{.*}} = private unnamed_addr constant [3 x i32*] [i32* @c1, i32* @c2, i32* @c3]
 ; DISABLE-NOT: @{{.*}} = private unnamed_addr constant [3 x i32*] [i32* @c1, i32* @c2, i32* @c3]
 ; ENABLE:      @{{.*}} = private unnamed_addr constant [3 x i32*] [i32* @g1, i32* @g2, i32* @g3]
 ; DISABLE-NOT: @{{.*}} = private unnamed_addr constant [3 x i32*] [i32* @g1, i32* @g2, i32* @g3]
 ; ENABLE:      @{{.*}} = private unnamed_addr constant [3 x i32 (i32, i32)*] [i32 (i32, i32)* @f1, i32 (i32, i32)* @f2, i32 (i32, i32)* @f3]
 ; DISABLE-NOT: @{{.*}} = private unnamed_addr constant [3 x i32 (i32, i32)*] [i32 (i32, i32)* @f1, i32 (i32, i32)* @f2, i32 (i32, i32)* @f3]

llvm/trunk/test/Transforms/SimplifyCFG/CoveredLookupTable.ll

-; RUN: opt -latesimplifycfg -S %s | FileCheck %s
+; RUN: opt -simplifycfg -switch-to-lookup -S %s | FileCheck %s
 ; rdar://15268442
 
 target datalayout = "e-p:64:64:64-S128-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f16:16:16-f32:32:32-f64:64:64-f128:128:128-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64"
 target triple = "x86_64-apple-darwin12.0.0"
 
 ; CHECK-LABEL: define i3 @coveredswitch_test(
 ; CHECK: entry:
 ; CHECK-NEXT: sub i3 %input, -4

llvm/trunk/test/Transforms/SimplifyCFG/ForwardSwitchConditionToPHI.ll

 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
-; RUN: opt < %s -latesimplifycfg -S | FileCheck %s
+; RUN: opt < %s -simplifycfg -forward-switch-cond=false -S | FileCheck %s --check-prefix=NO_FWD
+; RUN: opt < %s -simplifycfg -forward-switch-cond=true  -S | FileCheck %s --check-prefix=FWD
 
 ; PR10131
 
 target datalayout = "e-p:32:32:32-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:32:64-f32:32:32-f64:32:64-v64:64:64-v128:128:128-a0:0:64-f80:32:32-n8:16:32"
 target triple = "i386-pc-linux-gnu"
 
 define i32 @t(i32 %m) nounwind readnone {
-; CHECK-LABEL: @t(
-; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[SWITCH:%.*]] = icmp ult i32 [[M:%.*]], 4
-; CHECK-NEXT:    [[M_:%.*]] = select i1 [[SWITCH]], i32 [[M]], i32 4
-; CHECK-NEXT:    ret i32 [[M_]]
+; NO_FWD-LABEL: @t(
+; NO_FWD-NEXT:  entry:
+; NO_FWD-NEXT:    switch i32 [[M:%.*]], label [[SW_BB4:%.*]] [
+; NO_FWD-NEXT:    i32 0, label [[RETURN:%.*]]
+; NO_FWD-NEXT:    i32 1, label [[SW_BB1:%.*]]
+; NO_FWD-NEXT:    i32 2, label [[SW_BB2:%.*]]
+; NO_FWD-NEXT:    i32 3, label [[SW_BB3:%.*]]
+; NO_FWD-NEXT:    ]
+; NO_FWD:       sw.bb1:
+; NO_FWD-NEXT:    br label [[RETURN]]
+; NO_FWD:       sw.bb2:
+; NO_FWD-NEXT:    br label [[RETURN]]
+; NO_FWD:       sw.bb3:
+; NO_FWD-NEXT:    br label [[RETURN]]
+; NO_FWD:       sw.bb4:
+; NO_FWD-NEXT:    br label [[RETURN]]
+; NO_FWD:       return:
+; NO_FWD-NEXT:    [[RETVAL_0:%.*]] = phi i32 [ 4, [[SW_BB4]] ], [ 3, [[SW_BB3]] ], [ 2, [[SW_BB2]] ], [ 1, [[SW_BB1]] ], [ 0, [[ENTRY:%.*]] ]
+; NO_FWD-NEXT:    ret i32 [[RETVAL_0]]
+;
+; FWD-LABEL: @t(
+; FWD-NEXT:  entry:
+; FWD-NEXT:    [[SWITCH:%.*]] = icmp ult i32 [[M:%.*]], 4
+; FWD-NEXT:    [[M_:%.*]] = select i1 [[SWITCH]], i32 [[M]], i32 4
+; FWD-NEXT:    ret i32 [[M_]]
 ;
 entry:
   switch i32 %m, label %sw.bb4 [
   i32 0, label %sw.bb0
   i32 1, label %sw.bb1
   i32 2, label %sw.bb2
   i32 3, label %sw.bb3
   ]
return:                                           ; preds = %entry, %sw.bb4, %sw.bb3, %sw.bb2, %sw.bb1
   ret i32 %retval.0
 }
 
 ; If 1 incoming phi value is a case constant of a switch, convert it to the switch condition:
 ; https://bugs.llvm.org/show_bug.cgi?id=34471
 ; This then subsequently should allow squashing of the other trivial case blocks.
 
 define i32 @PR34471(i32 %x) {
-; CHECK-LABEL: @PR34471(
-; CHECK-NEXT:  entry:
-; CHECK-NEXT:    switch i32 [[X:%.*]], label [[ELSE3:%.*]] [
-; CHECK-NEXT:    i32 17, label [[RETURN:%.*]]
-; CHECK-NEXT:    i32 19, label [[RETURN]]
-; CHECK-NEXT:    i32 42, label [[RETURN]]
-; CHECK-NEXT:    ]
-; CHECK:       else3:
-; CHECK-NEXT:    br label [[RETURN]]
-; CHECK:       return:
-; CHECK-NEXT:    [[R:%.*]] = phi i32 [ 0, [[ELSE3]] ], [ [[X]], [[ENTRY:%.*]] ], [ [[X]], [[ENTRY]] ], [ [[X]], [[ENTRY]] ]
-; CHECK-NEXT:    ret i32 [[R]]
+; NO_FWD-LABEL: @PR34471(
+; NO_FWD-NEXT:  entry:
+; NO_FWD-NEXT:    switch i32 [[X:%.*]], label [[ELSE3:%.*]] [
+; NO_FWD-NEXT:    i32 17, label [[RETURN:%.*]]
+; NO_FWD-NEXT:    i32 19, label [[IF19:%.*]]
+; NO_FWD-NEXT:    i32 42, label [[IF42:%.*]]
+; NO_FWD-NEXT:    ]
+; NO_FWD:       if19:
+; NO_FWD-NEXT:    br label [[RETURN]]
+; NO_FWD:       if42:
+; NO_FWD-NEXT:    br label [[RETURN]]
+; NO_FWD:       else3:
+; NO_FWD-NEXT:    br label [[RETURN]]
+; NO_FWD:       return:
+; NO_FWD-NEXT:    [[R:%.*]] = phi i32 [ [[X]], [[IF19]] ], [ [[X]], [[IF42]] ], [ 0, [[ELSE3]] ], [ 17, [[ENTRY:%.*]] ]
+; NO_FWD-NEXT:    ret i32 [[R]]
+;
+; FWD-LABEL: @PR34471(
+; FWD-NEXT:  entry:
+; FWD-NEXT:    switch i32 [[X:%.*]], label [[ELSE3:%.*]] [
+; FWD-NEXT:    i32 17, label [[RETURN:%.*]]
+; FWD-NEXT:    i32 19, label [[RETURN]]
+; FWD-NEXT:    i32 42, label [[RETURN]]
+; FWD-NEXT:    ]
+; FWD:       else3:
+; FWD-NEXT:    br label [[RETURN]]
+; FWD:       return:
+; FWD-NEXT:    [[R:%.*]] = phi i32 [ 0, [[ELSE3]] ], [ [[X]], [[ENTRY:%.*]] ], [ [[X]], [[ENTRY]] ], [ [[X]], [[ENTRY]] ]
+; FWD-NEXT:    ret i32 [[R]]
 ;
 entry:
   switch i32 %x, label %else3 [
   i32 17, label %return
   i32 19, label %if19
   i32 42, label %if42
   ]
 

llvm/trunk/test/Transforms/SimplifyCFG/X86/disable-lookup-table.ll

-; RUN: opt < %s -latesimplifycfg -S -mtriple=x86_64-unknown-linux-gnu | FileCheck %s
-; In the presence of "-no-jump-tables"="true", late simplifycfg should not
-; convert any switch cases to lookup tables.
+; RUN: opt < %s -simplifycfg -switch-to-lookup -S -mtriple=x86_64-unknown-linux-gnu | FileCheck %s
+
+; In the presence of "-no-jump-tables"="true", simplifycfg should not convert switches to lookup tables.
+
 ; CHECK: @switch.table.bar = private unnamed_addr constant [4 x i32] [i32 55, i32 123, i32 0, i32 -1]
 ; CHECK-LABEL: foo
 ; CHECK-NOT: @switch.table.foo = private unnamed_addr constant [4 x i32] [i32 55, i32 123, i32 0, i32 -1]
 
 define i32 @foo(i32 %c) "no-jump-tables"="true" {
 entry:
   switch i32 %c, label %sw.default [
     i32 42, label %return

llvm/trunk/test/Transforms/SimplifyCFG/X86/switch-covered-bug.ll

-; RUN: opt -S -latesimplifycfg < %s -mtriple=x86_64-apple-darwin12.0.0 | FileCheck %s
+; RUN: opt -S -simplifycfg -switch-to-lookup < %s -mtriple=x86_64-apple-darwin12.0.0 | FileCheck %s
 ; rdar://17887153
 target datalayout = "e-p:64:64:64-S128-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f16:16:16-f32:32:32-f64:64:64-f128:128:128-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64"
 target triple = "x86_64-apple-darwin12.0.0"
 
 ; When we have a covered lookup table, make sure we don't delete PHINodes that
 ; are cached in PHIs.
 ; CHECK-LABEL: @test
 ; CHECK: entry:

llvm/trunk/test/Transforms/SimplifyCFG/X86/switch-table-bug.ll

-; RUN: opt -S -latesimplifycfg < %s -mtriple=x86_64-apple-darwin12.0.0 | FileCheck %s
+; RUN: opt -S -simplifycfg -switch-to-lookup < %s -mtriple=x86_64-apple-darwin12.0.0 | FileCheck %s
 ; rdar://17735071
 target datalayout = "e-p:64:64:64-S128-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f16:16:16-f32:32:32-f64:64:64-f128:128:128-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64"
 target triple = "x86_64-apple-darwin12.0.0"
 
 ; When tableindex can't fit into i2, we should extend the type to i3.
 ; CHECK-LABEL: @_TFO6reduce1E5toRawfS0_FT_Si
 ; CHECK: entry:
 ; CHECK-NEXT: sub i2 %0, -2

llvm/trunk/test/Transforms/SimplifyCFG/X86/switch_to_lookup_table.ll

-; RUN: opt < %s -latesimplifycfg -S -mtriple=x86_64-unknown-linux-gnu | FileCheck %s
+; RUN: opt < %s -simplifycfg -switch-to-lookup=true -keep-loops=false -S -mtriple=x86_64-unknown-linux-gnu | FileCheck %s
 
 target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
 ; The table for @f
 ; CHECK: @switch.table.f = private unnamed_addr constant [7 x i32] [i32 55, i32 123, i32 0, i32 -1, i32 27, i32 62, i32 1]
 
 ; The float table for @h

llvm/trunk/test/Transforms/SimplifyCFG/multiple-phis.ll

-; RUN: opt -latesimplifycfg -S < %s | FileCheck %s
+; RUN: opt -simplifycfg -keep-loops=false -S < %s | FileCheck %s
 
 ; It's not worthwhile to if-convert one of the phi nodes and leave
 ; the other behind, because that still requires a branch. If
 ; SimplifyCFG if-converts one of the phis, it should do both.
 
 ; CHECK:      %div.high.addr.0 = select i1 %cmp1, i32 %div, i32 %high.addr.0
 ; CHECK-NEXT: %low.0.add2 = select i1 %cmp1, i32 %low.0, i32 %add2
 ; CHECK-NEXT: br label %while.cond

llvm/trunk/test/Transforms/SimplifyCFG/preserve-llvm-loop-metadata.ll

-; RUN: opt -latesimplifycfg -S < %s | FileCheck %s
+; RUN: opt -simplifycfg -keep-loops=false -S < %s | FileCheck %s
 
 define void @test1(i32 %n) #0 {
 entry:
   %n.addr = alloca i32, align 4
   %count = alloca i32, align 4
   store i32 %n, i32* %n.addr, align 4
   %0 = bitcast i32* %count to i8*
   store i32 0, i32* %count, align 4

llvm/trunk/test/Transforms/SimplifyCFG/rangereduce.ll

 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
-; RUN: opt < %s -latesimplifycfg -S | FileCheck %s
+; RUN: opt < %s -simplifycfg -switch-to-lookup -S | FileCheck %s
 
 target datalayout = "e-n32"
 
 define i32 @test1(i32 %a) {
 ; CHECK-LABEL: @test1(
 ; CHECK-NEXT:    [[TMP1:%.*]] = sub i32 [[A:%.*]], 97
 ; CHECK-NEXT:    [[TMP2:%.*]] = lshr i32 [[TMP1]], 2
 ; CHECK-NEXT:    [[TMP3:%.*]] = shl i32 [[TMP1]], 30