[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

161

RDA...?

[SjoerdMeijer]

Yep, more RDA

[samparker]

Cheers! Just please remove TRI before committing.

llvm/lib/Target/ARM/MVEVPTBlockPass.cpp

145–146

Unused argument and looks like the formatting is off below.

llvm/lib/Target/ARM/MVEVPTBlockPass.cpp

 #include "llvm/ADT/StringRef.h"
 #include "llvm/CodeGen/MachineBasicBlock.h"
 #include "llvm/CodeGen/MachineFunction.h"
 #include "llvm/CodeGen/MachineFunctionPass.h"
 #include "llvm/CodeGen/MachineInstr.h"
 #include "llvm/CodeGen/MachineInstrBuilder.h"
 #include "llvm/CodeGen/MachineInstrBundle.h"
 #include "llvm/CodeGen/MachineOperand.h"
+#include "llvm/CodeGen/ReachingDefAnalysis.h"
 #include "llvm/IR/DebugLoc.h"
 #include "llvm/MC/MCInstrDesc.h"
-#include "llvm/MC/MCRegisterInfo.h"
 #include "llvm/Support/Debug.h"
 #include <cassert>
 #include <new>
 
 using namespace llvm;
 
 #define DEBUG_TYPE "arm-mve-vpt"
 
 namespace {
   class MVEVPTBlock : public MachineFunctionPass {
   public:
     static char ID;
-    const Thumb2InstrInfo *TII;
-    const TargetRegisterInfo *TRI;
 
     MVEVPTBlock() : MachineFunctionPass(ID) {}
 
     bool runOnMachineFunction(MachineFunction &Fn) override;
 
+    void getAnalysisUsage(AnalysisUsage &AU) const override {
+      AU.setPreservesCFG();
+      AU.addRequired<ReachingDefAnalysis>();
+      MachineFunctionPass::getAnalysisUsage(AU);
+    }
+
     MachineFunctionProperties getRequiredProperties() const override {
       return MachineFunctionProperties().set(
-          MachineFunctionProperties::Property::NoVRegs);
+          MachineFunctionProperties::Property::NoVRegs).set(
+          MachineFunctionProperties::Property::TracksLiveness);
     }
 
     StringRef getPassName() const override {
       return "MVE VPT block insertion pass";
     }
 
   private:
     bool InsertVPTBlocks(MachineBasicBlock &MBB);
+
+    const Thumb2InstrInfo *TII = nullptr;
+    ReachingDefAnalysis *RDA = nullptr;
   };
 
   char MVEVPTBlock::ID = 0;
 
 } // end anonymous namespace
 
 INITIALIZE_PASS(MVEVPTBlock, DEBUG_TYPE, "ARM MVE VPT block pass", false, false)
 
static unsigned VCMPOpcodeToVPT(unsigned Opcode) {
   case ARM::MVE_VCMPs32r:
     return ARM::MVE_VPTv4s32r;
 
   default:
     return 0;
   }
 }
 
-static MachineInstr *findVCMPToFoldIntoVPST(MachineBasicBlock::iterator MI,
-                                            const TargetRegisterInfo *TRI,
+static MachineInstr *findVCMPToFoldIntoVPST(MachineInstr *MI,
+                                            ReachingDefAnalysis *RDA,
                                             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;
-  while (CmpMI != MI->getParent()->begin()) {
-    --CmpMI;
-    if (CmpMI->modifiesRegister(ARM::VPR, TRI))
-      break;
-    if (CmpMI->readsRegister(ARM::VPR, TRI))
-      break;
-  }
-
-  if (CmpMI == MI)
-    return nullptr;
-  NewOpcode = VCMPOpcodeToVPT(CmpMI->getOpcode());
-  if (NewOpcode == 0)
+  // First, search backwards to the instruction that defines VPR
+  auto *Def = RDA->getReachingMIDef(MI, ARM::VPR);
+  if (!Def)
     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))
+  // Now check that Def is a VCMP
+  if (!(NewOpcode = VCMPOpcodeToVPT(Def->getOpcode())))
     return nullptr;
