[NFC][AMDGPU] Small cleanup to R600 CF Finalizer

[ldrumm]

Summary

A static analyzer was getting aggravated about the use of potentially
uninitialized DstMI / SrcMI. I don't think that's possible for the
present case which will only use VTX_INST || TEX_INST instructions - all
having a source and destination register - but it couldn't hurt to
ensure that's invariant.

optional will do the asserting for us.

[arsenm]

llvm/lib/Target/AMDGPU/R600ControlFlowFinalizer.cpp

268

Should just use Register in place of optional<unsigned>

[ldrumm]

Use Register instead of `optional<unsigned>

[ldrumm]

This now asserts for renamable $t0_xy = VTX_READ_64_eg killed renamable $t0_x, 0, 1

DstMI is set to zero which is apparently an invalid Register

TRI->getMatchingSuperReg(t0_xy, 1, R600_Reg128RegClass) is the culprit returning zero. Is that expected? I see t0_Xy is part of the R600_Reg64 class, but not part of the 128bit register class.

[arsenm]

In D159098#4628449, @ldrumm wrote:

DstMI is set to zero which is apparently an invalid Register

Correct

TRI->getMatchingSuperReg(t0_xy, 1, R600_Reg128RegClass) is the culprit returning zero. Is that expected? I see t0_Xy is part of the R600_Reg64 class, but not part of the 128bit register class.

getMatchingSuperReg should use a subreg index, not a 1? Assuming 1=sub0, Using a 32-bit subregister here with a 64-bit class doesn't make sense to me. It would need to use sub0_sub1. I don't know what getMatchingSuperReg is supposed to do in that case

[arsenm]

llvm/lib/Target/AMDGPU/R600ControlFlowFinalizer.cpp

294

is_contained

llvm/lib/Target/AMDGPU/R600ControlFlowFinalizer.cpp

case CF_END:
       break;
     }
     assert (Opcode && "No opcode selected");
     return TII->get(Opcode);
   }
 
   bool isCompatibleWithClause(const MachineInstr &MI,
                               std::set<unsigned> &DstRegs) const {
-    unsigned DstMI, SrcMI;
-    for (MachineInstr::const_mop_iterator I = MI.operands_begin(),
-                                          E = MI.operands_end();
-         I != E; ++I) {
-      const MachineOperand &MO = *I;
+    std::optional<unsigned> DstMI, SrcMI;
+    for (const MachineOperand &MO : MI.operands()) {
       if (!MO.isReg())
         continue;
       if (MO.isDef()) {
         Register Reg = MO.getReg();
         if (R600::R600_Reg128RegClass.contains(Reg))
           DstMI = Reg;
         else
           DstMI = TRI->getMatchingSuperReg(Reg,
               R600RegisterInfo::getSubRegFromChannel(TRI->getHWRegChan(Reg)),
               &R600::R600_Reg128RegClass);
-      }
-      if (MO.isUse()) {
+      } else {
+        assert(MO.isUse());
         Register Reg = MO.getReg();
         if (R600::R600_Reg128RegClass.contains(Reg))
           SrcMI = Reg;
         else
           SrcMI = TRI->getMatchingSuperReg(Reg,
               R600RegisterInfo::getSubRegFromChannel(TRI->getHWRegChan(Reg)),
               &R600::R600_Reg128RegClass);
       }
     }
-    if ((DstRegs.find(SrcMI) == DstRegs.end())) {
-      DstRegs.insert(DstMI);
-      return true;
-    } else
+    // If we've already seen this def in the given register set, we can't
+    // use a clause instruction
+    if (DstRegs.find(SrcMI.value()) != DstRegs.end())
       return false;
+    DstRegs.insert(DstMI.value());
+    return true;
   }
 
   ClauseFile
   MakeFetchClause(MachineBasicBlock &MBB, MachineBasicBlock::iterator &I)
       const {
     MachineBasicBlock::iterator ClauseHead = I;
     std::vector<MachineInstr *> ClauseContent;
     unsigned AluInstCount = 0;