[AMDGPU][NFC] Tidy SIOptimizeExecMaskingPreRA for extensibility

[critson]

Summary

Remove duplicate code and move things around to make it easier to
add additional optimisations to the pass.

[critson]

• Rebase

[critson]

• Consistently use Register type.

[foad]

Looks fine to me. More ideas inline.

llvm/lib/Target/AMDGPU/SIOptimizeExecMaskingPreRA.cpp

78–79

Make this a member function so you don't have to pass ExecReg in? Or just inline it? It's so simple now, it hardly seems worth having it as a separate function.

79

Since you're cleaning up, why not just return this expression?

121–122

Return Register?

128

Could just return Register() here and elsewhere.

[critson]

• Address review comments

[critson]

llvm/lib/Target/AMDGPU/SIOptimizeExecMaskingPreRA.cpp

78–79

Inlined.

128

Done, I think I got all the remaining uses.

[rampitec]

LGTM

llvm/lib/Target/AMDGPU/SIOptimizeExecMaskingPreRA.cpp

 
 namespace {
 
 class SIOptimizeExecMaskingPreRA : public MachineFunctionPass {
 private:
   const SIRegisterInfo *TRI;
   const SIInstrInfo *TII;
   MachineRegisterInfo *MRI;
+  LiveIntervals *LIS;
+
+  unsigned AndOpc;
+  unsigned Andn2Opc;
+  Register CondReg;
+  Register ExecReg;
+
+  Register optimizeVcndVcmpPair(MachineBasicBlock &MBB);
 
 public:
   static char ID;
 
   SIOptimizeExecMaskingPreRA() : MachineFunctionPass(ID) {
     initializeSIOptimizeExecMaskingPreRAPass(*PassRegistry::getPassRegistry());
   }
 

 
 char SIOptimizeExecMaskingPreRA::ID = 0;
 
 char &llvm::SIOptimizeExecMaskingPreRAID = SIOptimizeExecMaskingPreRA::ID;
 
 FunctionPass *llvm::createSIOptimizeExecMaskingPreRAPass() {
   return new SIOptimizeExecMaskingPreRA();
 }
 
-static bool isFullExecCopy(const MachineInstr& MI, const GCNSubtarget& ST) {
-  unsigned Exec = ST.isWave32() ? AMDGPU::EXEC_LO : AMDGPU::EXEC;
-
-  if (MI.isFullCopy() && MI.getOperand(1).getReg() == Exec)
-    return true;
-
-  return false;
-}
-
 // See if there is a def between \p AndIdx and \p SelIdx that needs to live
 // beyond \p AndIdx.
 static bool isDefBetween(const LiveRange &LR, SlotIndex AndIdx,
                          SlotIndex SelIdx) {
   LiveQueryResult AndLRQ = LR.Query(AndIdx);
   return (!AndLRQ.isKill() && AndLRQ.valueIn() != LR.Query(SelIdx).valueOut());
 }
 
 // FIXME: Why do we bother trying to handle physical registers here?

 //
 // It is the negation pattern inserted by DAGCombiner::visitBRCOND() in the
 // rebuildSetCC(). We start with S_CBRANCH to avoid exhaustive search, but
 // only 3 first instructions are really needed. S_AND_B64 with exec is a
 // required part of the pattern since V_CNDMASK_B32 writes zeroes for inactive
 // lanes.
 //
 // Returns %cc register on success.
-static unsigned optimizeVcndVcmpPair(MachineBasicBlock &MBB,
-                                     const GCNSubtarget &ST,
-                                     MachineRegisterInfo &MRI,
-                                     LiveIntervals *LIS) {
-  const SIRegisterInfo *TRI = ST.getRegisterInfo();
-  const SIInstrInfo *TII = ST.getInstrInfo();
-  bool Wave32 = ST.isWave32();
-  const unsigned AndOpc = Wave32 ? AMDGPU::S_AND_B32 : AMDGPU::S_AND_B64;
-  const unsigned Andn2Opc = Wave32 ? AMDGPU::S_ANDN2_B32 : AMDGPU::S_ANDN2_B64;
-  const unsigned CondReg = Wave32 ? AMDGPU::VCC_LO : AMDGPU::VCC;
-  const unsigned ExecReg = Wave32 ? AMDGPU::EXEC_LO : AMDGPU::EXEC;
-
+Register
+SIOptimizeExecMaskingPreRA::optimizeVcndVcmpPair(MachineBasicBlock &MBB) {
   auto I = llvm::find_if(MBB.terminators(), [](const MachineInstr &MI) {
                            unsigned Opc = MI.getOpcode();
                            return Opc == AMDGPU::S_CBRANCH_VCCZ ||
                                   Opc == AMDGPU::S_CBRANCH_VCCNZ; });
   if (I == MBB.terminators().end())
-    return AMDGPU::NoRegister;
+    return Register();
 
-  auto *And = TRI->findReachingDef(CondReg, AMDGPU::NoSubRegister,
-                                   *I, MRI, LIS);
+  auto *And =
+      TRI->findReachingDef(CondReg, AMDGPU::NoSubRegister, *I, *MRI, LIS);
   if (!And || And->getOpcode() != AndOpc ||
       !And->getOperand(1).isReg() || !And->getOperand(2).isReg())
-    return AMDGPU::NoRegister;
+    return Register();
 
   MachineOperand *AndCC = &And->getOperand(1);
   Register CmpReg = AndCC->getReg();
   unsigned CmpSubReg = AndCC->getSubReg();
   if (CmpReg == ExecReg) {
     AndCC = &And->getOperand(2);
     CmpReg = AndCC->getReg();
     CmpSubReg = AndCC->getSubReg();
   } else if (And->getOperand(2).getReg() != ExecReg) {
-    return AMDGPU::NoRegister;
+    return Register();
   }
 
-  auto *Cmp = TRI->findReachingDef(CmpReg, CmpSubReg, *And, MRI, LIS);
+  auto *Cmp = TRI->findReachingDef(CmpReg, CmpSubReg, *And, *MRI, LIS);
   if (!Cmp || !(Cmp->getOpcode() == AMDGPU::V_CMP_NE_U32_e32 ||
                 Cmp->getOpcode() == AMDGPU::V_CMP_NE_U32_e64) ||
       Cmp->getParent() != And->getParent())
-    return AMDGPU::NoRegister;
+    return Register();
 
   MachineOperand *Op1 = TII->getNamedOperand(*Cmp, AMDGPU::OpName::src0);
   MachineOperand *Op2 = TII->getNamedOperand(*Cmp, AMDGPU::OpName::src1);
   if (Op1->isImm() && Op2->isReg())
     std::swap(Op1, Op2);
   if (!Op1->isReg() || !Op2->isImm() || Op2->getImm() != 1)
-    return AMDGPU::NoRegister;
+    return Register();
 
   Register SelReg = Op1->getReg();
-  auto *Sel = TRI->findReachingDef(SelReg, Op1->getSubReg(), *Cmp, MRI, LIS);
+  auto *Sel = TRI->findReachingDef(SelReg, Op1->getSubReg(), *Cmp, *MRI, LIS);
   if (!Sel || Sel->getOpcode() != AMDGPU::V_CNDMASK_B32_e64)
-    return AMDGPU::NoRegister;
+    return Register();
 
   if (TII->hasModifiersSet(*Sel, AMDGPU::OpName::src0_modifiers) ||
       TII->hasModifiersSet(*Sel, AMDGPU::OpName::src1_modifiers))
-    return AMDGPU::NoRegister;
+    return Register();
 
   Op1 = TII->getNamedOperand(*Sel, AMDGPU::OpName::src0);
   Op2 = TII->getNamedOperand(*Sel, AMDGPU::OpName::src1);
   MachineOperand *CC = TII->getNamedOperand(*Sel, AMDGPU::OpName::src2);
   if (!Op1->isImm() || !Op2->isImm() || !CC->isReg() ||
       Op1->getImm() != 0 || Op2->getImm() != 1)
-    return AMDGPU::NoRegister;
+    return Register();
 
   Register CCReg = CC->getReg();
 
   // If there was a def between the select and the and, we would need to move it
   // to fold this.
   if (isDefBetween(*TRI, LIS, CCReg, *Sel, *And))
-    return AMDGPU::NoRegister;
+    return Register();
 
   LLVM_DEBUG(dbgs() << "Folding sequence:\n\t" << *Sel << '\t' << *Cmp << '\t'
                     << *And);
 
   LIS->RemoveMachineInstrFromMaps(*And);
   MachineInstr *Andn2 =
       BuildMI(MBB, *And, And->getDebugLoc(), TII->get(Andn2Opc),
               And->getOperand(0).getReg())
           .addReg(ExecReg)
           .addReg(CCReg, getUndefRegState(CC->isUndef()), CC->getSubReg());
   MachineOperand &AndSCC = And->getOperand(3);
   assert(AndSCC.getReg() == AMDGPU::SCC);
   MachineOperand &Andn2SCC = Andn2->getOperand(3);
   assert(Andn2SCC.getReg() == AMDGPU::SCC);
   Andn2SCC.setIsDead(AndSCC.isDead());
   And->eraseFromParent();
   LIS->InsertMachineInstrInMaps(*Andn2);
 
   LLVM_DEBUG(dbgs() << "=>\n\t" << *Andn2 << '\n');
 
   // Try to remove compare. Cmp value should not used in between of cmp
   // and s_and_b64 if VCC or just unused if any other register.
-  if ((CmpReg.isVirtual() && MRI.use_nodbg_empty(CmpReg)) ||
+  if ((CmpReg.isVirtual() && MRI->use_nodbg_empty(CmpReg)) ||
       (CmpReg == CondReg &&
        std::none_of(std::next(Cmp->getIterator()), Andn2->getIterator(),
                     [&](const MachineInstr &MI) {
                       return MI.readsRegister(CondReg, TRI);
                     }))) {
     LLVM_DEBUG(dbgs() << "Erasing: " << *Cmp << '\n');
 
     LIS->RemoveMachineInstrFromMaps(*Cmp);
     Cmp->eraseFromParent();
 
     // Try to remove v_cndmask_b32.
-    if (SelReg.isVirtual() && MRI.use_nodbg_empty(SelReg)) {
+    if (SelReg.isVirtual() && MRI->use_nodbg_empty(SelReg)) {
       LLVM_DEBUG(dbgs() << "Erasing: " << *Sel << '\n');
 
       LIS->RemoveMachineInstrFromMaps(*Sel);
       Sel->eraseFromParent();
     }
   }
 
   return CCReg;
 }
 
 bool SIOptimizeExecMaskingPreRA::runOnMachineFunction(MachineFunction &MF) {
   if (skipFunction(MF.getFunction()))
     return false;
 
   const GCNSubtarget &ST = MF.getSubtarget<GCNSubtarget>();
   TRI = ST.getRegisterInfo();
   TII = ST.getInstrInfo();
   MRI = &MF.getRegInfo();
+  LIS = &getAnalysis<LiveIntervals>();
+
+  const bool Wave32 = ST.isWave32();
+  AndOpc = Wave32 ? AMDGPU::S_AND_B32 : AMDGPU::S_AND_B64;
+  Andn2Opc = Wave32 ? AMDGPU::S_ANDN2_B32 : AMDGPU::S_ANDN2_B64;
+  CondReg = Wave32 ? AMDGPU::VCC_LO : AMDGPU::VCC;
+  ExecReg = Wave32 ? AMDGPU::EXEC_LO : AMDGPU::EXEC;
 
-  MachineRegisterInfo &MRI = MF.getRegInfo();
-  LiveIntervals *LIS = &getAnalysis<LiveIntervals>();
   DenseSet<Register> RecalcRegs({AMDGPU::EXEC_LO, AMDGPU::EXEC_HI});
-  unsigned Exec = ST.isWave32() ? AMDGPU::EXEC_LO : AMDGPU::EXEC;
   bool Changed = false;
 
   for (MachineBasicBlock &MBB : MF) {
 
-    if (unsigned Reg = optimizeVcndVcmpPair(MBB, ST, MRI, LIS)) {
+    if (Register Reg = optimizeVcndVcmpPair(MBB)) {
       RecalcRegs.insert(Reg);
       RecalcRegs.insert(AMDGPU::VCC_LO);
       RecalcRegs.insert(AMDGPU::VCC_HI);
       RecalcRegs.insert(AMDGPU::SCC);
       Changed = true;
     }
 
     // Try to remove unneeded instructions before s_endpgm.
for (MachineBasicBlock &MBB : MF) {
     //
     //    %0:sreg_64 = COPY $exec
     //    %1:sreg_64 = S_AND_B64 %0:sreg_64, %2:sreg_64
     // =>
     //    %1 = S_AND_B64 $exec, %2:sreg_64
     unsigned ScanThreshold = 10;
     for (auto I = MBB.rbegin(), E = MBB.rend(); I != E
          && ScanThreshold--; ++I) {
-      if (!isFullExecCopy(*I, ST))
+      // Continue scanning if this is not a full exec copy
+      if (!(I->isFullCopy() && I->getOperand(1).getReg() == ExecReg))
         continue;
 
       Register SavedExec = I->getOperand(0).getReg();
-      if (SavedExec.isVirtual() && MRI.hasOneNonDBGUse(SavedExec) &&
-          MRI.use_instr_nodbg_begin(SavedExec)->getParent() == I->getParent()) {
+      if (SavedExec.isVirtual() && MRI->hasOneNonDBGUse(SavedExec) &&
+          MRI->use_instr_nodbg_begin(SavedExec)->getParent() ==
+              I->getParent()) {
         LLVM_DEBUG(dbgs() << "Redundant EXEC COPY: " << *I << '\n');
         LIS->RemoveMachineInstrFromMaps(*I);
         I->eraseFromParent();
-        MRI.replaceRegWith(SavedExec, Exec);
+        MRI->replaceRegWith(SavedExec, ExecReg);
         LIS->removeInterval(SavedExec);
         Changed = true;
       }
       break;
     }
   }
 
   if (Changed) {
     for (auto Reg : RecalcRegs) {
       if (Reg.isVirtual()) {
         LIS->removeInterval(Reg);
-        if (!MRI.reg_empty(Reg))
+        if (!MRI->reg_empty(Reg))
           LIS->createAndComputeVirtRegInterval(Reg);
       } else {
         LIS->removeAllRegUnitsForPhysReg(Reg);
       }
     }
   }
 
   return Changed;
 }