From 9f4692d2953b47e9037ccfe5709a6e75de3911d4 Mon Sep 17 00:00:00 2001 From: Jakob Stoklund Olesen Date: Mon, 17 Dec 2012 23:55:38 +0000 Subject: 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 --- include/llvm/CodeGen/MachineBasicBlock.h | 62 +++++++++++++++++++++----------- 1 file changed, 41 insertions(+), 21 deletions(-) (limited to 'include/llvm/CodeGen/MachineBasicBlock.h') diff --git a/include/llvm/CodeGen/MachineBasicBlock.h b/include/llvm/CodeGen/MachineBasicBlock.h index 81912742fa..91dcf1c4c3 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(); } -- cgit v1.2.3-70-g09d2 From edc3503ca5a4ab9133dd825dce4abd46bc4a3e08 Mon Sep 17 00:00:00 2001 From: Jakob Stoklund Olesen Date: Tue, 18 Dec 2012 17:54:53 +0000 Subject: Tighten the insert() API for bundled instructions. The normal insert() function takes an MBB::iterator position, and inserts a stand-alone MachineInstr as before. The insert() function that takes an MBB::instr_iterator position can insert instructions inside a bundle, and will now update the bundle flags correctly when that happens. When the insert position is between two bundles, it is unclear whether the instruction should be appended to the previous bundle, prepended to the next bundle, or stand on its own. The MBB::insert() function doesn't bundle the instruction in that case, use the MIBundleBuilder class for that. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@170437 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/CodeGen/MachineBasicBlock.h | 34 +++++++++++++++++++------------- lib/CodeGen/MachineBasicBlock.cpp | 12 +++++++++++ 2 files changed, 32 insertions(+), 14 deletions(-) (limited to 'include/llvm/CodeGen/MachineBasicBlock.h') diff --git a/include/llvm/CodeGen/MachineBasicBlock.h b/include/llvm/CodeGen/MachineBasicBlock.h index 91dcf1c4c3..0a12de318f 100644 --- a/include/llvm/CodeGen/MachineBasicBlock.h +++ b/include/llvm/CodeGen/MachineBasicBlock.h @@ -441,26 +441,32 @@ public: void pop_back() { Insts.pop_back(); } void push_back(MachineInstr *MI) { Insts.push_back(MI); } - template - void insert(instr_iterator I, IT S, IT E) { - Insts.insert(I, S, E); - } - instr_iterator insert(instr_iterator I, MachineInstr *M) { - return Insts.insert(I, M); - } - instr_iterator insertAfter(instr_iterator I, MachineInstr *M) { - return Insts.insertAfter(I, M); - } + /// Insert MI into the instruction list before I, possibly inside a bundle. + /// + /// If the insertion point is inside a bundle, MI will be added to the bundle, + /// otherwise MI will not be added to any bundle. That means this function + /// alone can't be used to prepend or append instructions to bundles. See + /// MIBundleBuilder::insert() for a more reliable way of doing that. + instr_iterator insert(instr_iterator I, MachineInstr *M); + /// Insert a range of instructions into the instruction list before I. template void insert(iterator I, IT S, IT E) { Insts.insert(I.getInstrIterator(), S, E); } - iterator insert(iterator I, MachineInstr *M) { - return Insts.insert(I.getInstrIterator(), M); + + /// Insert MI into the instruction list before I. + iterator insert(iterator I, MachineInstr *MI) { + assert(!MI->isBundledWithPred() && !MI->isBundledWithSucc() && + "Cannot insert instruction with bundle flags"); + return Insts.insert(I.getInstrIterator(), MI); } - iterator insertAfter(iterator I, MachineInstr *M) { - return Insts.insertAfter(I.getInstrIterator(), M); + + /// Insert MI into the instruction list after I. + iterator insertAfter(iterator I, MachineInstr *MI) { + assert(!MI->isBundledWithPred() && !MI->isBundledWithSucc() && + "Cannot insert instruction with bundle flags"); + return Insts.insertAfter(I.getInstrIterator(), MI); } /// Remove an instruction from the instruction list and delete it. diff --git a/lib/CodeGen/MachineBasicBlock.cpp b/lib/CodeGen/MachineBasicBlock.cpp index b4ff6b4fcf..027f777fcf 100644 --- a/lib/CodeGen/MachineBasicBlock.cpp +++ b/lib/CodeGen/MachineBasicBlock.cpp @@ -814,6 +814,18 @@ MachineInstr *MachineBasicBlock::remove_instr(MachineInstr *MI) { return Insts.remove(MI); } +MachineBasicBlock::instr_iterator +MachineBasicBlock::insert(instr_iterator I, MachineInstr *MI) { + assert(!MI->isBundledWithPred() && !MI->isBundledWithSucc() && + "Cannot insert instruction with bundle flags"); + // Set the bundle flags when inserting inside a bundle. + if (I != instr_end() && I->isBundledWithPred()) { + MI->setFlag(MachineInstr::BundledPred); + MI->setFlag(MachineInstr::BundledSucc); + } + return Insts.insert(I, MI); +} + void MachineBasicBlock::splice(MachineBasicBlock::iterator where, MachineBasicBlock *Other, MachineBasicBlock::iterator From) { -- cgit v1.2.3-70-g09d2 From 9b04104a5e9fb51b24b7aeb55912d319802049b2 Mon Sep 17 00:00:00 2001 From: Jakob Stoklund Olesen Date: Tue, 18 Dec 2012 20:59:41 +0000 Subject: Tighten up the splice() API for bundled instructions. Remove the instr_iterator versions of the splice() functions. It doesn't seem useful to be able to splice sequences of instructions that don't consist of full bundles. The normal splice functions that take MBB::iterator arguments are not changed, and they can move whole bundles around without any problems. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@170456 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/CodeGen/MachineBasicBlock.h | 31 ++++++++++++++++--------------- lib/CodeGen/MachineBasicBlock.cpp | 13 ------------- 2 files changed, 16 insertions(+), 28 deletions(-) (limited to 'include/llvm/CodeGen/MachineBasicBlock.h') diff --git a/include/llvm/CodeGen/MachineBasicBlock.h b/include/llvm/CodeGen/MachineBasicBlock.h index 0a12de318f..1225c8a99e 100644 --- a/include/llvm/CodeGen/MachineBasicBlock.h +++ b/include/llvm/CodeGen/MachineBasicBlock.h @@ -524,23 +524,24 @@ public: Insts.clear(); } - /// splice - Take an instruction from MBB 'Other' at the position From, - /// and insert it into this MBB right before 'where'. - void splice(instr_iterator where, MachineBasicBlock *Other, - instr_iterator From) { - Insts.splice(where, Other->Insts, From); + /// Take an instruction from MBB 'Other' at the position From, and insert it + /// into this MBB right before 'Where'. + /// + /// If From points to a bundle of instructions, the whole bundle is moved. + void splice(iterator Where, MachineBasicBlock *Other, iterator From) { + // The range splice() doesn't allow noop moves, but this one does. + if (Where != From) + splice(Where, Other, From, llvm::next(From)); } - void splice(iterator where, MachineBasicBlock *Other, iterator From); - /// splice - Take a block of instructions from MBB 'Other' in the range [From, - /// To), and insert them into this MBB right before 'where'. - void splice(instr_iterator where, MachineBasicBlock *Other, instr_iterator From, - instr_iterator To) { - Insts.splice(where, Other->Insts, From, To); - } - void splice(iterator where, MachineBasicBlock *Other, iterator From, - iterator To) { - Insts.splice(where.getInstrIterator(), Other->Insts, + /// Take a block of instructions from MBB 'Other' in the range [From, To), + /// and insert them into this MBB right before 'Where'. + /// + /// The instruction at 'Where' must not be included in the range of + /// instructions to move. + void splice(iterator Where, MachineBasicBlock *Other, + iterator From, iterator To) { + Insts.splice(Where.getInstrIterator(), Other->Insts, From.getInstrIterator(), To.getInstrIterator()); } diff --git a/lib/CodeGen/MachineBasicBlock.cpp b/lib/CodeGen/MachineBasicBlock.cpp index 027f777fcf..207f21579c 100644 --- a/lib/CodeGen/MachineBasicBlock.cpp +++ b/lib/CodeGen/MachineBasicBlock.cpp @@ -826,19 +826,6 @@ MachineBasicBlock::insert(instr_iterator I, MachineInstr *MI) { return Insts.insert(I, MI); } -void MachineBasicBlock::splice(MachineBasicBlock::iterator where, - MachineBasicBlock *Other, - MachineBasicBlock::iterator From) { - if (From->isBundle()) { - MachineBasicBlock::iterator To = llvm::next(From); - Insts.splice(where.getInstrIterator(), Other->Insts, - From.getInstrIterator(), To.getInstrIterator()); - return; - } - - Insts.splice(where.getInstrIterator(), Other->Insts, From.getInstrIterator()); -} - /// removeFromParent - This method unlinks 'this' from the containing function, /// and returns it, but does not delete it. MachineBasicBlock *MachineBasicBlock::removeFromParent() { -- cgit v1.2.3-70-g09d2 From 2e4b639790a166e55a0bf14fac54ca6ce583e459 Mon Sep 17 00:00:00 2001 From: Jakob Stoklund Olesen Date: Tue, 18 Dec 2012 23:21:49 +0000 Subject: Use bidirectional bundle flags to simplify important functions. The bundle_iterator::operator++ function now doesn't need to dig out the basic block and check against end(). It can use the isBundledWithSucc() flag to find the last bundled instruction safely. Similarly, MachineInstr::isBundled() no longer needs to look at iterators etc. It only has to look at flags. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@170473 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/CodeGen/MachineBasicBlock.h | 12 ++++++------ include/llvm/CodeGen/MachineInstr.h | 4 +++- lib/CodeGen/MachineInstr.cpp | 10 ---------- 3 files changed, 9 insertions(+), 17 deletions(-) (limited to 'include/llvm/CodeGen/MachineBasicBlock.h') diff --git a/include/llvm/CodeGen/MachineBasicBlock.h b/include/llvm/CodeGen/MachineBasicBlock.h index 1225c8a99e..492a3ff49f 100644 --- a/include/llvm/CodeGen/MachineBasicBlock.h +++ b/include/llvm/CodeGen/MachineBasicBlock.h @@ -146,11 +146,11 @@ public: bundle_iterator(IterTy mii) : MII(mii) {} bundle_iterator(Ty &mi) : MII(mi) { - assert(!mi.isInsideBundle() && + assert(!mi.isBundledWithPred() && "It's not legal to initialize bundle_iterator with a bundled MI"); } bundle_iterator(Ty *mi) : MII(mi) { - assert((!mi || !mi->isInsideBundle()) && + assert((!mi || !mi->isBundledWithPred()) && "It's not legal to initialize bundle_iterator with a bundled MI"); } // Template allows conversion from const to nonconst. @@ -174,13 +174,13 @@ public: // Increment and decrement operators... bundle_iterator &operator--() { // predecrement - Back up do --MII; - while (MII->isInsideBundle()); + while (MII->isBundledWithPred()); return *this; } bundle_iterator &operator++() { // preincrement - Advance - IterTy E = MII->getParent()->instr_end(); - do ++MII; - while (MII != E && MII->isInsideBundle()); + while (MII->isBundledWithSucc()) + ++MII; + ++MII; return *this; } bundle_iterator operator--(int) { // postdecrement operators... diff --git a/include/llvm/CodeGen/MachineInstr.h b/include/llvm/CodeGen/MachineInstr.h index c7b8360471..2313e4404b 100644 --- a/include/llvm/CodeGen/MachineInstr.h +++ b/include/llvm/CodeGen/MachineInstr.h @@ -211,7 +211,9 @@ public: /// isBundled - Return true if this instruction part of a bundle. This is true /// if either itself or its following instruction is marked "InsideBundle". - bool isBundled() const; + bool isBundled() const { + return isBundledWithPred() || isBundledWithSucc(); + } /// Return true if this instruction is part of a bundle, and it is not the /// first instruction in the bundle. diff --git a/lib/CodeGen/MachineInstr.cpp b/lib/CodeGen/MachineInstr.cpp index 3b83d02bb0..ce77d61460 100644 --- a/lib/CodeGen/MachineInstr.cpp +++ b/lib/CodeGen/MachineInstr.cpp @@ -909,16 +909,6 @@ void MachineInstr::unbundleFromSucc() { Succ->clearFlag(BundledPred); } -/// isBundled - Return true if this instruction part of a bundle. This is true -/// if either itself or its following instruction is marked "InsideBundle". -bool MachineInstr::isBundled() const { - if (isInsideBundle()) - return true; - MachineBasicBlock::const_instr_iterator nextMI = this; - ++nextMI; - return nextMI != Parent->instr_end() && nextMI->isInsideBundle(); -} - bool MachineInstr::isStackAligningInlineAsm() const { if (isInlineAsm()) { unsigned ExtraInfo = getOperand(InlineAsm::MIOp_ExtraInfo).getImm(); -- cgit v1.2.3-70-g09d2