[AMDGPU] Refactor DPPCombine

[Joe_Nash]

Summary

NFC. Extract IsShrinkable into a helper function, and
make Subtarget a member variable.

[rampitec]

LGTM with a nit.

llvm/lib/Target/AMDGPU/GCNDPPCombine.cpp

57

You do not need '= nullptr' here, it is initialized in runOnMachineFunction anyway.

[Joe_Nash]

remove null initialization and fix clang-format

llvm/lib/Target/AMDGPU/GCNDPPCombine.cpp

 
 STATISTIC(NumDPPMovsCombined, "Number of DPP moves combined.");
 
 namespace {
 
 class GCNDPPCombine : public MachineFunctionPass {
   MachineRegisterInfo *MRI;
   const SIInstrInfo *TII;
+  const GCNSubtarget *ST;
 
   using RegSubRegPair = TargetInstrInfo::RegSubRegPair;
 
   MachineOperand *getOldOpndValue(MachineOperand &OldOpnd) const;
 
-  MachineInstr *createDPPInst(MachineInstr &OrigMI,
-                              MachineInstr &MovMI,
+  MachineInstr *createDPPInst(MachineInstr &OrigMI, MachineInstr &MovMI,
                               RegSubRegPair CombOldVGPR,
-                              MachineOperand *OldOpnd,
-                              bool CombBCZ) const;
+                              MachineOperand *OldOpnd, bool CombBCZ,
+                              bool IsShrinkable) const;
 
-  MachineInstr *createDPPInst(MachineInstr &OrigMI,
-                              MachineInstr &MovMI,
-                              RegSubRegPair CombOldVGPR,
-                              bool CombBCZ) const;
+  MachineInstr *createDPPInst(MachineInstr &OrigMI, MachineInstr &MovMI,
+                              RegSubRegPair CombOldVGPR, bool CombBCZ,
+                              bool IsShrinkable) const;
 
   bool hasNoImmOrEqual(MachineInstr &MI,
                        unsigned OpndName,
                        int64_t Value,
                        int64_t Mask = -1) const;
 
   bool combineDPPMov(MachineInstr &MI) const;
 
public:
   }
 
   MachineFunctionProperties getRequiredProperties() const override {
     return MachineFunctionProperties()
       .set(MachineFunctionProperties::Property::IsSSA);
   }
 
 private:
-  int getDPPOp(unsigned Op) const;
+  int getDPPOp(unsigned Op, bool IsShrinkable) const;
+  bool isShrinkable(MachineInstr &OrigMI, unsigned OrigOp) const;
 };
 
 } // end anonymous namespace
 
 INITIALIZE_PASS(GCNDPPCombine, DEBUG_TYPE, "GCN DPP Combine", false, false)
 
 char GCNDPPCombine::ID = 0;
 
 char &llvm::GCNDPPCombineID = GCNDPPCombine::ID;
 
 FunctionPass *llvm::createGCNDPPCombinePass() {
   return new GCNDPPCombine();
 }
 
-int GCNDPPCombine::getDPPOp(unsigned Op) const {
+bool GCNDPPCombine::isShrinkable(MachineInstr &OrigMI, unsigned OrigOp) const {
+  if (!TII->isVOP3(OrigOp)) {
+    return false;
+  }
+  if (!TII->hasVALU32BitEncoding(OrigOp)) {
+    LLVM_DEBUG(dbgs() << "  Inst hasn't e32 equivalent\n");
+    return false;
+  }
+  // check if other than abs|neg modifiers are set (opsel for example)
+  const int64_t Mask = ~(SISrcMods::ABS | SISrcMods::NEG);
+  if (!hasNoImmOrEqual(OrigMI, AMDGPU::OpName::src0_modifiers, 0, Mask) ||
+      !hasNoImmOrEqual(OrigMI, AMDGPU::OpName::src1_modifiers, 0, Mask) ||
+      !hasNoImmOrEqual(OrigMI, AMDGPU::OpName::clamp, 0) ||
+      !hasNoImmOrEqual(OrigMI, AMDGPU::OpName::omod, 0)) {
+    LLVM_DEBUG(dbgs() << "  Inst has non-default modifiers\n");
+    return false;
+  }
+  return true;
+}
+
+int GCNDPPCombine::getDPPOp(unsigned Op, bool IsShrinkable) const {
   auto DPP32 = AMDGPU::getDPPOp32(Op);
-  if (DPP32 == -1) {
+  if (IsShrinkable) {
+    assert(DPP32 == -1);
     auto E32 = AMDGPU::getVOPe32(Op);
-    DPP32 = (E32 == -1)? -1 : AMDGPU::getDPPOp32(E32);
+    DPP32 = (E32 == -1) ? -1 : AMDGPU::getDPPOp32(E32);
   }
   return (DPP32 == -1 || TII->pseudoToMCOpcode(DPP32) == -1) ? -1 : DPP32;
 }
 
 // tracks the register operand definition and returns:
 //   1. immediate operand used to initialize the register if found
 //   2. nullptr if the register operand is undef
 //   3. the operand itself otherwise
MachineOperand *GCNDPPCombine::getOldOpndValue(MachineOperand &OldOpnd) const {
   }
   }
   return &OldOpnd;
 }
 
 MachineInstr *GCNDPPCombine::createDPPInst(MachineInstr &OrigMI,
                                            MachineInstr &MovMI,
                                            RegSubRegPair CombOldVGPR,
-                                           bool CombBCZ) const {
+                                           bool CombBCZ,
+                                           bool IsShrinkable) const {
   assert(MovMI.getOpcode() == AMDGPU::V_MOV_B32_dpp ||
          MovMI.getOpcode() == AMDGPU::V_MOV_B64_DPP_PSEUDO);
 
   auto OrigOp = OrigMI.getOpcode();
-  auto DPPOp = getDPPOp(OrigOp);
+  auto DPPOp = getDPPOp(OrigOp, IsShrinkable);
   if (DPPOp == -1) {
     LLVM_DEBUG(dbgs() << "  failed: no DPP opcode\n");
     return nullptr;
   }
 
   auto DPPInst = BuildMI(*OrigMI.getParent(), OrigMI,
                          OrigMI.getDebugLoc(), TII->get(DPPOp))
     .setMIFlags(OrigMI.getFlags());
static bool isIdentityValue(unsigned OrigMIOp, MachineOperand *OldOpnd) {
   case AMDGPU::V_MUL_U32_U24_e64:
     if (OldOpnd->getImm() == 1)
       return true;
     break;
   }
   return false;
 }
 