-  if (registerDefinedBetween(CmpMI->getOperand(2).getReg(), std::next(CmpMI),
-                             MI, TRI))
+
+  // Check that Def's operands are not defined between the VCMP and MI, i.e.
+  // check that they have the same reaching def.
+  if (!RDA->hasSameReachingDef(Def, MI, Def->getOperand(1).getReg()) ||
+      !RDA->hasSameReachingDef(Def, MI, Def->getOperand(2).getReg()))
     return nullptr;
-  return &*CmpMI;
+
+  return Def;
 }
 
 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) {
while (MBIter != EndIter) {
     default:
       llvm_unreachable("Unexpected number of instruction in a VPT block");
     };
 
     // Search back for a VCMP that can be folded to create a VPT, or else create
     // a VPST directly
     MachineInstrBuilder MIBuilder;
     unsigned NewOpcode;
-    MachineInstr *VCMP = findVCMPToFoldIntoVPST(MI, TRI, NewOpcode);
+    MachineInstr *VCMP = findVCMPToFoldIntoVPST(MI, RDA, NewOpcode);
     if (VCMP) {
       LLVM_DEBUG(dbgs() << "  folding VCMP into VPST: "; VCMP->dump());
       MIBuilder = BuildMI(Block, MI, dl, TII->get(NewOpcode));
       MIBuilder.addImm(BlockMask);
       MIBuilder.add(VCMP->getOperand(1));
       MIBuilder.add(VCMP->getOperand(2));
       MIBuilder.add(VCMP->getOperand(3));
       VCMP->eraseFromParent();

 bool MVEVPTBlock::runOnMachineFunction(MachineFunction &Fn) {
   const ARMSubtarget &STI =
       static_cast<const ARMSubtarget &>(Fn.getSubtarget());
 
   if (!STI.isThumb2() || !STI.hasMVEIntegerOps())
     return false;
 
   TII = static_cast<const Thumb2InstrInfo *>(STI.getInstrInfo());
-  TRI = STI.getRegisterInfo();
+  RDA = &getAnalysis<ReachingDefAnalysis>();
 
   LLVM_DEBUG(dbgs() << "********** ARM MVE VPT BLOCKS **********\n"
                     << "********** Function: " << Fn.getName() << '\n');
 
   bool Modified = false;
   for (MachineBasicBlock &MBB : Fn)
     Modified |= InsertVPTBlocks(MBB);
 
   LLVM_DEBUG(dbgs() << "**************************************\n");
   return Modified;
 }
 
 /// createMVEVPTBlock - Returns an instance of the MVE VPT block
 /// insertion pass.
 FunctionPass *llvm::createMVEVPTBlockPass() { return new MVEVPTBlock(); }

llvm/test/CodeGen/ARM/O3-pipeline.ll

 ; CHECK-NEXT:      ARM Execution Domain Fix
 ; CHECK-NEXT:      BreakFalseDeps
 ; CHECK-NEXT:      ARM pseudo instruction expansion pass
 ; CHECK-NEXT:      Thumb2 instruction size reduce pass
 ; CHECK-NEXT:      MachineDominator Tree Construction
 ; CHECK-NEXT:      Machine Natural Loop Construction
 ; CHECK-NEXT:      Machine Block Frequency Analysis
 ; CHECK-NEXT:      If Converter
+; CHECK-NEXT:      ReachingDefAnalysis
 ; CHECK-NEXT:      MVE VPT block insertion pass
 ; CHECK-NEXT:      Thumb IT blocks insertion pass
 ; CHECK-NEXT:      MachineDominator Tree Construction
 ; CHECK-NEXT:      Machine Natural Loop Construction
 ; CHECK-NEXT:      PostRA Machine Instruction Scheduler
 ; CHECK-NEXT:      Post RA top-down list latency scheduler
 ; CHECK-NEXT:      Analyze Machine Code For Garbage Collection
 ; CHECK-NEXT:      Machine Block Frequency Analysis