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
Joachim Durchholz
2008-Apr-04 08:29 UTC
[LLVMdev] Virtual methods (was: LLVMBuilder vs LLVMFoldingBuilder)
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>