-MachineInstr *GCNDPPCombine::createDPPInst(MachineInstr &OrigMI,
-                                           MachineInstr &MovMI,
-                                           RegSubRegPair CombOldVGPR,
-                                           MachineOperand *OldOpndValue,
-                                           bool CombBCZ) const {
+MachineInstr *GCNDPPCombine::createDPPInst(
+    MachineInstr &OrigMI, MachineInstr &MovMI, RegSubRegPair CombOldVGPR,
+    MachineOperand *OldOpndValue, bool CombBCZ, bool IsShrinkable) const {
   assert(CombOldVGPR.Reg);
   if (!CombBCZ && OldOpndValue && OldOpndValue->isImm()) {
     auto *Src1 = TII->getNamedOperand(OrigMI, AMDGPU::OpName::src1);
     if (!Src1 || !Src1->isReg()) {
       LLVM_DEBUG(dbgs() << "  failed: no src1 or it isn't a register\n");
       return nullptr;
     }
     if (!isIdentityValue(OrigMI.getOpcode(), OldOpndValue)) {
       LLVM_DEBUG(dbgs() << "  failed: old immediate isn't an identity\n");
       return nullptr;
     }
     CombOldVGPR = getRegSubRegPair(*Src1);
     auto MovDst = TII->getNamedOperand(MovMI, AMDGPU::OpName::vdst);
     const TargetRegisterClass *RC = MRI->getRegClass(MovDst->getReg());
     if (!isOfRegClass(CombOldVGPR, *RC, *MRI)) {
       LLVM_DEBUG(dbgs() << "  failed: src1 has wrong register class\n");
       return nullptr;
     }
   }
-  return createDPPInst(OrigMI, MovMI, CombOldVGPR, CombBCZ);
+  return createDPPInst(OrigMI, MovMI, CombOldVGPR, CombBCZ, IsShrinkable);
 }
 
 // returns true if MI doesn't have OpndName immediate operand or the
 // operand has Value
 bool GCNDPPCombine::hasNoImmOrEqual(MachineInstr &MI, unsigned OpndName,
                                     int64_t Value, int64_t Mask) const {
   auto *Imm = TII->getNamedOperand(MI, OpndName);
   if (!Imm)
if (OrigOp == AMDGPU::REG_SEQUENCE) {
       for (auto &Op : MRI->use_nodbg_operands(FwdReg)) {
         if (Op.getSubReg() == FwdSubReg)
           Uses.push_back(&Op);
       }
       RegSeqWithOpNos[&OrigMI].push_back(OpNo);
       continue;
     }
 
-    if (TII->isVOP3(OrigOp)) {
-      if (!TII->hasVALU32BitEncoding(OrigOp)) {
-        LLVM_DEBUG(dbgs() << "  failed: VOP3 hasn't e32 equivalent\n");
-        break;
-      }
-      // check if other than abs|neg modifiers are set (opsel for example)
-      const int64_t Mask = ~(SISrcMods::ABS | SISrcMods::NEG);
-      if (!hasNoImmOrEqual(OrigMI, AMDGPU::OpName::src0_modifiers, 0, Mask) ||
-          !hasNoImmOrEqual(OrigMI, AMDGPU::OpName::src1_modifiers, 0, Mask) ||
-          !hasNoImmOrEqual(OrigMI, AMDGPU::OpName::clamp, 0) ||
-          !hasNoImmOrEqual(OrigMI, AMDGPU::OpName::omod, 0)) {
-        LLVM_DEBUG(dbgs() << "  failed: VOP3 has non-default modifiers\n");
-        break;
-      }
-    } else if (!TII->isVOP1(OrigOp) && !TII->isVOP2(OrigOp)) {
+    bool IsShrinkable = isShrinkable(OrigMI, OrigOp);
+    if (!(IsShrinkable || TII->isVOP1(OrigOp) || TII->isVOP2(OrigOp))) {
       LLVM_DEBUG(dbgs() << "  failed: not VOP1/2/3\n");
       break;
     }
 
     auto *Src0 = TII->getNamedOperand(OrigMI, AMDGPU::OpName::src0);
     auto *Src1 = TII->getNamedOperand(OrigMI, AMDGPU::OpName::src1);
     if (Use != Src0 && !(Use == Src1 && OrigMI.isCommutable())) { // [1]
       LLVM_DEBUG(dbgs() << "  failed: no suitable operands\n");
       break;
     }
 
     assert(Src0 && "Src1 without Src0?");
     if (Src1 && Src1->isIdenticalTo(*Src0)) {
       assert(Src1->isReg());
       LLVM_DEBUG(
           dbgs()
           << "  " << OrigMI
           << "  failed: DPP register is used more than once per instruction\n");
       break;
     }
 
     LLVM_DEBUG(dbgs() << "  combining: " << OrigMI);
     if (Use == Src0) {
       if (auto *DPPInst = createDPPInst(OrigMI, MovMI, CombOldVGPR,
-                                        OldOpndValue, CombBCZ)) {
+                                        OldOpndValue, CombBCZ, IsShrinkable)) {
         DPPMIs.push_back(DPPInst);
         Rollback = false;
       }
     } else {
       assert(Use == Src1 && OrigMI.isCommutable()); // by check [1]
       auto *BB = OrigMI.getParent();
       auto *NewMI = BB->getParent()->CloneMachineInstr(&OrigMI);
       BB->insert(OrigMI, NewMI);
       if (TII->commuteInstruction(*NewMI)) {
         LLVM_DEBUG(dbgs() << "  commuted:  " << *NewMI);
-        if (auto *DPPInst = createDPPInst(*NewMI, MovMI, CombOldVGPR,
-                                          OldOpndValue, CombBCZ)) {
+        if (auto *DPPInst =
+                createDPPInst(*NewMI, MovMI, CombOldVGPR, OldOpndValue, CombBCZ,
+                              IsShrinkable)) {
           DPPMIs.push_back(DPPInst);
           Rollback = false;
         }
       } else
         LLVM_DEBUG(dbgs() << "  failed: cannot be commuted\n");
       NewMI->eraseFromParent();
     }
     if (Rollback)
for (auto &S : RegSeqWithOpNos) {
         S.first->getOperand(S.second.pop_back_val()).setIsUndef(true);
     }
   }
 
   return !Rollback;
 }
 
 bool GCNDPPCombine::runOnMachineFunction(MachineFunction &MF) {
-  auto &ST = MF.getSubtarget<GCNSubtarget>();
-  if (!ST.hasDPP() || skipFunction(MF.getFunction()))
+  ST = &MF.getSubtarget<GCNSubtarget>();
+  if (!ST->hasDPP() || skipFunction(MF.getFunction()))
     return false;
 
   MRI = &MF.getRegInfo();
-  TII = ST.getInstrInfo();
+  TII = ST->getInstrInfo();
 
   bool Changed = false;
   for (auto &MBB : MF) {
     for (auto I = MBB.rbegin(), E = MBB.rend(); I != E;) {
       auto &MI = *I++;
       if (MI.getOpcode() == AMDGPU::V_MOV_B32_dpp && combineDPPMov(MI)) {
         Changed = true;
         ++NumDPPMovsCombined;
       } else if (MI.getOpcode() == AMDGPU::V_MOV_B64_DPP_PSEUDO) {
-        if (ST.has64BitDPP() && combineDPPMov(MI)) {
+        if (ST->has64BitDPP() && combineDPPMov(MI)) {
           Changed = true;
           ++NumDPPMovsCombined;
         } else {
           auto Split = TII->expandMovDPP64(MI);
           for (auto M : { Split.first, Split.second }) {
             if (M && combineDPPMov(*M))
               ++NumDPPMovsCombined;
           }
           Changed = true;
         }
       }
     }
   }
   return Changed;
 }