fixup! combine with optimizeCondBr

Author: michaelmaitland

llvm/lib/CodeGen/PeepholeOptimizer.cpp

@@ -568,14 +568,9 @@ class PeepholeOptimizerLegacy : public MachineFunctionPass {
   bool runOnMachineFunction(MachineFunction &MF) override;
 
   void getAnalysisUsage(AnalysisUsage &AU) const override {
-    AU.setPreservesCFG();
     MachineFunctionPass::getAnalysisUsage(AU);
     AU.addRequired<MachineLoopInfoWrapperPass>();
-    AU.addPreserved<MachineLoopInfoWrapperPass>();
-    if (Aggressive) {
-      AU.addRequired<MachineDominatorTreeWrapperPass>();
-      AU.addPreserved<MachineDominatorTreeWrapperPass>();
-    }
+    AU.addRequired<MachineDominatorTreeWrapperPass>();
   }
 
   MachineFunctionProperties getRequiredProperties() const override {
@@ -1652,27 +1647,20 @@ PreservedAnalyses
 PeepholeOptimizerPass::run(MachineFunction &MF,
                            MachineFunctionAnalysisManager &MFAM) {
   MFPropsModifier _(*this, MF);
-  auto *DT =
-      Aggressive ? &MFAM.getResult<MachineDominatorTreeAnalysis>(MF) : nullptr;
+  auto *DT = &MFAM.getResult<MachineDominatorTreeAnalysis>(MF);
   auto *MLI = &MFAM.getResult<MachineLoopAnalysis>(MF);
   PeepholeOptimizer Impl(DT, MLI);
   bool Changed = Impl.run(MF);
   if (!Changed)
     return PreservedAnalyses::all();
 
-  auto PA = getMachineFunctionPassPreservedAnalyses();
-  PA.preserve<MachineDominatorTreeAnalysis>();
-  PA.preserve<MachineLoopAnalysis>();
-  PA.preserveSet<CFGAnalyses>();
-  return PA;
+  return getMachineFunctionPassPreservedAnalyses();
 }
 
 bool PeepholeOptimizerLegacy::runOnMachineFunction(MachineFunction &MF) {
   if (skipFunction(MF.getFunction()))
     return false;
-  auto *DT = Aggressive
-                 ? &getAnalysis<MachineDominatorTreeWrapperPass>().getDomTree()
-                 : nullptr;
+  auto *DT = &getAnalysis<MachineDominatorTreeWrapperPass>().getDomTree();
   auto *MLI = &getAnalysis<MachineLoopInfoWrapperPass>().getLI();
   PeepholeOptimizer Impl(DT, MLI);
   return Impl.run(MF);
@@ -1793,6 +1781,20 @@ bool PeepholeOptimizer::run(MachineFunction &MF) {
       }
 
       if (MI->isConditionalBranch() && optimizeCondBranch(*MI)) {
+        // optimizeCondBranch might have converted a conditional branch to
+        // an unconditional branch. If there is a branch instruction after it,
+        // delete it.
+        MachineInstr *NewBr = &*std::prev(MII);
+        if (NewBr->isUnconditionalBranch()) {
+          if (MII != MBB.end()) {
+            MachineInstr *Dead = &*MII;
+            ++MII;
+            MachineBasicBlock *DeadDest = TII->getBranchDestBlock(*Dead);
+            if (TII->getBranchDestBlock(*NewBr)!= DeadDest)
+              DT->deleteEdge(&MBB, DeadDest);
+            Dead->eraseFromParent();
+          }
+        }
         Changed = true;
         continue;
       }

llvm/lib/Target/RISCV/RISCVInstrInfo.cpp

@@ -1005,109 +1005,6 @@ RISCVCC::CondCode RISCVCC::getOppositeBranchCondition(RISCVCC::CondCode CC) {
   }
 }
 
-// Return true if MO definitely contains the value one.
-static bool isOne(MachineOperand &MO) {
-  if (MO.isImm() && MO.getImm() == 1)
-    return true;
-
-  if (!MO.isReg() || !MO.getReg().isVirtual())
-    return false;
-
-  MachineRegisterInfo &MRI =
-      MO.getParent()->getParent()->getParent()->getRegInfo();
-  MachineInstr *DefMI = MRI.getUniqueVRegDef(MO.getReg());
-  if (!DefMI)
-    return false;
-
-  // For now, just check the canonical one value.
-  if (DefMI->getOpcode() == RISCV::ADDI &&
-      DefMI->getOperand(1).getReg() == RISCV::X0 &&
-      DefMI->getOperand(2).getImm() == 1)
-    return true;
-
-  return false;
-}
-
-// Return true if MO definitely contains the value zero.
-static bool isZero(MachineOperand &MO) {
-  if (MO.isImm() && MO.getImm() == 0)
-    return true;
-  if (MO.isReg() && MO.getReg() == RISCV::X0)
-    return true;
-  return false;
-}
-
-bool RISCVInstrInfo::trySimplifyCondBr(
-    MachineBasicBlock &MBB, MachineBasicBlock *TBB, MachineBasicBlock *FBB,
-    SmallVectorImpl<MachineOperand> &Cond) const {
-
-  if (!TBB || Cond.size() != 3)
-    return false;
-
-  RISCVCC::CondCode CC = static_cast<RISCVCC::CondCode>(Cond[0].getImm());
-  auto LHS = Cond[1];
-  auto RHS = Cond[2];
-
-  MachineBasicBlock *Folded = nullptr;
-  switch (CC) {
-  default:
-    // TODO: Implement for more CCs
-    return false;
-  case RISCVCC::COND_EQ: {
-    // We can statically evaluate that we take the first branch
-    if ((isZero(LHS) && isZero(RHS)) || (isOne(LHS) && isOne(RHS))) {
-      Folded = TBB;
-      break;
-    }
-    // We can statically evaluate that we take the second branch
-    if ((isZero(LHS) && isOne(RHS)) || (isOne(LHS) && isZero(RHS))) {
-      Folded = FBB;
-      break;
-    }
-    return false;
-  }
-  case RISCVCC::COND_NE: {
-    // We can statically evaluate that we take the first branch
-    if ((isOne(LHS) && isZero(RHS)) || (isZero(LHS) && isOne(RHS))) {
-      Folded = TBB;
-      break;
-    }
-    // We can statically evaluate that we take the second branch
-    if ((isZero(LHS) && isZero(RHS)) || (isOne(LHS) && isOne(RHS))) {
-      Folded = FBB;
-      break;
-    }
-    return false;
-  }
-  }
-
-  // At this point, its legal to optimize.
-  removeBranch(MBB);
-  Cond.clear();
-
-  // Only need to insert a branch if we're not falling through.
-  if (Folded) {
-    DebugLoc DL = MBB.findBranchDebugLoc();
-    insertBranch(MBB, Folded, nullptr, {}, DL);
-  }
-
-  // Update the successors. Remove them all and add back the correct one.
-  while (!MBB.succ_empty())
-    MBB.removeSuccessor(MBB.succ_end() - 1);
-
-  // If it's a fallthrough, we need to figure out where MBB is going.
-  if (!Folded) {
-    MachineFunction::iterator Fallthrough = ++MBB.getIterator();
-    if (Fallthrough != MBB.getParent()->end())
-      MBB.addSuccessor(&*Fallthrough);
-  } else
-    MBB.addSuccessor(Folded);
-
-  TBB = Folded;
-  FBB = nullptr;
-  return true;
-}
-
 bool RISCVInstrInfo::analyzeBranch(MachineBasicBlock &MBB,
                                    MachineBasicBlock *&TBB,
                                    MachineBasicBlock *&FBB,
@@ -1165,9 +1062,12 @@ bool RISCVInstrInfo::analyzeBranch(MachineBasicBlock &MBB,
   // Handle a single conditional branch.
   if (NumTerminators == 1 && I->getDesc().isConditionalBranch()) {
     parseCondBranch(*I, TBB, Cond);
-    // Try to fold the branch of the conditional branch into a the fallthru.
-    if (AllowModify)
-      trySimplifyCondBr(MBB, TBB, FBB, Cond);
+    // Try and optimize the conditional branch.
+    if (AllowModify) {
+      optimizeCondBranch(*I);
+      // The branch might have changed, reanalyze it.
+      return analyzeBranch(MBB, TBB, FBB, Cond, false);
+    }
     return false;
   }
 
@@ -1176,10 +1076,14 @@ bool RISCVInstrInfo::analyzeBranch(MachineBasicBlock &MBB,
       I->getDesc().isUnconditionalBranch()) {
     parseCondBranch(*std::prev(I), TBB, Cond);
     FBB = getBranchDestBlock(*I);
-    // Try to fold the branch of the conditional branch into an unconditional
-    // branch.
-    if (AllowModify)
-      trySimplifyCondBr(MBB, TBB, FBB, Cond);
+    // Try and optimize the pair.
+    if (AllowModify) {
+      if (optimizeCondBranch(*std::prev(I)))
+        I->eraseFromParent();
+
+      // The branch might have changed, reanalyze it.
+      return analyzeBranch(MBB, TBB, FBB, Cond, false);
+    }
     return false;
   }
 
@@ -1335,7 +1239,8 @@ bool RISCVInstrInfo::reverseBranchCondition(
 
 bool RISCVInstrInfo::optimizeCondBranch(MachineInstr &MI) const {
   MachineBasicBlock *MBB = MI.getParent();
-  MachineRegisterInfo &MRI = MBB->getParent()->getRegInfo();
+  if (!MBB)
+    return false;
 
   MachineBasicBlock *TBB, *FBB;
   SmallVector<MachineOperand, 3> Cond;
@@ -1345,8 +1250,98 @@ bool RISCVInstrInfo::optimizeCondBranch(MachineInstr &MI) const {
   RISCVCC::CondCode CC = static_cast<RISCVCC::CondCode>(Cond[0].getImm());
   assert(CC != RISCVCC::COND_INVALID);
 
-  if (CC == RISCVCC::COND_EQ || CC == RISCVCC::COND_NE)
+  // Right now we only care about LI (i.e. ADDI x0, imm)
+  auto isLoadImm = [](const MachineInstr *MI, int64_t &Imm) -> bool {
+    if (MI->getOpcode() == RISCV::ADDI && MI->getOperand(1).isReg() &&
+        MI->getOperand(1).getReg() == RISCV::X0) {
+      Imm = MI->getOperand(2).getImm();
+      return true;
+    }
     return false;
+  };
+
+  MachineRegisterInfo &MRI = MBB->getParent()->getRegInfo();
+  // Either a load from immediate instruction or X0.
+  auto isFromLoadImm = [&](const MachineOperand &Op, int64_t &Imm) -> bool {
+    if (!Op.isReg())
+      return false;
+    Register Reg = Op.getReg();
+    if (Reg == RISCV::X0) {
+      Imm = 0;
+      return true;
+    }
+    return Reg.isVirtual() && isLoadImm(MRI.getVRegDef(Reg), Imm);
+  };
+
+  // Try and convert a conditional branch that can be evaluated statically
+  // into an unconditional branch.
+  MachineBasicBlock *Folded = nullptr;
+  int64_t C0, C1;
+  if (isFromLoadImm(Cond[1], C0) && isFromLoadImm(Cond[2], C1)) {
+    switch (CC) {
+    default:
+      // TODO: Implement for more CCs
+      break;
+    case RISCVCC::COND_EQ: {
+      Folded = (C0 == C1) ? TBB : FBB;
+      break;
+    }
+    case RISCVCC::COND_NE: {
+      Folded = (C0 != C1) ? TBB : FBB;
+      break;
+    }
+    case RISCVCC::COND_LT: {
+      Folded = (C0 < C1) ? TBB : FBB;
+      break;
+    }
+    case RISCVCC::COND_GE: {
+      Folded = (C0 >= C1) ? TBB : FBB;
+      break;
+    }
+    case RISCVCC::COND_LTU: {
+      Folded = ((uint64_t)C0 < (uint64_t)C1) ? TBB : FBB;
+      break;
+    }
+    case RISCVCC::COND_GEU: {
+      Folded = ((uint64_t)C0 >= (uint64_t)C1) ? TBB : FBB;
+      break;
+    }
+    }
+
+    // Do the conversion
+    // Build the new unconditional branch
+    DebugLoc DL = MBB->findBranchDebugLoc();
+    if (Folded) {
+      BuildMI(*MBB, MI, DL, get(RISCV::PseudoBR)).addMBB(Folded);
+    } else {
+      MachineFunction::iterator Fallthrough = ++MBB->getIterator();
+      if (Fallthrough == MBB->getParent()->end())
+        return false;
+      BuildMI(*MBB, MI, DL, get(RISCV::PseudoBR)).addMBB(&*Fallthrough);
+    }
+
+    // Update successors of MBB.
+    if (Folded == TBB) {
+      // If we're taking TBB, then the succ to delete is the fallthrough (if
+      // it was a succ in the first place), or its the MBB from the
+      // unconditional branch.
+      if (!FBB) {
+        MachineFunction::iterator Fallthrough = ++MBB->getIterator();
+        if (Fallthrough != MBB->getParent()->end() &&
+            MBB->isSuccessor(&*Fallthrough))
+          MBB->removeSuccessor(&*Fallthrough, true);
+      } else {
+        MBB->removeSuccessor(FBB, true);
+      }
+    } else if (Folded == FBB) {
+      // If we're taking the fallthrough or unconditional branch, then the
+      // succ to remove is the one from the conditional branch.
+      MBB->removeSuccessor(TBB, true);
+    }
+
+    MI.eraseFromParent();
+    return true;
+  }
 
   // For two constants C0 and C1 from
   // ```
@@ -1365,24 +1360,6 @@ bool RISCVInstrInfo::optimizeCondBranch(MachineInstr &MI) const {
   //
   // To make sure this optimization is really beneficial, we only
   // optimize for cases where Y had only one use (i.e. only used by the branch).
-
-  // Right now we only care about LI (i.e. ADDI x0, imm)
-  auto isLoadImm = [](const MachineInstr *MI, int64_t &Imm) -> bool {
-    if (MI->getOpcode() == RISCV::ADDI && MI->getOperand(1).isReg() &&
-        MI->getOperand(1).getReg() == RISCV::X0) {
-      Imm = MI->getOperand(2).getImm();
-      return true;
-    }
-    return false;
-  };
-  // Either a load from immediate instruction or X0.
-  auto isFromLoadImm = [&](const MachineOperand &Op, int64_t &Imm) -> bool {
-    if (!Op.isReg())
-      return false;
-    Register Reg = Op.getReg();
-    return Reg.isVirtual() && isLoadImm(MRI.getVRegDef(Reg), Imm);
-  };
-
   MachineOperand &LHS = MI.getOperand(0);
   MachineOperand &RHS = MI.getOperand(1);
   // Try to find the register for constant Z; return
@@ -1401,7 +1378,6 @@ bool RISCVInstrInfo::optimizeCondBranch(MachineInstr &MI) const {
   };
 
   bool Modify = false;
-  int64_t C0;
   if (isFromLoadImm(LHS, C0) && MRI.hasOneUse(LHS.getReg())) {
     // Might be case 1.
     // Signed integer overflow is UB. (UINT64_MAX is bigger so we don't need

llvm/lib/Target/RISCV/RISCVInstrInfo.h

@@ -319,26 +319,6 @@ class RISCVInstrInfo : public RISCVGenInstrInfo {
                                const MachineInstr &MI2) const;
   bool hasReassociableVectorSibling(const MachineInstr &Inst,
                                     bool &Commuted) const;
-  /// Return true if the branch represented by the conditional branch with
-  /// components TBB, FBB, and CurCond was folded into an unconditional branch.
-  ///
-  /// If FBB is nullptr, then the the input represents a conditional branch with
-  /// a fallthrough.
-  ///
-  /// For example:
-  /// BRCOND EQ 0, 0, BB1
-  /// BR BB2
-  ///
-  /// can be simplified to BR BB1 since 0 == 0 statically. On the other hand,
-  ///
-  ///
-  /// BRCOND EQ 0, 1, BB1
-  /// BR BB2
-  ///
-  /// can be simplified to BR BB2 because 0 != 1 statically.
-  bool trySimplifyCondBr(MachineBasicBlock &MBB, MachineBasicBlock *TBB,
-                         MachineBasicBlock *FBB,
-                         SmallVectorImpl<MachineOperand> &Cond) const;
 };
 
 namespace RISCV {

llvm/test/CodeGen/AArch64/O3-pipeline.ll

@@ -158,6 +158,7 @@
 ; CHECK-NEXT:       MachinePostDominator Tree Construction
 ; CHECK-NEXT:       Machine Cycle Info Analysis
 ; CHECK-NEXT:       Machine code sinking
+; CHECK-NEXT:       MachineDominator Tree Construction
 ; CHECK-NEXT:       Peephole Optimizations
 ; CHECK-NEXT:       Remove dead machine instructions
 ; CHECK-NEXT:       AArch64 MI Peephole Optimization pass

llvm/test/CodeGen/RISCV/O3-pipeline.ll

@@ -115,6 +115,7 @@
 ; CHECK-NEXT:       MachinePostDominator Tree Construction
 ; CHECK-NEXT:       Machine Cycle Info Analysis
 ; CHECK-NEXT:       Machine code sinking
+; CHECK-NEXT:       MachineDominator Tree Construction
 ; CHECK-NEXT:       Peephole Optimizations
 ; CHECK-NEXT:       Remove dead machine instructions
 ; CHECK-NEXT:       Machine Trace Metrics