[ARM] Thumb2CondExecution. NFC

[SjoerdMeijer]

Summary

Some more refactoring, like registering the IT Block pass, less cryptic
variable names, and some simplification of loops.

This depends on D63418.

[samparker]

LGTM

llvm/lib/Target/ARM/Thumb2CondExecution.cpp

194 ↗

(On Diff #205043)

Just a bike shedding nit... MBB is the defacto variable name for a MachineBasicBlock.. and the same for the iterator names.

[SjoerdMeijer]

llvm/lib/Target/ARM/Thumb2CondExecution.cpp

103 ↗

(On Diff #205043)

Oopsy daisy: 'Uses' is wrong here, which I will fix.
This passed tests, indicating some tests are missing here...

109 ↗

(On Diff #205043)

I accidentally cleaned this up, i.e. didn't notice it. But it turns out I was lucky: this if and continue doesn't make any sense.

194 ↗

(On Diff #205043)

In general I like short variable names, but here all permutations MI, MIB, MBBI, MBB, NMI got a bit too much for me. But as I don't plan to touch this part, I will revert these changes.

[SjoerdMeijer]

just uploading the diff that I want to commit.

llvm/lib/Target/ARM/ARM.h

 
 void initializeARMParallelDSPPass(PassRegistry &);
 void initializeARMLoadStoreOptPass(PassRegistry &);
 void initializeARMPreAllocLoadStoreOptPass(PassRegistry &);
 void initializeARMCodeGenPreparePass(PassRegistry &);
 void initializeARMConstantIslandsPass(PassRegistry &);
 void initializeARMExpandPseudoPass(PassRegistry &);
 void initializeThumb2SizeReducePass(PassRegistry &);
+void initializeThumb2ITBlockPass(PassRegistry &);
 void initializeMVEVPTBlockPass(PassRegistry &);
 
 } // end namespace llvm
 
 #endif // LLVM_LIB_TARGET_ARM_ARM_H

llvm/lib/Target/ARM/Thumb2ITBlockPass.cpp

 #include "llvm/MC/MCInstrDesc.h"
 #include "llvm/MC/MCRegisterInfo.h"
 #include <cassert>
 #include <new>
 
 using namespace llvm;
 
 #define DEBUG_TYPE "thumb2-it"
+#define PASS_NAME "Thumb IT blocks insertion pass"
 
 STATISTIC(NumITs,        "Number of IT blocks inserted");
 STATISTIC(NumMovedInsts, "Number of predicated instructions moved");
 
+using RegisterSet = SmallSet<unsigned, 4>;
+
 namespace {
 
-  class Thumb2ITBlockPass : public MachineFunctionPass {
+  class Thumb2ITBlock : public MachineFunctionPass {
   public:
     static char ID;
 
     bool restrictIT;
     const Thumb2InstrInfo *TII;
     const TargetRegisterInfo *TRI;
     ARMFunctionInfo *AFI;
 
-    Thumb2ITBlockPass() : MachineFunctionPass(ID) {}
+    Thumb2ITBlock() : MachineFunctionPass(ID) {}
 
     bool runOnMachineFunction(MachineFunction &Fn) override;
 
     MachineFunctionProperties getRequiredProperties() const override {
       return MachineFunctionProperties().set(
           MachineFunctionProperties::Property::NoVRegs);
     }
 
     StringRef getPassName() const override {
-      return "Thumb IT blocks insertion pass";
+      return PASS_NAME;
     }
 
   private:
     bool MoveCopyOutOfITBlock(MachineInstr *MI,
                               ARMCC::CondCodes CC, ARMCC::CondCodes OCC,
-                              SmallSet<unsigned, 4> &Defs,
-                              SmallSet<unsigned, 4> &Uses);
-    bool InsertITInstructions(MachineBasicBlock &MBB);
+                              RegisterSet &Defs, RegisterSet &Uses);
+    bool InsertITInstructions(MachineBasicBlock &Block);
   };
 
-  char Thumb2ITBlockPass::ID = 0;
+  char Thumb2ITBlock::ID = 0;
 
 } // end anonymous namespace
 
+INITIALIZE_PASS(Thumb2ITBlock, DEBUG_TYPE, PASS_NAME, false, false)
+
 /// TrackDefUses - Tracking what registers are being defined and used by
 /// instructions in the IT block. This also tracks "dependencies", i.e. uses
 /// in the IT block that are defined before the IT instruction.
-static void TrackDefUses(MachineInstr *MI,
-                         SmallSet<unsigned, 4> &Defs,
-                         SmallSet<unsigned, 4> &Uses,
+static void TrackDefUses(MachineInstr *MI, RegisterSet &Defs, RegisterSet &Uses,
                          const TargetRegisterInfo *TRI) {
-  SmallVector<unsigned, 4> LocalDefs;
-  SmallVector<unsigned, 4> LocalUses;
+  using RegList = SmallVector<unsigned, 4>;
+  RegList LocalDefs;
+  RegList LocalUses;
 
-  for (unsigned i = 0, e = MI->getNumOperands(); i != e; ++i) {
-    MachineOperand &MO = MI->getOperand(i);
+  for (auto &MO : MI->operands()) {
     if (!MO.isReg())
       continue;
     unsigned Reg = MO.getReg();
     if (!Reg || Reg == ARM::ITSTATE || Reg == ARM::SP)
       continue;
     if (MO.isUse())
       LocalUses.push_back(Reg);
     else
       LocalDefs.push_back(Reg);
   }
 
-  for (unsigned i = 0, e = LocalUses.size(); i != e; ++i) {
-    unsigned Reg = LocalUses[i];
+  auto InsertUsesDefs = [&](RegList &Regs, RegisterSet &UsesDefs) {
+    for (unsigned Reg : Regs)
       for (MCSubRegIterator Subreg(Reg, TRI, /*IncludeSelf=*/true);
            Subreg.isValid(); ++Subreg)
-      Uses.insert(*Subreg);
-  }
+        UsesDefs.insert(*Subreg);
+  };
 
-  for (unsigned i = 0, e = LocalDefs.size(); i != e; ++i) {
-    unsigned Reg = LocalDefs[i];
-    for (MCSubRegIterator Subreg(Reg, TRI, /*IncludeSelf=*/true);
-         Subreg.isValid(); ++Subreg)
-      Defs.insert(*Subreg);
-    if (Reg == ARM::CPSR)
-      continue;
-  }
+  InsertUsesDefs(LocalDefs, Defs);
+  InsertUsesDefs(LocalUses, Uses);
 }
 
 /// Clear kill flags for any uses in the given set.  This will likely
 /// conservatively remove more kill flags than are necessary, but removing them
 /// is safer than incorrect kill flags remaining on instructions.
-static void ClearKillFlags(MachineInstr *MI, SmallSet<unsigned, 4> &Uses) {
+static void ClearKillFlags(MachineInstr *MI, RegisterSet &Uses) {
   for (MachineOperand &MO : MI->operands()) {
     if (!MO.isReg() || MO.isDef() || !MO.isKill())
       continue;
     if (!Uses.count(MO.getReg()))
       continue;
     MO.setIsKill(false);
   }
 }
 
 static bool isCopy(MachineInstr *MI) {
   switch (MI->getOpcode()) {
   default:
     return false;
   case ARM::MOVr:
   case ARM::MOVr_TC:
   case ARM::tMOVr:
   case ARM::t2MOVr:
     return true;
   }
 }
 
 bool
-Thumb2ITBlockPass::MoveCopyOutOfITBlock(MachineInstr *MI,
-                                      ARMCC::CondCodes CC, ARMCC::CondCodes OCC,
-                                        SmallSet<unsigned, 4> &Defs,
-                                        SmallSet<unsigned, 4> &Uses) {
+Thumb2ITBlock::MoveCopyOutOfITBlock(MachineInstr *MI,
+                                    RegisterSet &Defs, RegisterSet &Uses) {
   if (!isCopy(MI))
     return false;
   // llvm models select's as two-address instructions. That means a copy
   // is inserted before a t2MOVccr, etc. If the copy is scheduled in
   // between selects we would end up creating multiple IT blocks.
   assert(MI->getOperand(0).getSubReg() == 0 &&
          MI->getOperand(1).getSubReg() == 0 &&
          "Sub-register indices still around?");
Thumb2ITBlock::MoveCopyOutOfITBlock(MachineInstr *MI,
   //
   const MCInstrDesc &MCID = MI->getDesc();
   if (MI->hasOptionalDef() &&
       MI->getOperand(MCID.getNumOperands() - 1).getReg() == ARM::CPSR)
     return false;
 
   // Then peek at the next instruction to see if it's predicated on CC or OCC.
   // If not, then there is nothing to be gained by moving the copy.
-  MachineBasicBlock::iterator I = MI; ++I;
+  MachineBasicBlock::iterator I = MI;
+  ++I;
   MachineBasicBlock::iterator E = MI->getParent()->end();
+
   while (I != E && I->isDebugInstr())
     ++I;
+
   if (I != E) {
     unsigned NPredReg = 0;
     ARMCC::CondCodes NCC = getITInstrPredicate(*I, NPredReg);
     if (NCC == CC || NCC == OCC)
       return true;
   }
   return false;
 }
 
-bool Thumb2ITBlockPass::InsertITInstructions(MachineBasicBlock &MBB) {
+bool Thumb2ITBlock::InsertITInstructions(MachineBasicBlock &MBB) {
   bool Modified = false;
-
-  SmallSet<unsigned, 4> Defs;
-  SmallSet<unsigned, 4> Uses;
+  RegisterSet Defs, Uses;
   MachineBasicBlock::iterator MBBI = MBB.begin(), E = MBB.end();
+
   while (MBBI != E) {
     MachineInstr *MI = &*MBBI;
     DebugLoc dl = MI->getDebugLoc();
     unsigned PredReg = 0;
     ARMCC::CondCodes CC = getITInstrPredicate(*MI, PredReg);
     if (CC == ARMCC::AL) {
       ++MBBI;
       continue;
while (MBBI != E) {
 
     Modified = true;
     ++NumITs;
   }
 
   return Modified;
 }
 
-bool Thumb2ITBlockPass::runOnMachineFunction(MachineFunction &Fn) {
+bool Thumb2ITBlock::runOnMachineFunction(MachineFunction &Fn) {
   const ARMSubtarget &STI =
       static_cast<const ARMSubtarget &>(Fn.getSubtarget());
   if (!STI.isThumb2())
     return false;
   AFI = Fn.getInfo<ARMFunctionInfo>();
   TII = static_cast<const Thumb2InstrInfo *>(STI.getInstrInfo());
   TRI = STI.getRegisterInfo();
   restrictIT = STI.restrictIT();
 
   if (!AFI->isThumbFunction())
     return false;
 
   bool Modified = false;
-  for (MachineFunction::iterator MFI = Fn.begin(), E = Fn.end(); MFI != E; ) {
-    MachineBasicBlock &MBB = *MFI;
-    ++MFI;
+  for (auto &MBB : Fn )
     Modified |= InsertITInstructions(MBB);
-  }
 
   if (Modified)
     AFI->setHasITBlocks(true);
 
   return Modified;
 }
 
 /// createThumb2ITBlockPass - Returns an instance of the Thumb2 IT blocks
 /// insertion pass.
-FunctionPass *llvm::createThumb2ITBlockPass() {
-  return new Thumb2ITBlockPass();
-}
+FunctionPass *llvm::createThumb2ITBlockPass() { return new Thumb2ITBlock(); }
 
 #undef DEBUG_TYPE
 #define DEBUG_TYPE "arm-mve-vpt"
 
 namespace {
   class MVEVPTBlock : public MachineFunctionPass {
   public:
     static char ID;
while (MBIter != EndIter) {
 
     if (Pred == ARMVCC::None) {
       ++MBIter;
       continue;
     }
 
     MachineInstrBuilder MIBuilder =
         BuildMI(Block, MBIter, dl, TII->get(ARM::t2VPST));
-    MachineInstr *LastITMI = MI;
+    MachineInstr *LastMI = MI;
     MachineBasicBlock::iterator InsertPos = MIBuilder.getInstr();
 
     // The mask value for the VPST instruction is T = 0b1000 = 8
     MIBuilder.addImm(VPTMaskValue::T);
 
     finalizeBundle(Block, InsertPos.getInstrIterator(),
-                     ++LastITMI->getIterator());
+                   ++LastMI->getIterator());
     Modified = true;
     LLVM_DEBUG(dbgs() << "VPT block created for: "; MI->dump(););
 
     ++MBIter;
   }
   return Modified;
 }