[ARM][MVE] findVCMPToFoldIntoVPS. NFC.

[SjoerdMeijer]

Summary

While grepping for some inspriration, I came across this function, and I found the logic a bit difficult to follow, so I simplified it a bit.

[samparker]

If this is going to be rewritten, I'd suggest using RDA here which should hopefully be able to get your the reaching def of the VPR in a single call. Two more calls for checking for defs between instructions.

[SjoerdMeijer]

it's not that this is important, but the code would benefit from a rewrite. This was a more easy/straightforward change, but agreed that RDA would be even better, so will see what I can do.

[SjoerdMeijer]

Now using RDA

[samparker]

llvm/lib/Target/ARM/MVEVPTBlockPass.cpp

167

RDA...?

[SjoerdMeijer]

Yep, more RDA

[samparker]

Cheers! Just please remove TRI before committing.

llvm/lib/Target/ARM/MVEVPTBlockPass.cpp

138

Unused argument and looks like the formatting is off below.

llvm/lib/Target/ARM/MVEVPTBlockPass.cpp

case ARM::MVE_VCMPs32r:
     return ARM::MVE_VPTv4s32r;
 
   default:
     return 0;
   }
 }
 
 static MachineInstr *findVCMPToFoldIntoVPST(MachineBasicBlock::iterator MI,
                                             const TargetRegisterInfo *TRI,
                                             unsigned &NewOpcode) {
   // Search backwards to the instruction that defines VPR. This may or not
   // be a VCMP, we check that after this loop. If we find another instruction
   // that reads cpsr, we return nullptr.
   MachineBasicBlock::iterator CmpMI = MI;
+  NewOpcode = 0;
+
   while (CmpMI != MI->getParent()->begin()) {
     --CmpMI;
-    if (CmpMI->modifiesRegister(ARM::VPR, TRI))
-      break;
-    if (CmpMI->readsRegister(ARM::VPR, TRI))
+    if (CmpMI->modifiesRegister(ARM::VPR, TRI) ||
+        CmpMI->readsRegister(ARM::VPR, TRI)) {
+      NewOpcode = VCMPOpcodeToVPT(CmpMI->getOpcode());
       break;
     }
+  }
 
-  if (CmpMI == MI)
-    return nullptr;
-  NewOpcode = VCMPOpcodeToVPT(CmpMI->getOpcode());
-  if (NewOpcode == 0)
+  if (!NewOpcode)
     return nullptr;
 
   // Search forward from CmpMI to MI, checking if either register was def'd
   if (registerDefinedBetween(CmpMI->getOperand(1).getReg(), std::next(CmpMI),
-                             MI, TRI))
-    return nullptr;
-  if (registerDefinedBetween(CmpMI->getOperand(2).getReg(), std::next(CmpMI),
+                             MI, TRI) ||
+      registerDefinedBetween(CmpMI->getOperand(2).getReg(), std::next(CmpMI),
                              MI, TRI))
     return nullptr;
   return &*CmpMI;
 }
 
 bool MVEVPTBlock::InsertVPTBlocks(MachineBasicBlock &Block) {
   bool Modified = false;
   MachineBasicBlock::instr_iterator MBIter = Block.instr_begin();
   MachineBasicBlock::instr_iterator EndIter = Block.instr_end();
 
   while (MBIter != EndIter) {
     MachineInstr *MI = &*MBIter;
     unsigned PredReg = 0;
     DebugLoc dl = MI->getDebugLoc();