[PATCH] BasicBlock::getFirstNonPHI

Project Infrastructure LLVM Dev List Archives
1

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

BasicBlock.diff (1.68 KB)

2

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

3

Chris Lattner 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

4

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?

It should already assert when it attempts to dereference the end iterator.

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?

As you say, this can only happen mid-transformation...

-Chris

5

Chris Lattner 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

6

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.

Ok, that makes sense.

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.

True, but that would break "logical constness" which is more useful. :slight_smile: I'd prefer either two versions or just the single non-const version.

Thanks,

-Chris

7

Chris Lattner wrote: