Revert "[SROA] For non-speculatable loads of selects -- split blo…

…ck, insert then/else blocks, form two-entry PHI node"

The assertion about not modifying the CFG seems to not hold,
will recommit in a bit.

https://lab.llvm.org/buildbot#builders/139/builds/32412

This reverts commit 03e6d9d.
This reverts commit 4f90f4a.

llvm/include/llvm/Transforms/Scalar.h

@@ -105,7 +105,7 @@ FunctionPass *createBitTrackingDCEPass();
 //
 // SROA - Replace aggregates or pieces of aggregates with scalar SSA values.
 //
-FunctionPass *createSROAPass(bool PreserveCFG = true);
+FunctionPass *createSROAPass();
 
 //===----------------------------------------------------------------------===//
 //

llvm/include/llvm/Transforms/Scalar/SROA.h

@@ -1,4 +1,4 @@
-//===- SROA.h - Scalar Replacement Of Aggregates ----------------*- C++ -*-===//
+//===- SROA.h - Scalar Replacement Of Aggregates ----------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -15,8 +15,6 @@
 #ifndef LLVM_TRANSFORMS_SCALAR_SROA_H
 #define LLVM_TRANSFORMS_SCALAR_SROA_H
 
-#include "llvm/ADT/MapVector.h"
-#include "llvm/ADT/PointerIntPair.h"
 #include "llvm/ADT/SetVector.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/IR/PassManager.h"
@@ -26,10 +24,8 @@
 namespace llvm {
 
 class AllocaInst;
-class LoadInst;
 class AssumptionCache;
 class DominatorTree;
-class DomTreeUpdater;
 class Function;
 class LLVMContext;
 class PHINode;
@@ -45,31 +41,8 @@ class AllocaSlices;
 class Partition;
 class SROALegacyPass;
 
-class SelectHandSpeculativity {
-  unsigned char Storage = 0;
-  using TrueVal = Bitfield::Element<bool, 0, 1>;  // Low 0'th bit.
-  using FalseVal = Bitfield::Element<bool, 1, 1>; // Low 1'th bit.
-public:
-  SelectHandSpeculativity() = default;
-  SelectHandSpeculativity &setAsSpeculatable(bool isTrueVal);
-  bool isSpeculatable(bool isTrueVal) const;
-  bool areAllSpeculatable() const;
-  bool areAnySpeculatable() const;
-  bool areNoneSpeculatable() const;
-  // For interop as int half of PointerIntPair.
-  explicit operator intptr_t() const { return static_cast<intptr_t>(Storage); }
-  explicit SelectHandSpeculativity(intptr_t Storage_) : Storage(Storage_) {}
-};
-static_assert(sizeof(SelectHandSpeculativity) == sizeof(unsigned char));
-
-using PossiblySpeculatableLoad =
-    PointerIntPair<LoadInst *, 2, sroa::SelectHandSpeculativity>;
-using PossiblySpeculatableLoads = SmallVector<PossiblySpeculatableLoad, 2>;
-
 } // end namespace sroa
 
-enum class SROAOptions : bool { ModifyCFG, PreserveCFG };
-
 /// An optimization pass providing Scalar Replacement of Aggregates.
 ///
 /// This pass takes allocations which can be completely analyzed (that is, they
