Revert "[NFC][EarlyIfConverter] Replace boolean Predicate for a class (#108519)"

This reverts commit 9e73159.

Author: jmmartinez

llvm/lib/CodeGen/EarlyIfConversion.cpp

@@ -122,18 +122,6 @@ class SSAIfConv {
   /// The branch condition determined by analyzeBranch.
   SmallVector<MachineOperand, 4> Cond;
 
-  class PredicationStrategyBase {
-  public:
-    virtual bool canConvertIf(MachineBasicBlock *Tail) { return true; }
-    virtual bool canPredicateInstr(const MachineInstr &I) = 0;
-    virtual void predicateBlock(MachineBasicBlock *MBB,
-                                ArrayRef<MachineOperand> Cond,
-                                bool Reverse) = 0;
-    virtual ~PredicationStrategyBase() = default;
-  };
-
-  PredicationStrategyBase &Predicate;
-
 private:
   /// Instructions in Head that define values used by the conditional blocks.
   /// The hoisted instructions must be inserted after these instructions.
@@ -149,6 +137,10 @@ class SSAIfConv {
   /// and FBB.
   MachineBasicBlock::iterator InsertionPoint;
 
+  /// Return true if all non-terminator instructions in MBB can be safely
+  /// speculated.
+  bool canSpeculateInstrs(MachineBasicBlock *MBB);
+
   /// Return true if all non-terminator instructions in MBB can be safely
   /// predicated.
   bool canPredicateInstrs(MachineBasicBlock *MBB);
@@ -157,6 +149,10 @@ class SSAIfConv {
   /// Return false if any dependency is incompatible with if conversion.
   bool InstrDependenciesAllowIfConv(MachineInstr *I);
 
+  /// Predicate all instructions of the basic block with current condition
+  /// except for terminators. Reverse the condition if ReversePredicate is set.
+  void PredicateBlock(MachineBasicBlock *MBB, bool ReversePredicate);
+
   /// Find a valid insertion point in Head.
   bool findInsertionPoint();
 
@@ -167,8 +163,7 @@ class SSAIfConv {
   void rewritePHIOperands();
 
 public:
-  SSAIfConv(PredicationStrategyBase &Predicate, MachineFunction &MF)
-      : Predicate(Predicate) {
+  SSAIfConv(MachineFunction &MF) {
     TII = MF.getSubtarget().getInstrInfo();
     TRI = MF.getSubtarget().getRegisterInfo();
     MRI = &MF.getRegInfo();
@@ -180,14 +175,77 @@ class SSAIfConv {
 
   /// canConvertIf - If the sub-CFG headed by MBB can be if-converted,
   /// initialize the internal state, and return true.
-  bool canConvertIf(MachineBasicBlock *MBB);
+  /// If predicate is set try to predicate the block otherwise try to
+  /// speculatively execute it.
+  bool canConvertIf(MachineBasicBlock *MBB, bool Predicate = false);
 
   /// convertIf - If-convert the last block passed to canConvertIf(), assuming
   /// it is possible. Add any blocks that are to be erased to RemoveBlocks.
-  void convertIf(SmallVectorImpl<MachineBasicBlock *> &RemoveBlocks);
+  void convertIf(SmallVectorImpl<MachineBasicBlock *> &RemoveBlocks,
+                 bool Predicate = false);
 };
 } // end anonymous namespace
 
+
+/// canSpeculateInstrs - Returns true if all the instructions in MBB can safely
+/// be speculated. The terminators are not considered.
+///
+/// If instructions use any values that are defined in the head basic block,
+/// the defining instructions are added to InsertAfter.
+///
+/// Any clobbered regunits are added to ClobberedRegUnits.
+///
+bool SSAIfConv::canSpeculateInstrs(MachineBasicBlock *MBB) {
+  // Reject any live-in physregs. It's probably CPSR/EFLAGS, and very hard to
+  // get right.
+  if (!MBB->livein_empty()) {
+    LLVM_DEBUG(dbgs() << printMBBReference(*MBB) << " has live-ins.\n");
+    return false;
+  }
+
+  unsigned InstrCount = 0;
+
+  // Check all instructions, except the terminators. It is assumed that
+  // terminators never have side effects or define any used register values.
+  for (MachineInstr &MI :
+       llvm::make_range(MBB->begin(), MBB->getFirstTerminator())) {
+    if (MI.isDebugInstr())
+      continue;
+
+    if (++InstrCount > BlockInstrLimit && !Stress) {
+      LLVM_DEBUG(dbgs() << printMBBReference(*MBB) << " has more than "
+                        << BlockInstrLimit << " instructions.\n");
+      return false;
+    }
+
+    // There shouldn't normally be any phis in a single-predecessor block.
+    if (MI.isPHI()) {
+      LLVM_DEBUG(dbgs() << "Can't hoist: " << MI);
+      return false;
+    }
+
+    // Don't speculate loads. Note that it may be possible and desirable to
+    // speculate GOT or constant pool loads that are guaranteed not to trap,
+    // but we don't support that for now.
+    if (MI.mayLoad()) {
+      LLVM_DEBUG(dbgs() << "Won't speculate load: " << MI);
+      return false;
+    }
+
+    // We never speculate stores, so an AA pointer isn't necessary.
+    bool DontMoveAcrossStore = true;
+    if (!MI.isSafeToMove(DontMoveAcrossStore)) {
+      LLVM_DEBUG(dbgs() << "Can't speculate: " << MI);
+      return false;
+    }
+
+    // Check for any dependencies on Head instructions.
+    if (!InstrDependenciesAllowIfConv(&MI))
+      return false;
+  }
+  return true;
+}
+
 /// Check that there is no dependencies preventing if conversion.
 ///
 /// If instruction uses any values that are defined in the head basic block,
@@ -261,8 +319,17 @@ bool SSAIfConv::canPredicateInstrs(MachineBasicBlock *MBB) {
       return false;
     }
 
-    if (!Predicate.canPredicateInstr(*I))
+    // Check that instruction is predicable
+    if (!TII->isPredicable(*I)) {
+      LLVM_DEBUG(dbgs() << "Isn't predicable: " << *I);
+      return false;
+    }
+
+    // Check that instruction is not already predicated.
+    if (TII->isPredicated(*I) && !TII->canPredicatePredicatedInstr(*I)) {
+      LLVM_DEBUG(dbgs() << "Is already predicated: " << *I);
       return false;
+    }
 
     // Check for any dependencies on Head instructions.
     if (!InstrDependenciesAllowIfConv(&(*I)))
@@ -271,6 +338,24 @@ bool SSAIfConv::canPredicateInstrs(MachineBasicBlock *MBB) {
   return true;
 }
 
+// Apply predicate to all instructions in the machine block.
+void SSAIfConv::PredicateBlock(MachineBasicBlock *MBB, bool ReversePredicate) {
+  auto Condition = Cond;
+  if (ReversePredicate) {
+    bool CanRevCond = !TII->reverseBranchCondition(Condition);
+    assert(CanRevCond && "Reversed predicate is not supported");
+    (void)CanRevCond;
+  }
+  // Terminators don't need to be predicated as they will be removed.
+  for (MachineBasicBlock::iterator I = MBB->begin(),
+                                   E = MBB->getFirstTerminator();
+       I != E; ++I) {
+    if (I->isDebugInstr())
+      continue;
+    TII->PredicateInstruction(*I, Condition);
+  }
+}
+
 /// Find an insertion point in Head for the speculated instructions. The
 /// insertion point must be:
 ///
@@ -349,7 +434,7 @@ bool SSAIfConv::findInsertionPoint() {
 /// canConvertIf - analyze the sub-cfg rooted in MBB, and return true if it is
 /// a potential candidate for if-conversion. Fill out the internal state.
 ///
-bool SSAIfConv::canConvertIf(MachineBasicBlock *MBB) {
+bool SSAIfConv::canConvertIf(MachineBasicBlock *MBB, bool Predicate) {
   Head = MBB;
   TBB = FBB = Tail = nullptr;
 
@@ -389,17 +474,21 @@ bool SSAIfConv::canConvertIf(MachineBasicBlock *MBB) {
                       << printMBBReference(*Tail) << '\n');
   }
 
+  // This is a triangle or a diamond.
+  // Skip if we cannot predicate and there are no phis skip as there must be
+  // side effects that can only be handled with predication.
+  if (!Predicate && (Tail->empty() || !Tail->front().isPHI())) {
+    LLVM_DEBUG(dbgs() << "No phis in tail.\n");
+    return false;
+  }
+
   // The branch we're looking to eliminate must be analyzable.
   Cond.clear();
   if (TII->analyzeBranch(*Head, TBB, FBB, Cond)) {
     LLVM_DEBUG(dbgs() << "Branch not analyzable.\n");
     return false;
   }
 
-  if (!Predicate.canConvertIf(Tail)) {
-    return false;
-  }
-
   // This is weird, probably some sort of degenerate CFG.
   if (!TBB) {
     LLVM_DEBUG(dbgs() << "analyzeBranch didn't find conditional branch.\n");
@@ -447,9 +536,17 @@ bool SSAIfConv::canConvertIf(MachineBasicBlock *MBB) {
   // Check that the conditional instructions can be speculated.
   InsertAfter.clear();
   ClobberedRegUnits.reset();
-  for (MachineBasicBlock *MBB : {TBB, FBB})
-    if (MBB != Tail && !canPredicateInstrs(MBB))
+  if (Predicate) {
+    if (TBB != Tail && !canPredicateInstrs(TBB))
       return false;
+    if (FBB != Tail && !canPredicateInstrs(FBB))
+      return false;
+  } else {
+    if (TBB != Tail && !canSpeculateInstrs(TBB))
+      return false;
+    if (FBB != Tail && !canSpeculateInstrs(FBB))
+      return false;
+  }
 
   // Try to find a valid insertion point for the speculated instructions in the
   // head basic block.
@@ -582,7 +679,8 @@ void SSAIfConv::rewritePHIOperands() {
 ///
 /// Any basic blocks that need to be erased will be added to RemoveBlocks.
 ///
-void SSAIfConv::convertIf(SmallVectorImpl<MachineBasicBlock *> &RemoveBlocks) {
+void SSAIfConv::convertIf(SmallVectorImpl<MachineBasicBlock *> &RemoveBlocks,
+                          bool Predicate) {
   assert(Head && Tail && TBB && FBB && "Call canConvertIf first.");
 
   // Update statistics.
@@ -592,15 +690,16 @@ void SSAIfConv::convertIf(SmallVectorImpl<MachineBasicBlock *> &RemoveBlocks) {
     ++NumDiamondsConv;
 
   // Move all instructions into Head, except for the terminators.
-  for (MachineBasicBlock *MBB : {TBB, FBB}) {
-    if (MBB != Tail) {
-      // reverse the condition for the false bb
-      Predicate.predicateBlock(MBB, Cond, MBB == FBB);
-      Head->splice(InsertionPoint, MBB, MBB->begin(),
-                   MBB->getFirstTerminator());
-    }
+  if (TBB != Tail) {
+    if (Predicate)
+      PredicateBlock(TBB, /*ReversePredicate=*/false);
+    Head->splice(InsertionPoint, TBB, TBB->begin(), TBB->getFirstTerminator());
+  }
+  if (FBB != Tail) {
+    if (Predicate)
+      PredicateBlock(FBB, /*ReversePredicate=*/true);
+    Head->splice(InsertionPoint, FBB, FBB->begin(), FBB->getFirstTerminator());
   }
-
   // Are there extra Tail predecessors?
   bool ExtraPreds = Tail->pred_size() != 2;
   if (ExtraPreds)
@@ -764,46 +863,6 @@ template <typename Remark> Remark &operator<<(Remark &R, Cycles C) {
 }
 } // anonymous namespace
 
-class SpeculateStrategy : public SSAIfConv::PredicationStrategyBase {
-public:
-  bool canConvertIf(MachineBasicBlock *Tail) override {
-    // This is a triangle or a diamond.
-    // Skip if we cannot predicate and there are no phis skip as there must
-    // be side effects that can only be handled with predication.
-    if (Tail->empty() || !Tail->front().isPHI()) {
-      LLVM_DEBUG(dbgs() << "No phis in tail.\n");
-      return false;
-    }
-    return true;
-  }
-
-  bool canPredicateInstr(const MachineInstr &I) override {
-    // Don't speculate loads. Note that it may be possible and desirable to
-    // speculate GOT or constant pool loads that are guaranteed not to trap,
-    // but we don't support that for now.
-    if (I.mayLoad()) {
-      LLVM_DEBUG(dbgs() << "Won't speculate load: " << I);
-      return false;
-    }
-
-    // We never speculate stores, so an AA pointer isn't necessary.
-    bool DontMoveAcrossStore = true;
-    if (!I.isSafeToMove(DontMoveAcrossStore)) {
-      LLVM_DEBUG(dbgs() << "Can't speculate: " << I);
-      return false;
-    }
-    return true;
-  }
-
-  void predicateBlock(MachineBasicBlock *MBB, ArrayRef<MachineOperand> Cond,
-                      bool Reverse)
-      override { /* do nothing, everything is speculatable and it's valid to
-                    move the instructions into the head */
-  }
-
-  ~SpeculateStrategy() override = default;
-};
-
 /// Apply cost model and heuristics to the if-conversion in IfConv.
 /// Return true if the conversion is a good idea.
 ///
@@ -1037,8 +1096,7 @@ bool EarlyIfConverter::runOnMachineFunction(MachineFunction &MF) {
   MinInstr = nullptr;
 
   bool Changed = false;
-  SpeculateStrategy Speculate;
-  SSAIfConv IfConv(Speculate, MF);
+  SSAIfConv IfConv(MF);
 
   // Visit blocks in dominator tree post-order. The post-order enables nested
   // if-conversion in a single pass. The tryConvertIf() function may erase
@@ -1100,48 +1158,6 @@ void EarlyIfPredicator::getAnalysisUsage(AnalysisUsage &AU) const {
   MachineFunctionPass::getAnalysisUsage(AU);
 }
 
-class PredicatorStrategy : public SSAIfConv::PredicationStrategyBase {
-  const TargetInstrInfo *TII = nullptr;
-
-public:
-  PredicatorStrategy(const TargetInstrInfo *TII) : TII(TII) {}
-
-  bool canPredicateInstr(const MachineInstr &I) override {
-    // Check that instruction is predicable
-    if (!TII->isPredicable(I)) {
-      LLVM_DEBUG(dbgs() << "Isn't predicable: " << I);
-      return false;
-    }
-
-    // Check that instruction is not already predicated.
-    if (TII->isPredicated(I) && !TII->canPredicatePredicatedInstr(I)) {
-      LLVM_DEBUG(dbgs() << "Is already predicated: " << I);
-      return false;
-    }
-    return true;
-  }
-
-  void predicateBlock(MachineBasicBlock *MBB, ArrayRef<MachineOperand> Cond,
-                      bool Reverse) override {
-    SmallVector<MachineOperand> Condition(Cond);
-    if (Reverse) {
-      bool CanRevCond = !TII->reverseBranchCondition(Condition);
-      assert(CanRevCond && "Reversed predicate is not supported");
-      (void)CanRevCond;
-    }
-    // Terminators don't need to be predicated as they will be removed.
-    for (MachineBasicBlock::iterator I = MBB->begin(),
-                                     E = MBB->getFirstTerminator();
-         I != E; ++I) {
-      if (I->isDebugInstr())
-        continue;
-      TII->PredicateInstruction(*I, Condition);
-    }
-  }
-
-  ~PredicatorStrategy() override = default;
-};
-
 /// Apply the target heuristic to decide if the transformation is profitable.
 bool EarlyIfPredicator::shouldConvertIf(SSAIfConv &IfConv) {
   auto TrueProbability = MBPI->getEdgeProbability(IfConv.Head, IfConv.TBB);
@@ -1186,10 +1202,11 @@ bool EarlyIfPredicator::shouldConvertIf(SSAIfConv &IfConv) {
 bool EarlyIfPredicator::tryConvertIf(SSAIfConv &IfConv,
                                      MachineBasicBlock *MBB) {
   bool Changed = false;
-  while (IfConv.canConvertIf(MBB) && shouldConvertIf(IfConv)) {
+  while (IfConv.canConvertIf(MBB, /*Predicate=*/true) &&
+         shouldConvertIf(IfConv)) {
     // If-convert MBB and update analyses.
     SmallVector<MachineBasicBlock *, 4> RemoveBlocks;
-    IfConv.convertIf(RemoveBlocks);
+    IfConv.convertIf(RemoveBlocks, /*Predicate=*/true);
     Changed = true;
     updateDomTree(DomTree, IfConv, RemoveBlocks);
     for (MachineBasicBlock *MBB : RemoveBlocks)
@@ -1215,8 +1232,7 @@ bool EarlyIfPredicator::runOnMachineFunction(MachineFunction &MF) {
   MBPI = &getAnalysis<MachineBranchProbabilityInfoWrapperPass>().getMBPI();
 
   bool Changed = false;
-  PredicatorStrategy Predicate(TII);
-  SSAIfConv IfConv(Predicate, MF);
+  SSAIfConv IfConv(MF);
 
   // Visit blocks in dominator tree post-order. The post-order enables nested
   // if-conversion in a single pass. The tryConvertIf() function may erase