[SIFoldOperands] Small code cleanups, NFC.

[Pierre-vh]

Summary

I've been trying to understand the backend better and decided to read the code of this pass.
While doing so, I noticed parts that could be refactored to be a tiny bit clearer.
I tried to keep the changes minimal, a non-exhaustive list of changes is:

• Stylistic changes to better fit LLVM's coding style
• Removing dead/useless functions (e.g. FoldCandidate had getters, but it's a public struct!)
  • Saving regs/opcodes in variables if they're going to be used multiple times in the same condition

[foad]

Looks fine, just some nits inline.

llvm/lib/Target/AMDGPU/SIFoldOperands.cpp

191

Move this down one line?

691

I think the rule is to keep the braces if the body is more than one physical line, even if it's a single logical line.

1009

Probably does not deserve to be a doxygen (three-slash) comment now.

llvm/lib/Target/AMDGPU/SIFoldOperands.cpp

struct FoldCandidate {
   }
 
   bool isReg() const {
     return Kind == MachineOperand::MO_Register;
   }
 
   bool isGlobal() const { return Kind == MachineOperand::MO_GlobalAddress; }
 
-  bool isCommuted() const {
-    return Commuted;
-  }
-
-  bool needsShrink() const {
-    return ShrinkOpcode != -1;
-  }
-
-  int getShrinkOpcode() const {
-    return ShrinkOpcode;
-  }
+  bool needsShrink() const { return ShrinkOpcode != -1; }
 };
 
 class SIFoldOperands : public MachineFunctionPass {
 public:
   static char ID;
   MachineRegisterInfo *MRI;
   const SIInstrInfo *TII;
   const SIRegisterInfo *TRI;

 
 // TODO: Add heuristic that the frame index might not fit in the addressing mode
 // immediate offset to avoid materializing in loops.
 bool SIFoldOperands::frameIndexMayFold(const MachineInstr &UseMI, int OpNo,
                                        const MachineOperand &OpToFold) const {
   if (!OpToFold.isFI())
     return false;
 
+  const unsigned Opc = UseMI.getOpcode();
   if (TII->isMUBUF(UseMI))
-    return OpNo == AMDGPU::getNamedOperandIdx(UseMI.getOpcode(),
-                                              AMDGPU::OpName::vaddr);
+    return OpNo == AMDGPU::getNamedOperandIdx(Opc, AMDGPU::OpName::vaddr);
   if (!TII->isFLATScratch(UseMI))
     return false;
 
-  int SIdx = AMDGPU::getNamedOperandIdx(UseMI.getOpcode(),
-                                        AMDGPU::OpName::saddr);
+  int SIdx = AMDGPU::getNamedOperandIdx(Opc, AMDGPU::OpName::saddr);
   if (OpNo == SIdx)
     return true;
 
-  int VIdx = AMDGPU::getNamedOperandIdx(UseMI.getOpcode(),
-                                        AMDGPU::OpName::vaddr);
+  int VIdx = AMDGPU::getNamedOperandIdx(Opc, AMDGPU::OpName::vaddr);
   return OpNo == VIdx && SIdx == -1;
 }
 
 FunctionPass *llvm::createSIFoldOperandsPass() {
   return new SIFoldOperands();
 }
 
 bool SIFoldOperands::updateOperand(FoldCandidate &Fold) const {
   MachineInstr *MI = Fold.UseMI;
   MachineOperand &Old = MI->getOperand(Fold.UseOpNo);
   assert(Old.isReg());
 
+
+  const uint64_t TSFlags = MI->getDesc().TSFlags;
   if (Fold.isImm()) {
-    if (MI->getDesc().TSFlags & SIInstrFlags::IsPacked &&
-        !(MI->getDesc().TSFlags & SIInstrFlags::IsMAI) &&
-        (!ST->hasDOTOpSelHazard() ||
-         !(MI->getDesc().TSFlags & SIInstrFlags::IsDOT)) &&
+    if (TSFlags & SIInstrFlags::IsPacked && !(TSFlags & SIInstrFlags::IsMAI) &&
+        (!ST->hasDOTOpSelHazard() || !(TSFlags & SIInstrFlags::IsDOT)) &&
         AMDGPU::isFoldableLiteralV216(Fold.ImmToFold,
                                       ST->hasInv2PiInlineImm())) {
       // Set op_sel/op_sel_hi on this operand or bail out if op_sel is
       // already set.
       unsigned Opcode = MI->getOpcode();
       int OpNo = MI->getOperandNo(&Old);
       int ModIdx = -1;
       if (OpNo == AMDGPU::getNamedOperandIdx(Opcode, AMDGPU::OpName::src0))
bool SIFoldOperands::updateOperand(FoldCandidate &Fold) const {
   if ((Fold.isImm() || Fold.isFI() || Fold.isGlobal()) && Fold.needsShrink()) {
     MachineBasicBlock *MBB = MI->getParent();
     auto Liveness = MBB->computeRegisterLiveness(TRI, AMDGPU::VCC, MI, 16);
     if (Liveness != MachineBasicBlock::LQR_Dead) {
       LLVM_DEBUG(dbgs() << "Not shrinking " << MI << " due to vcc liveness\n");
       return false;
     }
 
-    int Op32 = Fold.getShrinkOpcode();
+    int Op32 = Fold.ShrinkOpcode;
     MachineOperand &Dst0 = MI->getOperand(0);
     MachineOperand &Dst1 = MI->getOperand(1);
     assert(Dst0.isDef() && Dst1.isDef());
 
     bool HaveNonDbgCarryUse = !MRI->use_nodbg_empty(Dst1.getReg());
 
     const TargetRegisterClass *Dst0RC = MRI->getRegClass(Dst0.getReg());
     Register NewReg0 = MRI->createVirtualRegister(Dst0RC);
if ((Fold.isImm() || Fold.isFI() || Fold.isGlobal()) && Fold.needsShrink()) {
     // FIXME: We should not invert how this pass looks at operands to avoid
     // this. Should track set of foldable movs instead of looking for uses
     // when looking at a use.
     Dst0.setReg(NewReg0);
     for (unsigned I = MI->getNumOperands() - 1; I > 0; --I)
       MI->removeOperand(I);
     MI->setDesc(TII->get(AMDGPU::IMPLICIT_DEF));
 
-    if (Fold.isCommuted())
+    if (Fold.Commuted)
       TII->commuteInstruction(*Inst32, false);
     return true;
   }
 
   assert(!Fold.needsShrink() && "not handled");
 
   if (Fold.isImm()) {
     if (Old.isTied()) {
bool SIFoldOperands::updateOperand(FoldCandidate &Fold) const {
   MachineOperand *New = Fold.OpToFold;
   Old.substVirtReg(New->getReg(), New->getSubReg(), *TRI);
   Old.setIsUndef(New->isUndef());
   return true;
 }
 
 static bool isUseMIInFoldList(ArrayRef<FoldCandidate> FoldList,
                               const MachineInstr *MI) {
-  for (auto Candidate : FoldList) {
-    if (Candidate.UseMI == MI)
-      return true;
-  }
-  return false;
+  return any_of(FoldList, [&](const auto &C) { return C.UseMI == MI; });
 }
 
 static void appendFoldCandidate(SmallVectorImpl<FoldCandidate> &FoldList,
                                 MachineInstr *MI, unsigned OpNo,
                                 MachineOperand *FoldOp, bool Commuted = false,
                                 int ShrinkOp = -1) {
   // Skip additional folding on the same operand.
   for (FoldCandidate &Fold : FoldList)
bool SIFoldOperands::isUseSafeToFold(const MachineInstr &MI,
   case AMDGPU::V_MOV_B64_PSEUDO:
   case AMDGPU::V_MOV_B64_e32:
   case AMDGPU::V_MOV_B64_e64:
     // Do not fold into an indirect mov.
     return !MI.hasRegisterImplicitUseOperand(AMDGPU::M0);
   }
 
   return true;
-  //return !MI.hasRegisterImplicitUseOperand(UseMO.getReg());
 }
 
 // Find a def of the UseReg, check if it is a reg_sequence and find initializers
 // for each subreg, tracking it to foldable inline immediate if possible.
 // Returns true on success.
 bool SIFoldOperands::getRegSeqInit(
     SmallVectorImpl<std::pair<MachineOperand *, unsigned>> &Defs,
     Register UseReg, uint8_t OpTy) const {
void SIFoldOperands::foldOperand(
   SmallVectorImpl<FoldCandidate> &FoldList,
   SmallVectorImpl<MachineInstr *> &CopiesToReplace) const {
   const MachineOperand &UseOp = UseMI->getOperand(UseOpIdx);
 
   if (!isUseSafeToFold(*UseMI, UseOp))
     return;
 
   // FIXME: Fold operands with subregs.
-  if (UseOp.isReg() && OpToFold.isReg()) {
-    if (UseOp.isImplicit() || UseOp.getSubReg() != AMDGPU::NoSubRegister)
-      return;
-  }
+  if (UseOp.isReg() && OpToFold.isReg() &&
+      (UseOp.isImplicit() || UseOp.getSubReg() != AMDGPU::NoSubRegister))
 
   // Special case for REG_SEQUENCE: We can't fold literals into
   // REG_SEQUENCE instructions, so we have to fold them into the
   // uses of REG_SEQUENCE.
   if (UseMI->isRegSequence()) {
     Register RegSeqDstReg = UseMI->getOperand(0).getReg();
     unsigned RegSeqDstSubReg = UseMI->getOperand(UseOpIdx + 1).getImm();
 
if (TII->isMUBUF(*UseMI)) {
       if (!SOff.isImm() || SOff.getImm() != 0)
         return;
     }
 
     // A frame index will resolve to a positive constant, so it should always be
     // safe to fold the addressing mode, even pre-GFX9.
     UseMI->getOperand(UseOpIdx).ChangeToFrameIndex(OpToFold.getIndex());
 
+    const unsigned Opc = UseMI->getOpcode();
     if (TII->isFLATScratch(*UseMI) &&
-        AMDGPU::getNamedOperandIdx(UseMI->getOpcode(),
-                                   AMDGPU::OpName::vaddr) != -1 &&
-        AMDGPU::getNamedOperandIdx(UseMI->getOpcode(),
-                                   AMDGPU::OpName::saddr) == -1) {
-      unsigned NewOpc = AMDGPU::getFlatScratchInstSSfromSV(UseMI->getOpcode());
+        AMDGPU::getNamedOperandIdx(Opc, AMDGPU::OpName::vaddr) != -1 &&
+        AMDGPU::getNamedOperandIdx(Opc, AMDGPU::OpName::saddr) == -1) {
+      unsigned NewOpc = AMDGPU::getFlatScratchInstSSfromSV(Opc);
       UseMI->setDesc(TII->get(NewOpc));
     }
 
     return;
   }
 
   bool FoldingImmLike =
       OpToFold.isImm() || OpToFold.isFI() || OpToFold.isGlobal();
if (!DestReg.isPhysical()) {
           // There's no point trying to fold into an implicit operand.
           if (Use.isImplicit())
             continue;
 
           CopyUses.emplace_back(Use.getParent(),
                                 Use.getParent()->getOperandNo(&Use),
                                 &UseMI->getOperand(1));
         }
+
         for (auto &F : CopyUses) {
-          foldOperand(*F.OpToFold, F.UseMI, F.UseOpNo, FoldList, CopiesToReplace);
+          foldOperand(*F.OpToFold, F.UseMI, F.UseOpNo, FoldList,
+                      CopiesToReplace);
         }
       }
 
       if (DestRC == &AMDGPU::AGPR_32RegClass &&
           TII->isInlineConstant(OpToFold, AMDGPU::OPERAND_REG_INLINE_C_INT32)) {
         UseMI->setDesc(TII->get(AMDGPU::V_ACCVGPR_WRITE_B32_e64));
         UseMI->getOperand(1).ChangeToImmediate(OpToFold.getImm());
         CopiesToReplace.push_back(UseMI);
         return;
       }
if (UseMI->isCopy() && OpToFold.isReg() &&
           B.addImm(Defs[I].second);
         }
         LLVM_DEBUG(dbgs() << "Folded " << *UseMI);
         return;
       }
 
       if (Size != 4)
         return;
-      if (TRI->isAGPR(*MRI, UseMI->getOperand(0).getReg()) &&
-          TRI->isVGPR(*MRI, UseMI->getOperand(1).getReg()))
+
+      Register Reg0 = UseMI->getOperand(0).getReg();
+      Register Reg1 = UseMI->getOperand(1).getReg();
+      if (TRI->isAGPR(*MRI, Reg0) && TRI->isVGPR(*MRI, Reg1))
         UseMI->setDesc(TII->get(AMDGPU::V_ACCVGPR_WRITE_B32_e64));
-      else if (TRI->isVGPR(*MRI, UseMI->getOperand(0).getReg()) &&
-               TRI->isAGPR(*MRI, UseMI->getOperand(1).getReg()))
+      else if (TRI->isVGPR(*MRI, Reg0) && TRI->isAGPR(*MRI, Reg1))
         UseMI->setDesc(TII->get(AMDGPU::V_ACCVGPR_READ_B32_e64));
-      else if (ST->hasGFX90AInsts() &&
-               TRI->isAGPR(*MRI, UseMI->getOperand(0).getReg()) &&
-               TRI->isAGPR(*MRI, UseMI->getOperand(1).getReg()))
+      else if (ST->hasGFX90AInsts() && TRI->isAGPR(*MRI, Reg0) &&
+               TRI->isAGPR(*MRI, Reg1))
         UseMI->setDesc(TII->get(AMDGPU::V_ACCVGPR_MOV_B32));
       return;
     }
 
     unsigned UseOpc = UseMI->getOpcode();
     if (UseOpc == AMDGPU::V_READFIRSTLANE_B32 ||
         (UseOpc == AMDGPU::V_READLANE_B32 &&
          (int)UseOpIdx ==
default:
     return false;
   }
 }
 
 static unsigned getMovOpc(bool IsScalar) {
   return IsScalar ? AMDGPU::S_MOV_B32 : AMDGPU::V_MOV_B32_e32;
 }
 
-/// Remove any leftover implicit operands from mutating the instruction. e.g.
-/// if we replace an s_and_b32 with a copy, we don't need the implicit scc def
-/// anymore.
-static void stripExtraCopyOperands(MachineInstr &MI) {
+static void mutateCopyOp(MachineInstr &MI, const MCInstrDesc &NewDesc) {
+  MI.setDesc(NewDesc);
+
+  // Remove any leftover implicit operands from mutating the instruction. e.g.
+  // if we replace an s_and_b32 with a copy, we don't need the implicit scc def
+  // anymore.
   const MCInstrDesc &Desc = MI.getDesc();
   unsigned NumOps = Desc.getNumOperands() +
                     Desc.getNumImplicitUses() +
                     Desc.getNumImplicitDefs();
 
   for (unsigned I = MI.getNumOperands() - 1; I >= NumOps; --I)
     MI.removeOperand(I);
 }
 
-static void mutateCopyOp(MachineInstr &MI, const MCInstrDesc &NewDesc) {
-  MI.setDesc(NewDesc);
-  stripExtraCopyOperands(MI);
-}
-
 MachineOperand *
 SIFoldOperands::getImmOrMaterializedImm(MachineOperand &Op) const {
-  if (Op.isReg()) {
-    // If this has a subregister, it obviously is a register source.
-    if (Op.getSubReg() != AMDGPU::NoSubRegister || !Op.getReg().isVirtual())
-      return &Op;
+  if (!Op.isReg() || Op.getSubReg() != AMDGPU::NoSubRegister ||
+      !Op.getReg().isVirtual())
 
-    MachineInstr *Def = MRI->getVRegDef(Op.getReg());
-    if (Def && Def->isMoveImmediate()) {
-      MachineOperand &ImmSrc = Def->getOperand(1);
-      if (ImmSrc.isImm())
-        return &ImmSrc;
-    }
-  }
 
   return &Op;
 }
 
 // Try to simplify operations with a constant that may appear after instruction
 // selection.
 // TODO: See if a frame index with a fixed offset can fold.
 bool SIFoldOperands::tryConstantFoldOp(MachineInstr *MI) const {
if (Src1Val == 0) {
       MI->removeOperand(Src1Idx);
       mutateCopyOp(*MI, TII->get(getMovOpc(Opc == AMDGPU::S_OR_B32)));
     } else
       return false;
 
     return true;
   }
 
-  if (MI->getOpcode() == AMDGPU::V_AND_B32_e64 ||
-      MI->getOpcode() == AMDGPU::V_AND_B32_e32 ||
-      MI->getOpcode() == AMDGPU::S_AND_B32) {
+  if (Opc == AMDGPU::V_AND_B32_e64 || Opc == AMDGPU::V_AND_B32_e32 ||
+      Opc == AMDGPU::S_AND_B32) {
     if (Src1Val == 0) {
       // y = and x, 0 => y = v_mov_b32 0
       MI->removeOperand(Src0Idx);
       mutateCopyOp(*MI, TII->get(getMovOpc(Opc == AMDGPU::S_AND_B32)));
     } else if (Src1Val == -1) {
       // y = and x, -1 => y = copy x
       MI->removeOperand(Src1Idx);
       mutateCopyOp(*MI, TII->get(AMDGPU::COPY));
-      stripExtraCopyOperands(*MI);
     } else
       return false;
 
     return true;
   }
 
-  if (MI->getOpcode() == AMDGPU::V_XOR_B32_e64 ||
-      MI->getOpcode() == AMDGPU::V_XOR_B32_e32 ||
-      MI->getOpcode() == AMDGPU::S_XOR_B32) {
+  if (Opc == AMDGPU::V_XOR_B32_e64 || Opc == AMDGPU::V_XOR_B32_e32 ||
+      Opc == AMDGPU::S_XOR_B32) {
     if (Src1Val == 0) {
       // y = xor x, 0 => y = copy x
       MI->removeOperand(Src1Idx);
       mutateCopyOp(*MI, TII->get(AMDGPU::COPY));
       return true;
     }
   }
 
if (MI.getOpcode() != AMDGPU::V_AND_B32_e64 &&
     return false;
 
   MachineOperand *Src0 = getImmOrMaterializedImm(MI.getOperand(1));
   if (!Src0->isImm() || Src0->getImm() != 0xffff)
     return false;
 
   Register Src1 = MI.getOperand(2).getReg();
   MachineInstr *SrcDef = MRI->getVRegDef(Src1);
-  if (ST->zeroesHigh16BitsOfDest(SrcDef->getOpcode())) {
-    Register Dst = MI.getOperand(0).getReg();
-    MRI->replaceRegWith(Dst, SrcDef->getOperand(0).getReg());
-    MI.eraseFromParent();
-    return true;
-  }
+  if (!ST->zeroesHigh16BitsOfDest(SrcDef->getOpcode()))
+    return false;
+
 
-  return false;
-}
-
 bool SIFoldOperands::foldInstOperand(MachineInstr &MI,
                                      MachineOperand &OpToFold) const {
   // We need mutate the operands of new mov instructions to add implicit
   // uses of EXEC, but adding them invalidates the use_iterator, so defer
   // this.
   SmallVector<MachineInstr *, 4> CopiesToReplace;
   SmallVector<FoldCandidate, 4> FoldList;
   MachineOperand &Dst = MI.getOperand(0);
if (updateOperand(Fold)) {
         // FIXME: Probably shouldn't bother trying to fold if not an
         // SGPR. PeepholeOptimizer can eliminate redundant VGPR->VGPR
         // copies.
         MRI->clearKillFlags(Fold.OpToFold->getReg());
       }
       LLVM_DEBUG(dbgs() << "Folded source from " << MI << " into OpNo "
                         << static_cast<int>(Fold.UseOpNo) << " of "
                         << *Fold.UseMI);
-    } else if (Fold.isCommuted()) {
+    } else if (Fold.Commuted) {
       // Restoring instruction's original operand order if fold has failed.
       TII->commuteInstruction(*Fold.UseMI, false);
     }
   }
   return true;
 }
 
 bool SIFoldOperands::tryFoldFoldableCopy(
bool SIFoldOperands::tryFoldLoad(MachineInstr &MI) {
 
   Register DefReg = Def.getReg();
 
   if (DefReg.isPhysical() || !TRI->isVGPR(*MRI, DefReg))
     return false;
 
   SmallVector<const MachineInstr*, 8> Users;
   SmallVector<Register, 8> MoveRegs;
-  for (const MachineInstr &I : MRI->use_nodbg_instructions(DefReg)) {
+  for (const MachineInstr &I : MRI->use_nodbg_instructions(DefReg))
     Users.push_back(&I);
-  }
+
   if (Users.empty())
     return false;
 
   // Check that all uses a copy to an agpr or a reg_sequence producing an agpr.
   while (!Users.empty()) {
     const MachineInstr *I = Users.pop_back_val();
     if (!I->isCopy() && !I->isRegSequence())
       return false;
     Register DstReg = I->getOperand(0).getReg();
     if (TRI->isAGPR(*MRI, DstReg))
       continue;
     MoveRegs.push_back(DstReg);
-    for (const MachineInstr &U : MRI->use_nodbg_instructions(DstReg)) {
+    for (const MachineInstr &U : MRI->use_nodbg_instructions(DstReg))
       Users.push_back(&U);
-    }
-  }
 
   const TargetRegisterClass *RC = MRI->getRegClass(DefReg);
   MRI->setRegClass(DefReg, TRI->getEquivalentAGPRClass(RC));
   if (!TII->isOperandLegal(MI, 0, &Def)) {
     MRI->setRegClass(DefReg, RC);
     return false;
   }