NVPTX: Avoid implicit iterator conversions, NFC

[dexonsmith]

Summary

Avoid implicit conversions from MachineInstrBundleInstr to MachineInstr*
in the MSP430 backend, mainly by preferring MachineInstr& over
MachineInstr* when a pointer isn't nullable and using range-based for
loops.

There was one piece of questionable code in
NVPTXInstrInfo::AnalyzeBranch, where a condition checked a pointer
converted from an iterator for nullptr. Since this case is impossible
(moreover, the code above guarantees that the iterator is valid), I
removed the check when I changed the pointer to a reference.

Despite that case, there should be no functionality change here.

[jlebar]

in the MSP430 backend

Wrong commit message?

from MachineInstrBundleInstr

MachineInstrBundleIterator?

Otherwise looks good to me. This is a simplification, so I like it on face, but do you also expect this change (when made everywhere in llvm) to have performance implications?

[dexonsmith]

Thanks for the review! Committed in r274931. (Answers to your questions below, inline.)

lib/Target/NVPTX/NVPTXFrameLowering.cpp

NVPTXFrameLowering::NVPTXFrameLowering()
     : TargetFrameLowering(TargetFrameLowering::StackGrowsUp, 8, 0) {}
 
 bool NVPTXFrameLowering::hasFP(const MachineFunction &MF) const { return true; }
 
 void NVPTXFrameLowering::emitPrologue(MachineFunction &MF,
                                       MachineBasicBlock &MBB) const {
   if (MF.getFrameInfo()->hasStackObjects()) {
     assert(&MF.front() == &MBB && "Shrink-wrapping not yet supported");
-    MachineInstr *MI = MBB.begin();
+    MachineInstr *MI = &MBB.front();
     MachineRegisterInfo &MR = MF.getRegInfo();
 
     // This instruction really occurs before first instruction
     // in the BB, so giving it no debug location.
     DebugLoc dl = DebugLoc();
 
     // Emits
     //   mov %SPL, %depot;

lib/Target/NVPTX/NVPTXInstrInfo.cpp

bool NVPTXInstrInfo::AnalyzeBranch(
     MachineBasicBlock &MBB, MachineBasicBlock *&TBB, MachineBasicBlock *&FBB,
     SmallVectorImpl<MachineOperand> &Cond, bool AllowModify) const {
   // If the block has no terminators, it just falls into the block after it.
   MachineBasicBlock::iterator I = MBB.end();
   if (I == MBB.begin() || !isUnpredicatedTerminator(*--I))
     return false;
 
   // Get the last instruction in the block.
-  MachineInstr *LastInst = I;
+  MachineInstr &LastInst = *I;
 
   // If there is only one terminator instruction, process it.
   if (I == MBB.begin() || !isUnpredicatedTerminator(*--I)) {
-    if (LastInst->getOpcode() == NVPTX::GOTO) {
-      TBB = LastInst->getOperand(0).getMBB();
+    if (LastInst.getOpcode() == NVPTX::GOTO) {
+      TBB = LastInst.getOperand(0).getMBB();
       return false;
-    } else if (LastInst->getOpcode() == NVPTX::CBranch) {
+    } else if (LastInst.getOpcode() == NVPTX::CBranch) {
       // Block ends with fall-through condbranch.
-      TBB = LastInst->getOperand(1).getMBB();
-      Cond.push_back(LastInst->getOperand(0));
+      TBB = LastInst.getOperand(1).getMBB();
+      Cond.push_back(LastInst.getOperand(0));
       return false;
     }
     // Otherwise, don't know what this is.
     return true;
   }
 
   // Get the instruction before it if it's a terminator.
-  MachineInstr *SecondLastInst = I;
+  MachineInstr &SecondLastInst = *I;
 
   // If there are three terminators, we don't know what sort of block this is.
-  if (SecondLastInst && I != MBB.begin() && isUnpredicatedTerminator(*--I))
+  if (I != MBB.begin() && isUnpredicatedTerminator(*--I))
     return true;
 
   // If the block ends with NVPTX::GOTO and NVPTX:CBranch, handle it.
-  if (SecondLastInst->getOpcode() == NVPTX::CBranch &&
-      LastInst->getOpcode() == NVPTX::GOTO) {
-    TBB = SecondLastInst->getOperand(1).getMBB();
-    Cond.push_back(SecondLastInst->getOperand(0));
-    FBB = LastInst->getOperand(0).getMBB();
+  if (SecondLastInst.getOpcode() == NVPTX::CBranch &&
+      LastInst.getOpcode() == NVPTX::GOTO) {
+    TBB = SecondLastInst.getOperand(1).getMBB();
+    Cond.push_back(SecondLastInst.getOperand(0));
+    FBB = LastInst.getOperand(0).getMBB();
     return false;
   }
 
   // If the block ends with two NVPTX:GOTOs, handle it.  The second one is not
   // executed, so remove it.
-  if (SecondLastInst->getOpcode() == NVPTX::GOTO &&
-      LastInst->getOpcode() == NVPTX::GOTO) {
-    TBB = SecondLastInst->getOperand(0).getMBB();
+  if (SecondLastInst.getOpcode() == NVPTX::GOTO &&
+      LastInst.getOpcode() == NVPTX::GOTO) {
+    TBB = SecondLastInst.getOperand(0).getMBB();
     I = LastInst;
     if (AllowModify)
       I->eraseFromParent();
     return false;
   }
 
   // Otherwise, can't handle this.
   return true;

lib/Target/NVPTX/NVPTXPrologEpilogPass.cpp

 bool NVPTXPrologEpilogPass::runOnMachineFunction(MachineFunction &MF) {
   const TargetSubtargetInfo &STI = MF.getSubtarget();
   const TargetFrameLowering &TFI = *STI.getFrameLowering();
   const TargetRegisterInfo &TRI = *STI.getRegisterInfo();
   bool Modified = false;
 
   calculateFrameObjectOffsets(MF);
 
-  for (MachineFunction::iterator BB = MF.begin(), E = MF.end(); BB != E; ++BB) {
-    for (MachineBasicBlock::iterator I = BB->begin(); I != BB->end(); ++I) {
-      MachineInstr *MI = I;
-      for (unsigned i = 0, e = MI->getNumOperands(); i != e; ++i) {
-        if (!MI->getOperand(i).isFI())
+  for (MachineBasicBlock &MBB : MF) {
+    for (MachineInstr &MI : MBB) {
+      for (unsigned i = 0, e = MI.getNumOperands(); i != e; ++i) {
+        if (!MI.getOperand(i).isFI())
           continue;
         TRI.eliminateFrameIndex(MI, 0, i, nullptr);
         Modified = true;
       }
     }
   }
 
   // Add function prolog/epilog