llvm dev - Jul 2004 - [LLVMdev] MRegisterInfo::eliminateFrameIndex

The docs for the above-mentioned function say:

   This method may modify or replace the specified instruction, as long as it   
   keeps the iterator pointing the the finished product.

What does it mean to "keep an interator". Was "invalidates the
iterator"
intended, so something else. 

Another question, is how do I really replace the instruction. The operator= is 
private and erase/insert might well invalidate the iterator.

As it stands, I need to manually change all the operands and opcode of the 
instruction, which is not convenient.

Also, why RemoveOperand and SetMachineOperand methods in MachineInstr are the 
only methods in that class which start with uppercase letter?

- Volodya

On Thu, 1 Jul 2004, Vladimir Prus wrote:
> The docs for the above-mentioned function say:
>
>    This method may modify or replace the specified instruction, as long as
it
>    keeps the iterator pointing the the finished product.
>
> What does it mean to "keep an interator". Was "invalidates
the iterator"
> intended, so something else.
Hrm, this method appears to be broken.  The basic point is that it wants
you to be able to delete the instruction and replace it with something
else if needed.  If this happens, the iterator passed in will be invalid
(pointing to a deleted instruction) so the caller needs a new iterator to
the new instruction to carry on its job with.  However, the iterator is
passed in by value, so there is no way for the implementation to do this!

It looks like the method should be changed to return a new MBB::iterator,
which points to the resultant instruction, other the preexisting one, or a
replacement.
> Another question, is how do I really replace the instruction. The operator=
is
> private and erase/insert might well invalidate the iterator.
Instructions are on a doubly linked list, so all of the iterator
invalidation semantics follow that of a linked list.  The funny thing abt
this is that it's not an std::list, but an LLVM ilist (the basic
difference being that the next/prev pointers go in the object).  Because
of that, you have to use ilist methods like "remove" (take an
instruction
out of the list) or "erase" (take it out and delete it).  If you
don't
want to lose your position in the chain of instructions, erase returns an
iterator, so you can do this:

MachineBasicBlock::iterator I = MBB.erase(MI);

The returned iterator points to the instruction after MI.
> As it stands, I need to manually change all the operands and opcode of the
> instruction, which is not convenient.
Ick. :)

Also note that there is a version of BuildMI that allows you to pass in a
MBB& and MBB::iterator to specify where to autoinsert into.  Combined with
the erase method above, this makes it pretty easy to do the stuff you
need.
> Also, why RemoveOperand and SetMachineOperand methods in MachineInstr are
the
> only methods in that class which start with uppercase letter?
Uhhh... historical artifacts.  They need to be changed.  The MachineInstr
and MachineOperand classes both need to be cleaned up quite a bit.

-Chris

-- 
http://llvm.cs.uiuc.edu/
http://www.nondot.org/~sabre/Projects/