[RISCV] Use analyzeBranch in RISCVRedundantCopyElimination.

[craig.topper]

Summary

The existing code was incorrect if we had more than one conditional
branch instruction in a basic block. Though I don't think that will
occur, using analyzeBranch detects that as an unsupported case.

Overall this results in simpler code in RISCVRedundantCopyElimination.

[kito-cheng]

llvm/lib/Target/RISCV/RISCVRedundantCopyElimination.cpp

157

CondBr is gone, maybe we need to update this comment.

[reames]

LGTM w/the comment change already pointed out.

llvm/lib/Target/RISCV/RISCVRedundantCopyElimination.cpp

123

The reserved register check can be pulled out of the loop and become an early return. Not related to this change, just noticed while glancing at the code. Might be worth a separate cleanup change.

[craig.topper]

llvm/lib/Target/RISCV/RISCVRedundantCopyElimination.cpp

151

This line is wrong. It’s a copy of the operand not a reference to the original operand. This doesn’t change the original instruction

[craig.topper]

Use getFirstTerminator to find the CondBr instruction. Add asserts to make sure
it matches what analyzeBranch found.

llvm/lib/Target/RISCV/RISCVRedundantCopyElimination.cpp

 // AArch64RedundantCopyElimination.
 //
 // FIXME: Support compares with constants other than zero? This is harder to
 // do on RISC-V since branches can't have immediates.
 //
 //===----------------------------------------------------------------------===//
 
 #include "RISCV.h"
