From: Jakob Stoklund Olesen Date: Mon, 17 Dec 2012 23:55:38 +0000 (+0000) Subject: Tighten up the erase/remove API for bundled instructions. X-Git-Url: http://plrg.eecs.uci.edu/git/?a=commitdiff_plain;h=9f4692d2953b47e9037ccfe5709a6e75de3911d4;p=oota-llvm.git 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. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@170384 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/llvm/CodeGen/MachineBasicBlock.h b/include/llvm/CodeGen/MachineBasicBlock.h index 81912742fae..91dcf1c4c39 100644 --- a/include/llvm/CodeGen/MachineBasicBlock.h +++ b/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()); } + + /// 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 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) { - iterator MII(I); - return erase(MII); + return erase(iterator(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 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(); } diff --git a/include/llvm/CodeGen/MachineInstr.h b/include/llvm/CodeGen/MachineInstr.h index 57da779ca12..320cd0dc53d 100644 --- a/include/llvm/CodeGen/MachineInstr.h +++ b/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 { diff --git a/lib/CodeGen/MachineBasicBlock.cpp b/lib/CodeGen/MachineBasicBlock.cpp index 48008537e75..b4ff6b4fcfc 100644 --- a/lib/CodeGen/MachineBasicBlock.cpp +++ b/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, diff --git a/lib/CodeGen/MachineInstr.cpp b/lib/CodeGen/MachineInstr.cpp index 3aebdcdabbe..f545a9ce56d 100644 --- a/lib/CodeGen/MachineInstr.cpp +++ b/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. ///