[MIBundle] Turn MachineOperandIteratorBase into a forward iterator.

[fhahn]

Summary

This patch turns MachineOperandIteratorBase into a regular forward
iterator, which can be used with iterator_range.

It also adds mi_bundle_ops and const_mi_bundle_ops that return iterator
ranges over all operands in a bundle and updates a use of the old
iterator.

[fhahn]

ping

[arsenm]

LGTM

llvm/include/llvm/CodeGen/MachineInstrBundle.h

 /// Intended use:
 ///
 ///   for (MIBundleOperands MIO(MI); MIO.isValid(); ++MIO) {
 ///     if (!MIO->isReg())
 ///       continue;
 ///     ...
 ///   }
 ///
-class MIBundleOperandIteratorBase {
+template <typename ValueT>
+class MIBundleOperandIteratorBase
+    : public iterator_facade_base<MIBundleOperandIteratorBase<ValueT>,
+                                  std::forward_iterator_tag, ValueT> {
   MachineBasicBlock::instr_iterator InstrI, InstrE;
   MachineInstr::mop_iterator OpI, OpE;
 
   // If the operands on InstrI are exhausted, advance InstrI to the next
   // bundled instruction with operands.
   void advance() {
     while (OpI == OpE) {
       // Don't advance off the basic block, or into a new bundle.
-      if (++InstrI == InstrE || !InstrI->isInsideBundle())
+      if (++InstrI == InstrE || !InstrI->isInsideBundle()) {
+        InstrI = InstrE;
         break;
+      }
       OpI = InstrI->operands_begin();
       OpE = InstrI->operands_end();
     }
   }
 
 protected:
   /// MIBundleOperandIteratorBase - Create an iterator that visits all operands
   /// on MI, or all operands on every instruction in the bundle containing MI.
   ///
   /// @param MI The instruction to examine.
   ///
   explicit MIBundleOperandIteratorBase(MachineInstr &MI) {
     InstrI = getBundleStart(MI.getIterator());
     InstrE = MI.getParent()->instr_end();
     OpI = InstrI->operands_begin();
     OpE = InstrI->operands_end();
     advance();
   }
 
-  MachineOperand &deref() const { return *OpI; }
+  /// Constructor for an iterator past the last iteration: both instruction
+  /// iterators point to the end of the BB and OpI == OpE.
+  explicit MIBundleOperandIteratorBase(MachineBasicBlock::instr_iterator InstrE,
+                                       MachineInstr::mop_iterator OpE)
+      : InstrI(InstrE), InstrE(InstrE), OpI(OpE), OpE(OpE) {}
 
 public:
   /// isValid - Returns true until all the operands have been visited.
   bool isValid() const { return OpI != OpE; }
 
   /// Preincrement.  Move to the next operand.
   void operator++() {
     assert(isValid() && "Cannot advance MIOperands beyond the last operand");
     ++OpI;
     advance();
   }
 
+  ValueT &operator*() const { return *OpI; }
+  ValueT *operator->() const { return &*OpI; }
+
+  bool operator==(const MIBundleOperandIteratorBase &Arg) const {
+    // Iterators are equal, if InstrI matches and either OpIs match or OpI ==
+    // OpE match for both. The second condition allows us to construct an 'end'
+    // iterator, without finding the last instruction in a bundle up-front.
+    return InstrI == Arg.InstrI &&
+           (OpI == Arg.OpI || (OpI == OpE && Arg.OpI == Arg.OpE));
+  }
   /// getOperandNo - Returns the number of the current operand relative to its
   /// instruction.
   ///
   unsigned getOperandNo() const {
     return OpI - InstrI->operands_begin();
   }
 };
 
 /// MIBundleOperands - Iterate over all operands in a bundle of machine
 /// instructions.
 ///
-class MIBundleOperands : public MIBundleOperandIteratorBase {
+class MIBundleOperands : public MIBundleOperandIteratorBase<MachineOperand> {
+  /// Constructor for an iterator past the last iteration.
+  MIBundleOperands(MachineBasicBlock::instr_iterator InstrE,
+                   MachineInstr::mop_iterator OpE)
+      : MIBundleOperandIteratorBase(InstrE, OpE) {}
+
 public:
   MIBundleOperands(MachineInstr &MI) : MIBundleOperandIteratorBase(MI) {}
-  MachineOperand &operator* () const { return deref(); }
-  MachineOperand *operator->() const { return &deref(); }
+
+  /// Returns an iterator past the last iteration.
+  static MIBundleOperands end(const MachineBasicBlock &MBB) {
+    return {const_cast<MachineBasicBlock &>(MBB).instr_end(),
+            const_cast<MachineBasicBlock &>(MBB).instr_begin()->operands_end()};
+  }
 };
 
 /// ConstMIBundleOperands - Iterate over all operands in a const bundle of
 /// machine instructions.
 ///
-class ConstMIBundleOperands : public MIBundleOperandIteratorBase {
+class ConstMIBundleOperands
+    : public MIBundleOperandIteratorBase<const MachineOperand> {
+
+  /// Constructor for an iterator past the last iteration.
+  ConstMIBundleOperands(MachineBasicBlock::instr_iterator InstrE,
+                        MachineInstr::mop_iterator OpE)
+      : MIBundleOperandIteratorBase(InstrE, OpE) {}
+
 public:
   ConstMIBundleOperands(const MachineInstr &MI)
       : MIBundleOperandIteratorBase(const_cast<MachineInstr &>(MI)) {}
-  const MachineOperand &operator* () const { return deref(); }
-  const MachineOperand *operator->() const { return &deref(); }
+
+  /// Returns an iterator past the last iteration.
+  static ConstMIBundleOperands end(const MachineBasicBlock &MBB) {
+    return {const_cast<MachineBasicBlock &>(MBB).instr_end(),
+            const_cast<MachineBasicBlock &>(MBB).instr_begin()->operands_end()};
+  }
 };
 
+inline iterator_range<ConstMIBundleOperands>
+const_mi_bundle_ops(const MachineInstr &MI) {
+  return make_range(ConstMIBundleOperands(MI),
+                    ConstMIBundleOperands::end(*MI.getParent()));
+}
+
+inline iterator_range<MIBundleOperands> mi_bundle_ops(MachineInstr &MI) {
+  return make_range(MIBundleOperands(MI),
+                    MIBundleOperands::end(*MI.getParent()));
+}
+
 /// VirtRegInfo - Information about a virtual register used by a set of
 /// operands.
 ///
 struct VirtRegInfo {
   /// Reads - One of the operands read the virtual register.  This does not
   /// include undef or internal use operands, see MO::readsReg().
   bool Reads;
 

llvm/lib/CodeGen/LiveIntervals.cpp

void handleMoveDown(LiveRange &LR) {
     if (SlotIndex::isEarlierInstr(OldIdxIn->start, OldIdx)) {
       // If the live-in value already extends to NewIdx, there is nothing to do.
       if (SlotIndex::isEarlierEqualInstr(NewIdx, OldIdxIn->end))
         return;
       // Aggressively remove all kill flags from the old kill point.
       // Kill flags shouldn't be used while live intervals exist, they will be
       // reinserted by VirtRegRewriter.
       if (MachineInstr *KillMI = LIS.getInstructionFromIndex(OldIdxIn->end))
-        for (MIBundleOperands MO(*KillMI); MO.isValid(); ++MO)
-          if (MO->isReg() && MO->isUse())
-            MO->setIsKill(false);
+        for (MachineOperand &MOP : mi_bundle_ops(*KillMI))
+          if (MOP.isReg() && MOP.isUse())
+            MOP.setIsKill(false);
 
       // Is there a def before NewIdx which is not OldIdx?
       LiveRange::iterator Next = std::next(OldIdxIn);
       if (Next != E && !SlotIndex::isSameInstr(OldIdx, Next->start) &&
           SlotIndex::isEarlierInstr(Next->start, NewIdx)) {
         // If we are here then OldIdx was just a use but not a def. We only have
         // to ensure liveness extends to NewIdx.
         LiveRange::iterator NewIdxIn =