@@ -90,9 +63,8 @@ enum class SROAOptions : bool { ModifyCFG, PreserveCFG };
 ///    SSA vector values.
 class SROAPass : public PassInfoMixin<SROAPass> {
   LLVMContext *C = nullptr;
-  DomTreeUpdater *DTU = nullptr;
+  DominatorTree *DT = nullptr;
   AssumptionCache *AC = nullptr;
-  const bool PreserveCFG;
 
   /// Worklist of alloca instructions to simplify.
   ///
@@ -126,58 +98,35 @@ class SROAPass : public PassInfoMixin<SROAPass> {
   /// All of these PHIs have been checked for the safety of speculation and by
   /// being speculated will allow promoting allocas currently in the promotable
   /// queue.
-  SetVector<PHINode *, SmallVector<PHINode *, 8>> SpeculatablePHIs;
+  SetVector<PHINode *, SmallVector<PHINode *, 2>> SpeculatablePHIs;
 
-  /// A worklist of select instructions to rewrite prior to promoting
+  /// A worklist of select instructions to speculate prior to promoting
   /// allocas.
-  SmallMapVector<SelectInst *, sroa::PossiblySpeculatableLoads, 8>
-      SelectsToRewrite;
-
-  /// Select instructions that use an alloca and are subsequently loaded can be
-  /// rewritten to load both input pointers and then select between the result,
-  /// allowing the load of the alloca to be promoted.
-  /// From this:
-  ///   %P2 = select i1 %cond, ptr %Alloca, ptr %Other
-  ///   %V = load <type>, ptr %P2
-  /// to:
-  ///   %V1 = load <type>, ptr %Alloca      -> will be mem2reg'd
-  ///   %V2 = load <type>, ptr %Other
-  ///   %V = select i1 %cond, <type> %V1, <type> %V2
   ///
-  /// We can do this to a select if its only uses are loads
-  /// and if either the operand to the select can be loaded unconditionally,
-  ///        or if we are allowed to perform CFG modifications.
-  /// If found an intervening bitcast with a single use of the load,
-  /// allow the promotion.
-  static std::optional<sroa::PossiblySpeculatableLoads>
-  isSafeSelectToSpeculate(SelectInst &SI, bool PreserveCFG);
+  /// All of these select instructions have been checked for the safety of
+  /// speculation and by being speculated will allow promoting allocas
+  /// currently in the promotable queue.
+  SetVector<SelectInst *, SmallVector<SelectInst *, 2>> SpeculatableSelects;
 
 public:
-  /// If \p PreserveCFG is set, then the pass is not allowed to modify CFG
-  /// in any way, even if it would update CFG analyses.
-  SROAPass(SROAOptions PreserveCFG);
+  SROAPass() = default;
 
   /// Run the pass over the function.
   PreservedAnalyses run(Function &F, FunctionAnalysisManager &AM);
 
-  void printPipeline(raw_ostream &OS,
-                     function_ref<StringRef(StringRef)> MapClassName2PassName);
-
 private:
   friend class sroa::AllocaSliceRewriter;
   friend class sroa::SROALegacyPass;
 
   /// Helper used by both the public run method and by the legacy pass.
-  PreservedAnalyses runImpl(Function &F, DomTreeUpdater &RunDTU,
-                            AssumptionCache &RunAC);
   PreservedAnalyses runImpl(Function &F, DominatorTree &RunDT,
                             AssumptionCache &RunAC);
 
   bool presplitLoadsAndStores(AllocaInst &AI, sroa::AllocaSlices &AS);
   AllocaInst *rewritePartition(AllocaInst &AI, sroa::AllocaSlices &AS,
                                sroa::Partition &P);
   bool splitAlloca(AllocaInst &AI, sroa::AllocaSlices &AS);
-  std::pair<bool /*Changed*/, bool /*CFGChanged*/> runOnAlloca(AllocaInst &AI);
+  bool runOnAlloca(AllocaInst &AI);
   void clobberUse(Use &U);
   bool deleteDeadInstructions(SmallPtrSetImpl<AllocaInst *> &DeletedAllocas);
   bool promoteAllocas(Function &F);

llvm/include/llvm/Transforms/Utils/BasicBlockUtils.h

@@ -464,13 +464,10 @@ Instruction *SplitBlockAndInsertIfThen(Value *Cond, Instruction *SplitBefore,
 ///     ElseBlock
 ///   SplitBefore
 ///   Tail
-///
-/// Updates DT if given.
 void SplitBlockAndInsertIfThenElse(Value *Cond, Instruction *SplitBefore,
                                    Instruction **ThenTerm,
                                    Instruction **ElseTerm,
-                                   MDNode *BranchWeights = nullptr,
-                                   DomTreeUpdater *DTU = nullptr);
+                                   MDNode *BranchWeights = nullptr);
 
 /// Check whether BB is the merge point of a if-region.
 /// If so, return the branch instruction that determines which entry into

llvm/lib/Passes/PassBuilder.cpp

@@ -836,19 +836,6 @@ Expected<GVNOptions> parseGVNOptions(StringRef Params) {
   return Result;
 }
 
-Expected<SROAOptions> parseSROAOptions(StringRef Params) {
-  if (Params.empty() || Params == "modify-cfg")
-    return SROAOptions::ModifyCFG;
-  if (Params == "preserve-cfg")
-    return SROAOptions::PreserveCFG;
-  return make_error<StringError>(
-      formatv("invalid SROA pass parameter '{0}' (either preserve-cfg or "
-              "modify-cfg can be specified)",
-              Params)
-          .str(),
-      inconvertibleErrorCode());
-}
-
 Expected<StackLifetime::LivenessType>
 parseStackLifetimeOptions(StringRef Params) {
   StackLifetime::LivenessType Result = StackLifetime::LivenessType::May;

llvm/lib/Passes/PassBuilderPipelines.cpp

@@ -328,7 +328,7 @@ PassBuilder::buildO1FunctionSimplificationPipeline(OptimizationLevel Level,
 
   // Form SSA out of local memory accesses after breaking apart aggregates into
   // scalars.
-  FPM.addPass(SROAPass(SROAOptions::ModifyCFG));
+  FPM.addPass(SROAPass());
 
   // Catch trivial redundancies
   FPM.addPass(EarlyCSEPass(true /* Enable mem-ssa. */));
@@ -427,7 +427,7 @@ PassBuilder::buildO1FunctionSimplificationPipeline(OptimizationLevel Level,
                                               /*UseBlockFrequencyInfo=*/false));
 
   // Delete small array after loop unroll.
-  FPM.addPass(SROAPass(SROAOptions::ModifyCFG));
+  FPM.addPass(SROAPass());
 
   // Specially optimize memory movement as it doesn't look like dataflow in SSA.
   FPM.addPass(MemCpyOptPass());
@@ -478,7 +478,7 @@ PassBuilder::buildFunctionSimplificationPipeline(OptimizationLevel Level,
 
   // Form SSA out of local memory accesses after breaking apart aggregates into
   // scalars.
-  FPM.addPass(SROAPass(SROAOptions::ModifyCFG));
+  FPM.addPass(SROAPass());
 
   // Catch trivial redundancies
   FPM.addPass(EarlyCSEPass(true /* Enable mem-ssa. */));
@@ -613,7 +613,7 @@ PassBuilder::buildFunctionSimplificationPipeline(OptimizationLevel Level,
                                               /*UseBlockFrequencyInfo=*/false));
 
   // Delete small array after loop unroll.
-  FPM.addPass(SROAPass(SROAOptions::ModifyCFG));
+  FPM.addPass(SROAPass());
 
   // Try vectorization/scalarization transforms that are both improvements
   // themselves and can allow further folds with GVN and InstCombine.
@@ -714,7 +714,7 @@ void PassBuilder::addPGOInstrPasses(ModulePassManager &MPM,
     CGSCCPassManager &CGPipeline = MIWP.getPM();
 
     FunctionPassManager FPM;
-    FPM.addPass(SROAPass(SROAOptions::ModifyCFG));
+    FPM.addPass(SROAPass());
     FPM.addPass(EarlyCSEPass()); // Catch trivial redundancies.
     FPM.addPass(SimplifyCFGPass(SimplifyCFGOptions().convertSwitchRangeToICmp(
         true)));                    // Merge & remove basic blocks.
@@ -963,7 +963,7 @@ PassBuilder::buildModuleSimplificationPipeline(OptimizationLevel Level,
   // Compare/branch metadata may alter the behavior of passes like SimplifyCFG.
   EarlyFPM.addPass(LowerExpectIntrinsicPass());
   EarlyFPM.addPass(SimplifyCFGPass());
-  EarlyFPM.addPass(SROAPass(SROAOptions::ModifyCFG));
+  EarlyFPM.addPass(SROAPass());
   EarlyFPM.addPass(EarlyCSEPass());
   if (Level == OptimizationLevel::O3)
     EarlyFPM.addPass(CallSiteSplittingPass());
@@ -1113,10 +1113,7 @@ void PassBuilder::addVectorPasses(OptimizationLevel Level,
     // Now that we are done with loop unrolling, be it either by LoopVectorizer,
     // or LoopUnroll passes, some variable-offset GEP's into alloca's could have
     // become constant-offset, thus enabling SROA and alloca promotion. Do so.
-    // NOTE: we are very late in the pipeline, and we don't have any LICM
-    // or SimplifyCFG passes scheduled after us, that would cleanup
-    // the CFG mess this may created if allowed to modify CFG, so forbid that.
-    FPM.addPass(SROAPass(SROAOptions::PreserveCFG));
+    FPM.addPass(SROAPass());
   }
 
   if (!IsFullLTO) {
@@ -1207,10 +1204,7 @@ void PassBuilder::addVectorPasses(OptimizationLevel Level,
     // Now that we are done with loop unrolling, be it either by LoopVectorizer,
     // or LoopUnroll passes, some variable-offset GEP's into alloca's could have
     // become constant-offset, thus enabling SROA and alloca promotion. Do so.
-    // NOTE: we are very late in the pipeline, and we don't have any LICM
-    // or SimplifyCFG passes scheduled after us, that would cleanup
-    // the CFG mess this may created if allowed to modify CFG, so forbid that.
-    FPM.addPass(SROAPass(SROAOptions::PreserveCFG));
+    FPM.addPass(SROAPass());
     FPM.addPass(InstCombinePass());
     FPM.addPass(
         RequireAnalysisPass<OptimizationRemarkEmitterAnalysis, Function>());
@@ -1751,7 +1745,7 @@ PassBuilder::buildLTODefaultPipeline(OptimizationLevel Level,
   }
 
   // Break up allocas
-  FPM.addPass(SROAPass(SROAOptions::ModifyCFG));
+  FPM.addPass(SROAPass());
 
   // LTO provides additional opportunities for tailcall elimination due to
   // link-time inlining, and visibility of nocapture attribute.

llvm/lib/Passes/PassRegistry.def

@@ -376,6 +376,7 @@ FUNCTION_PASS("sink", SinkingPass())
 FUNCTION_PASS("slp-vectorizer", SLPVectorizerPass())
 FUNCTION_PASS("slsr", StraightLineStrengthReducePass())
 FUNCTION_PASS("speculative-execution", SpeculativeExecutionPass())
+FUNCTION_PASS("sroa", SROAPass())
 FUNCTION_PASS("strip-gc-relocates", StripGCRelocates())
 FUNCTION_PASS("structurizecfg", StructurizeCFGPass())
 FUNCTION_PASS("tailcallelim", TailCallElimPass())
@@ -472,13 +473,6 @@ FUNCTION_PASS_WITH_PARAMS("gvn",
                           "no-load-pre;load-pre;"
                           "no-split-backedge-load-pre;split-backedge-load-pre;"
                           "no-memdep;memdep")
-FUNCTION_PASS_WITH_PARAMS("sroa",
-                          "SROAPass",
-                          [](SROAOptions PreserveCFG) {
-                            return SROAPass(PreserveCFG);
-                          },
-                          parseSROAOptions,
-                          "preserve-cfg;modify-cfg")
 FUNCTION_PASS_WITH_PARAMS("print<stack-lifetime>",
                           "StackLifetimePrinterPass",
                            [](StackLifetime::LivenessType Type) {