llvm dev - Apr 2008 - [LLVMdev] Virtual methods (was: LLVMBuilder vs LLVMFoldingBuilder)

Hello llvm dev peeps

I would like to use an LLVMBuilder pointer as a base pointer to 
reference either an LLVMBuilder or an LLVMFoldingBuilder. As the methods 
in the Folding builder have the same names as the base class, I thought 
about submitting a patch whereby the base class methods would become 
virtual. However, the base class methods return specific types while the 
Folding builder, for good reason, return more general Value* types.

Would it be reasonable for me to submit a patch whereby the LLVMBuilder 
methods also return the general Value* type and the LLVMFoldingBuilder 
methods become virtual overrides of the base class methods?

Users (such as myself) could then decide at runtime which type of 
builder they wish to use.

Another option that was discussed in #llvm is to nuke LLVMBuilder and 
rename LLVMFoldingBuilder to LLVMBuilder. If this was the case, I'd 
argue for a flag in the Builder that could retain the old non-folding 
functionality for debugging purposes.

Your thoughts please?

Duncan Sands wrote:
>> Another option that was discussed in #llvm is to nuke LLVMBuilder and 
>> rename LLVMFoldingBuilder to LLVMBuilder. If this was the case, I'd
>> argue for a flag in the Builder that could retain the old non-folding 
>> functionality for debugging purposes.
>>     
>
> this plan sounds good to me.  However it's not clear to me how useful a
> debug flag would really be.
>
>   
I know that using the LLVMBuilder was useful to me in the early days of 
my work with LLVM to see exactly how the IR matched with what I expected 
it to given the input. It's only now that I'm working on global 
variables that require ConstantExpr types for initialisers that I 
started to use the LLVMFoldingBuilder.

I'll work on the patch and submit it here when it's ready.

Thanks

Am Donnerstag, den 03.04.2008, 19:29 -0700 schrieb Chris
Lattner:
> On Apr 2, 2008, at 9:54 AM, Dominic Hamon wrote:
> 
> > Would it be reasonable for me to submit a patch whereby [...] the
> > LLVMFoldingBuilder methods become virtual overrides of the base
> > class methods?
> 
> No, please don't do this.  The idea of llvmbuilder is that it is a  
> "free" wrapper around the other existing API calls.  Making the  
> methods virtual would make them much more expensive.
Wouldn't the class of the objects be known at compile time in most
cases? This is essentially just a case of precomputing constants, so I
think this should be possible.

If yes, the compiler can predetermine the type, hence the virtual method
table that will be used, and can replace the virtual call with a static
one.

Does such an approach make sense with LLVM?
(I'd want to do such things for my personal language project, so the
answer would be affecting compilation strategy.)

Regards,
Jo

Chris Lattner wrote:
> On Apr 2, 2008, at 9:54 AM, Dominic Hamon wrote:
>   
>> Would it be reasonable for me to submit a patch whereby the  
>> LLVMBuilder
>> methods also return the general Value* type and the LLVMFoldingBuilder
>> methods become virtual overrides of the base class methods?
>>     
>
> No, please don't do this.  The idea of llvmbuilder is that it is a  
> "free" wrapper around the other existing API calls.  Making the  
> methods virtual would make them much more expensive.
>
>   
Is this why the other API calls are all inline in the class? To avoid 
the overhead of the function call?
>> Users (such as myself) could then decide at runtime which type of
>> builder they wish to use.
>>     
>
> Why do you want this?
>   
At the time that I was considering this, I saw that the Folding Builder 
was a derived class of Builder and assumed, apparently wrongly, that the 
intention was to create further derived versions of the Builder with 
other potential optimisations.

If the Folding Builder is meant as a general replacement for the 
Builder, then the second approach does make much more sense.
>> Another option that was discussed in #llvm is to nuke LLVMBuilder and
>> rename LLVMFoldingBuilder to LLVMBuilder.
>>     
>
> I'd strongly prefer this.  The non-folding builder is not very useful  
> for anything, and anyone who wants unfolded instructions can call the  
> instruction ctors directly.
>
>   
>> If this was the case, I'd
>> argue for a flag in the Builder that could retain the old non-folding
>> functionality for debugging purposes.
>>     
>
> Why?
>
>   
I used this non-folding functionality to create more explicit IR that I 
could check against my input to my parser. It made debugging far easier 
in the early stages of adding new parsing functionality.

Dom

Duncan Sands wrote:
>> Another option that was discussed in #llvm is to nuke LLVMBuilder and 
>> rename LLVMFoldingBuilder to LLVMBuilder. If this was the case, I'd
>> argue for a flag in the Builder that could retain the old non-folding 
>> functionality for debugging purposes.
>>     
>
> this plan sounds good to me.  However it's not clear to me how useful a
> debug flag would really be.
>
>   
After further discussion in #llvm, and lots of compiling and 
re-compiling, please find attached the first attempt at the patch with 
this change. There are two patches attached, one for llvm and one for 
llvm-gcc42.

Please note, there's quite a substantial API change in this patch: 
LLVMBuilder has been renamed to IRBuilder and has absorbed the 
constant-folding functionality from LLVMFoldingBuilder.

I've updated the tutorial html to refer to the correct includes and 
classes and tweaked Chapter 4 in particular to explain the constant 
folding optimizations rather than to explain how to use the 
LLVMFoldingBuilder to improve the code generation and these changes are 
included in the patch.

This is my first patch so please let me know if there are any problems 
or anything I need to do differently.

Thanks


-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: llvm-gcc42.IRBuilder.patch
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20080410/8c36b1a1/attachment.ksh>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: llvm.IRBuilder.patch
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20080410/8c36b1a1/attachment-0001.ksh>