Tighten up the erase/remove API for bundled instructions.

Most code is oblivious to bundles and uses the MBB::iterator which only
visits whole bundles. MBB::erase() operates on whole bundles at a time
as before.

MBB::remove() now refuses to remove bundled instructions. It is not safe
to remove all instructions in a bundle without deleting them since there
is no way of returning pointers to all the removed instructions.

MBB::remove_instr() and MBB::erase_instr() will now update bundle flags
correctly, lifting individual instructions out of bundles while leaving
the remaining bundle intact.

The MachineInstr convenience functions are updated so

  eraseFromParent() erases a whole bundle as before
  eraseFromBundle() erases a single instruction, leaving the rest of its bundle.
  removeFromParent() refuses to operate on bundled instructions, and
  removeFromBundle() lifts a single instruction out of its bundle.

These functions will no longer accidentally split or coalesce bundles -
bundle flags are updated to preserve the existing bundling, and explicit
bundleWith* / unbundleFrom* functions should be used to change the
instruction bundling.

This API update is still a work in progress. I am going to update APIs
first so they maintain bundle flags automatically when possible. Then
I'll add stricter verification of the bundle flags.

llvm-svn: 170384

include/llvm/CodeGen/MachineBasicBlock.h

@@ -463,37 +463,57 @@ public:
     return Insts.insertAfter(I.getInstrIterator(), M);
   }
 
-  /// erase - Remove the specified element or range from the instruction list.
-  /// These functions delete any instructions removed.
+  /// Remove an instruction from the instruction list and delete it.
   ///
-  instr_iterator erase(instr_iterator I) {
-    return Insts.erase(I);
-  }
-  instr_iterator erase(instr_iterator I, instr_iterator E) {
-    return Insts.erase(I, E);
-  }
+  /// If the instruction is part of a bundle, the other instructions in the
+  /// bundle will still be bundled after removing the single instruction.
+  instr_iterator erase(instr_iterator I);
+
+  /// Remove an instruction from the instruction list and delete it.
+  ///
+  /// If the instruction is part of a bundle, the other instructions in the
+  /// bundle will still be bundled after removing the single instruction.
   instr_iterator erase_instr(MachineInstr *I) {
-    instr_iterator MII(I);
-    return erase(MII);
+    return erase(instr_iterator(I));
   }
 
-  iterator erase(iterator I);
+  /// Remove a range of instructions from the instruction list and delete them.
   iterator erase(iterator I, iterator E) {
     return Insts.erase(I.getInstrIterator(), E.getInstrIterator());
   }
