[llvm-commits] [Review request] MaybeFolder (was: Fold() function on TargetFolder class - removal of redundant constantexpr)

[Frits van Bommel]

On Tue, Mar 1, 2011 at 3:40 PM, Duncan Sands <baldrick at free.fr> wrote:
>> On Tue, Mar 1, 2011 at 11:29 AM, Duncan Sands<baldrick at free.fr>  wrote:
>>> it would be nice if this (folding vs no folding) could be controlled with
>>> a
>>> runtime flag.  If it was I would probably add it permanently to
>>> dragonegg.
>>> Unfortunately the way IRBuilder is declared to take the folder as a
>>> template
>>> parameter makes this hard.  Any ideas for how to achieve this?
>>
[snip my suggestion to add a flag to IRBuilder]
>
> thanks for the suggestion.  I guess another way of doing this is to create a
> new
> folder class that dispatches to a target folder or a no folder depending on
> the
> value of some flag.

Something like the (second) attached patch?

The first one just removes the LLVMContext& argument from *Folder
constructors, as it doesn't seem to be used anywhere. I noticed this
while working on the second one.

The second one adds a MaybeFolder and a little support in IRBuilder to
make it work.
Two things I wasn't sure about:
a) Should this '#include "llvm/Support/ConstantFolder.h"' just so it
can be used as a default template argument?
It's probably fine though; this class is mainly for consumption of
IRBuilder which already does the exact same thing.

b) Should the methods that in other folders just dispatch to a sibling
method delegate to Folder/NoFolder.<same name>() or just to their
sibling method in MaybeFolder like a regular ConstantFolder does?
I chose the first approach so that while for example
ConstantFolder::CreateBitCast(...) calls
ConstantFolder::CreateCast(Instruction::BitCast, ...),
MaybeFolder::CreateBitCast() calls either Folder::CreateBitCast() or
NoFolder::CreateBitCast().
I made this choice mainly to ensure that NoFolder calls are compiled
by the buildbots if this is used in e.g. dragonegg. I'm hoping this
will then make them yell at the next person who forgets to update the
NoFolder when working on ConstantFolder and TargetFolder :).


This of course adds an overhead of a flag check + a dyn_cast<> for
every IRBuilder call that tries to constant fold using this new class;
I haven't checked whether this is significant (but I doubt it).
-------------- next part --------------
A non-text attachment was scrubbed...
Name: folders-remove-context-arg.patch
Type: text/x-patch
Size: 2246 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20110306/eb11cdfc/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: maybe-folder.patch
Type: text/x-patch
Size: 15289 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20110306/eb11cdfc/attachment-0001.bin>