+#include "RISCVInstrInfo.h"
 #include "llvm/ADT/Statistic.h"
 #include "llvm/CodeGen/MachineFunctionPass.h"
 #include "llvm/CodeGen/MachineRegisterInfo.h"
 #include "llvm/Support/Debug.h"
 
 using namespace llvm;
 
 #define DEBUG_TYPE "riscv-copyelim"
 
 STATISTIC(NumCopiesRemoved, "Number of copies removed.");
 
 namespace {
 class RISCVRedundantCopyElimination : public MachineFunctionPass {
   const MachineRegisterInfo *MRI;
   const TargetRegisterInfo *TRI;
+  const TargetInstrInfo *TII;
 
 public:
   static char ID;
   RISCVRedundantCopyElimination() : MachineFunctionPass(ID) {
     initializeRISCVRedundantCopyEliminationPass(
         *PassRegistry::getPassRegistry());
   }
 

 
 } // end anonymous namespace
 
 char RISCVRedundantCopyElimination::ID = 0;
 
 INITIALIZE_PASS(RISCVRedundantCopyElimination, "riscv-copyelim",
                 "RISCV redundant copy elimination pass", false, false)
 
-static bool guaranteesZeroRegInBlock(const MachineInstr &MI,
-                                     const MachineBasicBlock &MBB) {
-  unsigned Opc = MI.getOpcode();
-  if (Opc == RISCV::BEQ && MI.getOperand(1).getReg() == RISCV::X0 &&
-      &MBB == MI.getOperand(2).getMBB())
+static bool
+guaranteesZeroRegInBlock(MachineBasicBlock &MBB,
+                         const SmallVectorImpl<MachineOperand> &Cond,
+                         MachineBasicBlock *TBB) {
+  assert(Cond.size() == 3 && "Unexpected number of operands");
+  assert(TBB != nullptr && "Expected branch target basic block");
+  auto CC = static_cast<RISCVCC::CondCode>(Cond[0].getImm());
+  if (CC == RISCVCC::COND_EQ && Cond[2].getReg() == RISCV::X0 && TBB == &MBB)
     return true;
-  if (Opc == RISCV::BNE && MI.getOperand(1).getReg() == RISCV::X0 &&
-      &MBB != MI.getOperand(2).getMBB())
+  if (CC == RISCVCC::COND_NE && Cond[2].getReg() == RISCV::X0 && TBB != &MBB)
     return true;
-
   return false;
 }
 
 bool RISCVRedundantCopyElimination::optimizeBlock(MachineBasicBlock &MBB) {
   // Check if the current basic block has a single predecessor.
   if (MBB.pred_size() != 1)
     return false;
 
   // Check if the predecessor has two successors, implying the block ends in a
   // conditional branch.
   MachineBasicBlock *PredMBB = *MBB.pred_begin();
   if (PredMBB->succ_size() != 2)
     return false;
 
-  MachineBasicBlock::iterator CondBr = PredMBB->getLastNonDebugInstr();
-  if (CondBr == PredMBB->end())
+  MachineBasicBlock *TBB = nullptr, *FBB = nullptr;
+  SmallVector<MachineOperand, 3> Cond;
+  if (TII->analyzeBranch(*PredMBB, TBB, FBB, Cond, /*AllowModify*/ false) ||
+      Cond.empty())
     return false;
 
-  while (true) {
-    // If we run out of terminators, give up.
-    if (!CondBr->isTerminator())
-      return false;
-    // If we found a branch with X0, stop searching and try to remove copies.
-    if (guaranteesZeroRegInBlock(*CondBr, MBB))
-      break;
-    // If we reached the beginning of the basic block, give up.
-    if (CondBr == PredMBB->begin())
-      return false;
-    --CondBr;
-  }
+  // Is this a branch with X0?
+  if (!guaranteesZeroRegInBlock(MBB, Cond, TBB))
 
-  Register TargetReg = CondBr->getOperand(0).getReg();
+  Register TargetReg = Cond[1].getReg();
   if (!TargetReg)
     return false;
 
   bool Changed = false;
   MachineBasicBlock::iterator LastChange = MBB.begin();
   // Remove redundant Copy instructions unless TargetReg is modified.
   for (MachineBasicBlock::iterator I = MBB.begin(), E = MBB.end(); I != E;) {
     MachineInstr *MI = &*I;
     ++I;
     if (MI->isCopy() && MI->getOperand(0).isReg() &&
         MI->getOperand(1).isReg()) {
       Register DefReg = MI->getOperand(0).getReg();
       Register SrcReg = MI->getOperand(1).getReg();
 
       if (SrcReg == RISCV::X0 && !MRI->isReserved(DefReg) &&
           TargetReg == DefReg) {
         LLVM_DEBUG(dbgs() << "Remove redundant Copy : ");
         LLVM_DEBUG(MI->print(dbgs()));
 
         MI->eraseFromParent();
         Changed = true;
         LastChange = I;
         ++NumCopiesRemoved;
         continue;
       }
     }
 
     if (MI->modifiesRegister(TargetReg, TRI))
       break;
   }
 
   if (!Changed)
     return false;
 
+  MachineBasicBlock::iterator CondBr = PredMBB->getFirstTerminator();
+  assert((CondBr->getOpcode() == RISCV::BEQ ||
+          CondBr->getOpcode() == RISCV::BNE) &&
+         "Unexpected opcode");
+  assert(CondBr->getOperand(0).getReg() == TargetReg && "Unexpected register");
+
   // Otherwise, we have to fixup the use-def chain, starting with the
   // BEQ/BNE. Conservatively mark as much as we can live.
   CondBr->clearRegisterKills(TargetReg, TRI);
 
   // Add newly used reg to the block's live-in list if it isn't there already.
   if (!MBB.isLiveIn(TargetReg))
     MBB.addLiveIn(TargetReg);
 
   // Clear any kills of TargetReg between CondBr and the last removed COPY.
   for (MachineInstr &MMI : make_range(MBB.begin(), LastChange))
     MMI.clearRegisterKills(TargetReg, TRI);
 
   return true;
 }
 
 bool RISCVRedundantCopyElimination::runOnMachineFunction(MachineFunction &MF) {
   if (skipFunction(MF.getFunction()))
     return false;
 
+  TII = MF.getSubtarget().getInstrInfo();
   TRI = MF.getSubtarget().getRegisterInfo();
   MRI = &MF.getRegInfo();
 
   bool Changed = false;
   for (MachineBasicBlock &MBB : MF)
     Changed |= optimizeBlock(MBB);
 
   return Changed;
 }
 
 FunctionPass *llvm::createRISCVRedundantCopyEliminationPass() {
   return new RISCVRedundantCopyElimination();
 }