MSP430: Avoid implicit iterator conversions, NFC

[dexonsmith]

Summary

Avoid implicit conversions from MachineInstrBundleInstr to MachineInstr*
in the MSP430 backend by preferring MachineInstr& over MachineInstr*
when a pointer isn't nullable.

[dexonsmith]

FYI: these diffs are identical; the re-upload just has extra context to make Phab users happy.

[asl]

LGTM from me as well.

[dexonsmith]

Thanks both for the review. Committed in r274933.

lib/Target/MSP430/MSP430BranchSelector.cpp

bool MSP430BSel::runOnMachineFunction(MachineFunction &Fn) {
   const MSP430InstrInfo *TII =
       static_cast<const MSP430InstrInfo *>(Fn.getSubtarget().getInstrInfo());
   // Give the blocks of the function a dense, in-order, numbering.
   Fn.RenumberBlocks();
   BlockSizes.resize(Fn.getNumBlockIDs());
 
   // Measure each MBB and compute a size for the entire function.
   unsigned FuncSize = 0;
-  for (MachineFunction::iterator MFI = Fn.begin(), E = Fn.end(); MFI != E;
-       ++MFI) {
-    MachineBasicBlock *MBB = &*MFI;
-
+  for (MachineBasicBlock &MBB : Fn) {
     unsigned BlockSize = 0;
-    for (MachineBasicBlock::iterator MBBI = MBB->begin(), EE = MBB->end();
-         MBBI != EE; ++MBBI)
-      BlockSize += TII->GetInstSizeInBytes(MBBI);
+    for (MachineInstr &MI : MBB)
+      BlockSize += TII->GetInstSizeInBytes(MI);
 
-    BlockSizes[MBB->getNumber()] = BlockSize;
+    BlockSizes[MBB.getNumber()] = BlockSize;
     FuncSize += BlockSize;
   }
 
   // If the entire function is smaller than the displacement of a branch field,
   // we know we don't need to shrink any branches in this function.  This is a
   // common case.
   if (FuncSize < (1 << 9)) {
     BlockSizes.clear();
while (MadeChange) {
     for (MachineFunction::iterator MFI = Fn.begin(), E = Fn.end(); MFI != E;
          ++MFI) {
       MachineBasicBlock &MBB = *MFI;
       unsigned MBBStartOffset = 0;
       for (MachineBasicBlock::iterator I = MBB.begin(), E = MBB.end();
            I != E; ++I) {
         if ((I->getOpcode() != MSP430::JCC || I->getOperand(0).isImm()) &&
             I->getOpcode() != MSP430::JMP) {
-          MBBStartOffset += TII->GetInstSizeInBytes(I);
+          MBBStartOffset += TII->GetInstSizeInBytes(*I);
           continue;
         }
 
         // Determine the offset from the current branch to the destination
         // block.
         MachineBasicBlock *Dest = I->getOperand(0).getMBB();
 
         int BranchSize;
for (MachineFunction::iterator MFI = Fn.begin(), E = Fn.end(); MFI != E;
         // If this branch is in range, ignore it.
         if (isInt<10>(BranchSize)) {
           MBBStartOffset += 2;
           continue;
         }
 
         // Otherwise, we have to expand it to a long branch.
         unsigned NewSize;
-        MachineInstr *OldBranch = I;
-        DebugLoc dl = OldBranch->getDebugLoc();
+        MachineInstr &OldBranch = *I;
+        DebugLoc dl = OldBranch.getDebugLoc();
 
         if (I->getOpcode() == MSP430::JMP) {
           NewSize = 4;
         } else {
           // The BCC operands are:
           // 0. MSP430 branch predicate
           // 1. Target MBB
           SmallVector<MachineOperand, 1> Cond;
           Cond.push_back(I->getOperand(1));
 
           // Jump over the uncond branch inst (i.e. $+6) on opposite condition.
           TII->ReverseBranchCondition(Cond);
           BuildMI(MBB, I, dl, TII->get(MSP430::JCC))
             .addImm(4).addOperand(Cond[0]);
 
           NewSize = 6;
         }
         // Uncond branch to the real destination.
         I = BuildMI(MBB, I, dl, TII->get(MSP430::Bi)).addMBB(Dest);
 
         // Remove the old branch from the function.
-        OldBranch->eraseFromParent();
+        OldBranch.eraseFromParent();
 
         // Remember that this instruction is NewSize bytes, increase the size of the
         // block by NewSize-2, remember to iterate.
         BlockSizes[MBB.getNumber()] += NewSize-2;
         MBBStartOffset += NewSize;
 
         ++NumExpanded;
         MadeChange = true;
       }
     }
     EverMadeChange |= MadeChange;
   }
 
   BlockSizes.clear();
   return true;
 }

lib/Target/MSP430/MSP430FrameLowering.cpp

const MSP430InstrInfo &TII =
       *static_cast<const MSP430InstrInfo *>(MF.getSubtarget().getInstrInfo());
   unsigned StackAlign = getStackAlignment();
 
   if (!hasReservedCallFrame(MF)) {
     // If the stack pointer can be changed after prologue, turn the
     // adjcallstackup instruction into a 'sub SP, <amt>' and the
     // adjcallstackdown instruction into 'add SP, <amt>'
     // TODO: consider using push / pop instead of sub + store / add
-    MachineInstr *Old = I;
-    uint64_t Amount = Old->getOperand(0).getImm();
+    MachineInstr &Old = *I;
+    uint64_t Amount = Old.getOperand(0).getImm();
     if (Amount != 0) {
       // We need to keep the stack aligned properly.  To do this, we round the
       // amount of space needed for the outgoing arguments up to the next
       // alignment boundary.
       Amount = (Amount+StackAlign-1)/StackAlign*StackAlign;
 
       MachineInstr *New = nullptr;
-      if (Old->getOpcode() == TII.getCallFrameSetupOpcode()) {
-        New = BuildMI(MF, Old->getDebugLoc(),
-                      TII.get(MSP430::SUB16ri), MSP430::SP)
-          .addReg(MSP430::SP).addImm(Amount);
+      if (Old.getOpcode() == TII.getCallFrameSetupOpcode()) {
+        New =
+            BuildMI(MF, Old.getDebugLoc(), TII.get(MSP430::SUB16ri), MSP430::SP)
+                .addReg(MSP430::SP)
+                .addImm(Amount);
       } else {
-        assert(Old->getOpcode() == TII.getCallFrameDestroyOpcode());
+        assert(Old.getOpcode() == TII.getCallFrameDestroyOpcode());
         // factor out the amount the callee already popped.
-        uint64_t CalleeAmt = Old->getOperand(1).getImm();
+        uint64_t CalleeAmt = Old.getOperand(1).getImm();
         Amount -= CalleeAmt;
         if (Amount)
-          New = BuildMI(MF, Old->getDebugLoc(),
-                        TII.get(MSP430::ADD16ri), MSP430::SP)
-            .addReg(MSP430::SP).addImm(Amount);
+          New = BuildMI(MF, Old.getDebugLoc(), TII.get(MSP430::ADD16ri),
+                        MSP430::SP)
+                    .addReg(MSP430::SP)
+                    .addImm(Amount);
       }
 
       if (New) {
         // The SRW implicit def is dead.
         New->getOperand(3).setIsDead();
 
         // Replace the pseudo instruction with a new instruction...
         MBB.insert(I, New);
       }
     }
   } else if (I->getOpcode() == TII.getCallFrameDestroyOpcode()) {
     // If we are performing frame pointer elimination and if the callee pops
     // something off the stack pointer, add it back.
     if (uint64_t CalleeAmt = I->getOperand(1).getImm()) {
-      MachineInstr *Old = I;
+      MachineInstr &Old = *I;
       MachineInstr *New =
-        BuildMI(MF, Old->getDebugLoc(), TII.get(MSP430::SUB16ri),
-                MSP430::SP).addReg(MSP430::SP).addImm(CalleeAmt);
+          BuildMI(MF, Old.getDebugLoc(), TII.get(MSP430::SUB16ri), MSP430::SP)
+              .addReg(MSP430::SP)
+              .addImm(CalleeAmt);
       // The SRW implicit def is dead.
       New->getOperand(3).setIsDead();
 
       MBB.insert(I, New);
     }
   }
 
   return MBB.erase(I);

lib/Target/MSP430/MSP430InstrInfo.h

void storeRegToStackSlot(MachineBasicBlock &MBB,
                            const TargetRegisterClass *RC,
                            const TargetRegisterInfo *TRI) const override;
   void loadRegFromStackSlot(MachineBasicBlock &MBB,
                             MachineBasicBlock::iterator MI,
                             unsigned DestReg, int FrameIdx,
                             const TargetRegisterClass *RC,
                             const TargetRegisterInfo *TRI) const override;
 
-  unsigned GetInstSizeInBytes(const MachineInstr *MI) const;
+  unsigned GetInstSizeInBytes(const MachineInstr &MI) const;
 
   // Branch folding goodness
   bool
   ReverseBranchCondition(SmallVectorImpl<MachineOperand> &Cond) const override;
   bool isUnpredicatedTerminator(const MachineInstr &MI) const override;
   bool AnalyzeBranch(MachineBasicBlock &MBB,
                      MachineBasicBlock *&TBB, MachineBasicBlock *&FBB,
                      SmallVectorImpl<MachineOperand> &Cond,

lib/Target/MSP430/MSP430InstrInfo.cpp

if (FBB) {
     ++Count;
   }
   return Count;
 }
 
 /// GetInstSize - Return the number of bytes of code the specified
 /// instruction may be.  This returns the maximum number of bytes.
 ///
-unsigned MSP430InstrInfo::GetInstSizeInBytes(const MachineInstr *MI) const {
-  const MCInstrDesc &Desc = MI->getDesc();
+unsigned MSP430InstrInfo::GetInstSizeInBytes(const MachineInstr &MI) const {
+  const MCInstrDesc &Desc = MI.getDesc();
 
   switch (Desc.TSFlags & MSP430II::SizeMask) {
   default:
     switch (Desc.getOpcode()) {
     default: llvm_unreachable("Unknown instruction size!");
     case TargetOpcode::CFI_INSTRUCTION:
     case TargetOpcode::EH_LABEL:
     case TargetOpcode::IMPLICIT_DEF:
     case TargetOpcode::KILL:
     case TargetOpcode::DBG_VALUE:
       return 0;
     case TargetOpcode::INLINEASM: {
-      const MachineFunction *MF = MI->getParent()->getParent();
+      const MachineFunction *MF = MI.getParent()->getParent();
       const TargetInstrInfo &TII = *MF->getSubtarget().getInstrInfo();
-      return TII.getInlineAsmLength(MI->getOperand(0).getSymbolName(),
+      return TII.getInlineAsmLength(MI.getOperand(0).getSymbolName(),
                                     *MF->getTarget().getMCAsmInfo());
     }
     }
   case MSP430II::SizeSpecial:
-    switch (MI->getOpcode()) {
+    switch (MI.getOpcode()) {
     default: llvm_unreachable("Unknown instruction size!");
     case MSP430::SAR8r1c:
     case MSP430::SAR16r1c:
       return 4;
     }
   case MSP430II::Size2Bytes:
     return 2;
   case MSP430II::Size4Bytes:
     return 4;
   case MSP430II::Size6Bytes:
     return 6;
   }
 }