[llvm-commits] [llvm] r170384 - in /llvm/trunk: include/llvm/CodeGen/MachineBasicBlock.h include/llvm/CodeGen/MachineInstr.h lib/CodeGen/MachineBasicBlock.cpp lib/CodeGen/MachineInstr.cpp
Sergei Larin
slarin at codeaurora.org
Thu Jan 3 11:20:58 PST 2013
Jakob,
What is supposed to happen in
MachineBasicBlock::instr_iterator
MachineBasicBlock::erase(MachineBasicBlock::instr_iterator I)
If it is "given" a bundle header itself?
Thanks.
Sergei
---
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by
The Linux Foundation
> -----Original Message-----
> From: llvm-commits-bounces at cs.uiuc.edu [mailto:llvm-commits-
> bounces at cs.uiuc.edu] On Behalf Of Jakob Stoklund Olesen
> Sent: Monday, December 17, 2012 5:56 PM
> To: llvm-commits at cs.uiuc.edu
> Subject: [llvm-commits] [llvm] r170384 - in /llvm/trunk:
> include/llvm/CodeGen/MachineBasicBlock.h
> include/llvm/CodeGen/MachineInstr.h lib/CodeGen/MachineBasicBlock.cpp
> lib/CodeGen/MachineInstr.cpp
>
> Author: stoklund
> Date: Mon Dec 17 17:55:38 2012
> New Revision: 170384
>
> URL: http://llvm.org/viewvc/llvm-project?rev=170384&view=rev
> Log:
> 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.
>
> Modified:
> llvm/trunk/include/llvm/CodeGen/MachineBasicBlock.h
> llvm/trunk/include/llvm/CodeGen/MachineInstr.h
> llvm/trunk/lib/CodeGen/MachineBasicBlock.cpp
> llvm/trunk/lib/CodeGen/MachineInstr.cpp
>
> Modified: llvm/trunk/include/llvm/CodeGen/MachineBasicBlock.h
> URL: http://llvm.org/viewvc/llvm-
> project/llvm/trunk/include/llvm/CodeGen/MachineBasicBlock.h?rev=170384&
> r1=170383&r2=170384&view=diff
> =======================================================================
> =======
> --- llvm/trunk/include/llvm/CodeGen/MachineBasicBlock.h (original)
> +++ llvm/trunk/include/llvm/CodeGen/MachineBasicBlock.h Mon Dec 17
> +++ 17:55:38 2012
> @@ -463,37 +463,57 @@
> 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();
> }
>
> Modified: llvm/trunk/include/llvm/CodeGen/MachineInstr.h
> URL: http://llvm.org/viewvc/llvm-
> project/llvm/trunk/include/llvm/CodeGen/MachineInstr.h?rev=170384&r1=17
> 0383&r2=170384&view=diff
> =======================================================================
> =======
> --- llvm/trunk/include/llvm/CodeGen/MachineInstr.h (original)
> +++ llvm/trunk/include/llvm/CodeGen/MachineInstr.h Mon Dec 17 17:55:38
> +++ 2012
> @@ -590,14 +590,33 @@
> 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 {
>
> Modified: llvm/trunk/lib/CodeGen/MachineBasicBlock.cpp
> URL: http://llvm.org/viewvc/llvm-
> project/llvm/trunk/lib/CodeGen/MachineBasicBlock.cpp?rev=170384&r1=1703
> 83&r2=170384&view=diff
> =======================================================================
> =======
> --- llvm/trunk/lib/CodeGen/MachineBasicBlock.cpp (original)
> +++ llvm/trunk/lib/CodeGen/MachineBasicBlock.cpp Mon Dec 17 17:55:38
> +++ 2012
> @@ -788,27 +788,30 @@
> 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());
> -}
> -
> -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);
> - }
> - }
> -
> - return Insts.remove(I);
> +/// 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.
> +}
> +
> +MachineBasicBlock::instr_iterator
> +MachineBasicBlock::erase(MachineBasicBlock::instr_iterator I) {
> + unbundleSingleMI(I);
> + return Insts.erase(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,
>
> Modified: llvm/trunk/lib/CodeGen/MachineInstr.cpp
> URL: http://llvm.org/viewvc/llvm-
> project/llvm/trunk/lib/CodeGen/MachineInstr.cpp?rev=170384&r1=170383&r2
> =170384&view=diff
> =======================================================================
> =======
> --- llvm/trunk/lib/CodeGen/MachineInstr.cpp (original)
> +++ llvm/trunk/lib/CodeGen/MachineInstr.cpp Mon Dec 17 17:55:38 2012
> @@ -838,46 +838,25 @@
> 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.
> ///
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list