llvm dev - Jun 2006 - [LLVMdev] [PATCH] BasicBlock::getFirstNonPHI

Hi,
everytime one has to add instruction at the beginning of a basic block, one
has to skip past PHI nodes that are already there. How about adding a new
method to BasicBlock, to get that first non-PHI instruction? So, adding an
instruction will be as simple as:

      new SomeInstruction(............., BB->getFirstNonPHI())

Patch attached. Comments?

- Volodya
-------------- next part --------------
A non-text attachment was scrubbed...
Name: BasicBlock.diff
Type: text/x-diff
Size: 1679 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20060604/de97f530/attachment.diff>

On Sun, 4 Jun 2006, Vladimir Prus wrote:
> everytime one has to add instruction at the beginning of a basic block, one
> has to skip past PHI nodes that are already there. How about adding a new
> method to BasicBlock, to get that first non-PHI instruction? So, adding an
> instruction will be as simple as:
>
>      new SomeInstruction(............., BB->getFirstNonPHI())
Sure, sounds good.  A couple requests:

1. Please add a const version that returns a const Instruction* also.
2. There is no need to check for running off the end of the basic block, 
you are guaranteed that a block has a terminator, which is not a PHI.

-Chris

-- 
http://nondot.org/sabre/
http://llvm.org/

Chris Lattner wrote:
> On Sun, 4 Jun 2006, Vladimir Prus wrote:
> 
>> everytime one has to add instruction at the beginning of a basic
>> block, one
>> has to skip past PHI nodes that are already there. How about adding a
new
>> method to BasicBlock, to get that first non-PHI instruction? So,
>> adding an
>> instruction will be as simple as:
>>
>>      new SomeInstruction(............., BB->getFirstNonPHI())
> 
> 2. There is no need to check for running off the end of the basic block,
> you are guaranteed that a block has a terminator, which is not a PHI.
Assuming it's a valid BasicBlock. Which, if I understand correctly,
isn't always true in mid-transformation. Perhaps you should turn it into
an assert instead, to minimize surprises?

I can't check if this actually happens with llvm.org seemingly down, but
I do recall a situation like that. Maybe it was the BB list within a
function?

Nick

Chris Lattner wrote:
> On Sun, 4 Jun 2006, Vladimir Prus wrote:
>> everytime one has to add instruction at the beginning of a basic block,
>> one has to skip past PHI nodes that are already there. How about adding
a
>> new method to BasicBlock, to get that first non-PHI instruction? So,
>> adding an instruction will be as simple as:
>>
>>      new SomeInstruction(............., BB->getFirstNonPHI())
> 
> Sure, sounds good.  A couple requests:
> 
> 1. Please add a const version that returns a const Instruction* also.
I was considering it, but then decided that given that you can't pass
'const
Instruction*' as 'insert before' parameter of any other instruction,
there's no point in adding it. 

Maybe, the method should be

  Instruction* getFirstNonPHI() const

? Given that in C++ constness is generally not deep -- i.e. don't apply to
contained objects.

- Volodya