-  iterator erase(MachineInstr *I) {
-    iterator MII(I);
-    return erase(MII);
+
+  /// Remove an instruction or bundle from the instruction list and delete it.
+  ///
+  /// If I points to a bundle of instructions, they are all erased.
+  iterator erase(iterator I) {
+    return erase(I, llvm::next(I));
   }
 
-  /// remove - Remove the instruction from the instruction list. This function
-  /// does not delete the instruction. WARNING: Note, if the specified
-  /// instruction is a bundle this function will remove all the bundled
-  /// instructions as well. It is up to the caller to keep a list of the
-  /// bundled instructions and re-insert them if desired. This function is
-  /// *not recommended* for manipulating instructions with bundles. Use
-  /// splice instead.
-  MachineInstr *remove(MachineInstr *I);
+  /// Remove an instruction from the instruction list and delete it.
+  ///
+  /// If I is the head of a bundle of instructions, the whole bundle will be
+  /// erased.
+  iterator erase(MachineInstr *I) {
+    return erase(iterator(I));
+  }
+
+  /// Remove the unbundled instruction from the instruction list without
+  /// deleting it.
+  ///
+  /// This function can not be used to remove bundled instructions, use
+  /// remove_instr to remove individual instructions from a bundle.
+  MachineInstr *remove(MachineInstr *I) {
+    assert(!I->isBundled() && "Cannot remove bundled instructions");
+    return Insts.remove(I);
+  }
+
+  /// Remove the possibly bundled instruction from the instruction list
+  /// without deleting it.
+  ///
+  /// If the instruction is part of a bundle, the other instructions in the
+  /// bundle will still be bundled after removing the single instruction.
+  MachineInstr *remove_instr(MachineInstr *I);
+
   void clear() {
     Insts.clear();
   }

include/llvm/CodeGen/MachineInstr.h

@@ -590,14 +590,33 @@ public:
   bool isIdenticalTo(const MachineInstr *Other,
                      MICheckType Check = CheckDefs) const;
 
-  /// removeFromParent - This method unlinks 'this' from the containing basic
-  /// block, and returns it, but does not delete it.
+  /// Unlink 'this' from the containing basic block, and return it without
+  /// deleting it.
+  ///
+  /// This function can not be used on bundled instructions, use
+  /// removeFromBundle() to remove individual instructions from a bundle.
   MachineInstr *removeFromParent();
 
-  /// eraseFromParent - This method unlinks 'this' from the containing basic
-  /// block and deletes it.
+  /// Unlink this instruction from its basic block and return it without
+  /// deleting it.
+  ///
+  /// If the instruction is part of a bundle, the other instructions in the
+  /// bundle remain bundled.
+  MachineInstr *removeFromBundle();
+
+  /// Unlink 'this' from the containing basic block and delete it.
+  ///
+  /// If this instruction is the header of a bundle, the whole bundle is erased.
+  /// This function can not be used for instructions inside a bundle, use
+  /// eraseFromBundle() to erase individual bundled instructions.
   void eraseFromParent();
 
+  /// Unlink 'this' form its basic block and delete it.
+  ///
+  /// If the instruction is part of a bundle, the other instructions in the
+  /// bundle remain bundled.
+  void eraseFromBundle();
+
   /// isLabel - Returns true if the MachineInstr represents a label.
   ///
   bool isLabel() const {

lib/CodeGen/MachineBasicBlock.cpp

@@ -788,27 +788,30 @@ MachineBasicBlock::SplitCriticalEdge(MachineBasicBlock *Succ, Pass *P) {
   return NMBB;
 }
 
-MachineBasicBlock::iterator
-MachineBasicBlock::erase(MachineBasicBlock::iterator I) {
-  if (I->isBundle()) {
-    MachineBasicBlock::iterator E = llvm::next(I);
-    return Insts.erase(I.getInstrIterator(), E.getInstrIterator());
-  }
-
-  return Insts.erase(I.getInstrIterator());
+/// Prepare MI to be removed from its bundle. This fixes bundle flags on MI's
+/// neighboring instructions so the bundle won't be broken by removing MI.
+static void unbundleSingleMI(MachineInstr *MI) {
+  // Removing the first instruction in a bundle.
+  if (MI->isBundledWithSucc() && !MI->isBundledWithPred())
+    MI->unbundleFromSucc();
+  // Removing the last instruction in a bundle.
+  if (MI->isBundledWithPred() && !MI->isBundledWithSucc())
+    MI->unbundleFromPred();
+  // If MI is not bundled, or if it is internal to a bundle, the neighbor flags
+  // are already fine.
 }
 
-MachineInstr *MachineBasicBlock::remove(MachineInstr *I) {
-  if (I->isBundle()) {
-    instr_iterator MII = llvm::next(I);
-    iterator E = end();
-    while (MII != E && MII->isInsideBundle()) {
-      MachineInstr *MI = &*MII++;
-      Insts.remove(MI);
-    }
-  }
+MachineBasicBlock::instr_iterator
+MachineBasicBlock::erase(MachineBasicBlock::instr_iterator I) {
+  unbundleSingleMI(I);
+  return Insts.erase(I);
+}
 
-  return Insts.remove(I);
+MachineInstr *MachineBasicBlock::remove_instr(MachineInstr *MI) {
+  unbundleSingleMI(MI);
+  MI->clearFlag(MachineInstr::BundledPred);
+  MI->clearFlag(MachineInstr::BundledSucc);
+  return Insts.remove(MI);
 }
 
 void MachineBasicBlock::splice(MachineBasicBlock::iterator where,

lib/CodeGen/MachineInstr.cpp

@@ -838,46 +838,25 @@ bool MachineInstr::isIdenticalTo(const MachineInstr *Other,
   return true;
 }
 
-/// removeFromParent - This method unlinks 'this' from the containing basic
-/// block, and returns it, but does not delete it.
 MachineInstr *MachineInstr::removeFromParent() {
   assert(getParent() && "Not embedded in a basic block!");
-
-  // If it's a bundle then remove the MIs inside the bundle as well.
-  if (isBundle()) {
-    MachineBasicBlock *MBB = getParent();
-    MachineBasicBlock::instr_iterator MII = *this; ++MII;
-    MachineBasicBlock::instr_iterator E = MBB->instr_end();
-    while (MII != E && MII->isInsideBundle()) {
-      MachineInstr *MI = &*MII;
-      ++MII;
-      MBB->remove(MI);
-    }
-  }
-  getParent()->remove(this);
-  return this;
+  return getParent()->remove(this);
 }
 
+MachineInstr *MachineInstr::removeFromBundle() {
+  assert(getParent() && "Not embedded in a basic block!");
+  return getParent()->remove_instr(this);
+}
 
-/// eraseFromParent - This method unlinks 'this' from the containing basic
-/// block, and deletes it.
 void MachineInstr::eraseFromParent() {
   assert(getParent() && "Not embedded in a basic block!");
-  // If it's a bundle then remove the MIs inside the bundle as well.
-  if (isBundle()) {
-    MachineBasicBlock *MBB = getParent();
-    MachineBasicBlock::instr_iterator MII = *this; ++MII;
-    MachineBasicBlock::instr_iterator E = MBB->instr_end();
-    while (MII != E && MII->isInsideBundle()) {
-      MachineInstr *MI = &*MII;
-      ++MII;
-      MBB->erase(MI);
-    }
-  }
-  // Erase the individual instruction, which may itself be inside a bundle.
-  getParent()->erase_instr(this);
+  getParent()->erase(this);
 }
 
+void MachineInstr::eraseFromBundle() {
+  assert(getParent() && "Not embedded in a basic block!");
+  getParent()->erase_instr(this);
+}
 
 /// getNumExplicitOperands - Returns the number of non-implicit operands.
 ///