[llvm-commits] Fold() function on TargetFolder class - removal of redundant constantexpr

[Jin Gu Kang]

I am sorry Nick.

This patch is related to previous e-mails with Duncan.

On previous e-mail with Duncan, 

Duncan said:
> ...
> If you want to disable all constant folding in llvm-gcc then
> I suggest you change all instances of TargetFolder to NoFolder in
> llvm-internal.h and llvm-backend.cpp.  This line
>    TheFolder = new TargetFolder(TheTarget->getTargetData());
> probably needs to become something like this:
>    TheFolder = new NoFolder(getGlobalContext());
> However as I mentioned it probably will require some work on IRBuilder and
> NoFolder to get it working properly.  If you succeed please send in a patch.

As you can see on above statements, I made this patch because of disabling 
all constant folding in llvm-gcc.

Thanks,
Jin-Gu Kang
________________________________________
From: Nick Lewycky [nicholas at mxc.ca]
Sent: Friday, February 25, 2011 3:31 PM
To: Jin Gu Kang
Cc: Duncan Sands; llvm-commits at cs.uiuc.edu
Subject: Re: [llvm-commits] Fold() function on TargetFolder class - removal of redundant constantexpr

Jin Gu Kang wrote:
> Hi Nick
>
> It was difficult for me to decide implementation style.
>
> Is it acceptable to make new named Constant constructors
> which make unfoled Constants?
> For example, like getNoFoldAdd().

Why do you ever want unfolded constants? It isn't an accident that
they're impossible to create in LLVM. You would need an *excellent*
reason to add them to LLVM regardless of how they're implemented. (Put
another way, do you realize what else needs to change if we add these?
The optimizers will *never* fold them because they assume that such
constants are impossible. Simple things like "1+1 == 2" will fail. You
could argue that the rest of LLVM doesn't need to create them, but if
the rest of LLVM doesn't support them, why should they exist at all?)

If you're learning LLVM and are having trouble seeing generated IR
because it gets constant folded away, then you should use the IRBuilder
with NoFolder.

Nick

> Thanks,
> Jin-Gu Kang
> ________________________________________
> From: Nick Lewycky [nicholas at mxc.ca]
> Sent: Friday, February 25, 2011 3:02 PM
> To: Jin Gu Kang
> Cc: Duncan Sands; llvm-commits at cs.uiuc.edu
> Subject: Re: [llvm-commits] Fold() function on TargetFolder class - removal of redundant constantexpr
>
> Jin Gu Kang wrote:
>> Hi Duncan
>>
>> I send patches about NoFolder.
>
> The idea behind llvm::NoFolder is that you create instructions instead
> of constants; that's what prevents the folding. When IRBuilder<true,
> NoFolder>.CreateAdd(i32 1, i32 1) is called it should create an add
> instruction (instead of a constant) with i32 1 and i32 1 as arguments,
> and no folding will occur.
>
> Adding a DoFold argument to every Constant constructor is not an
> acceptable solution to commit to LLVM.
>
> Nick
>
>> NoFolder's functions return pointer of Constant because llvm-gcc's codes which
>> call TheFolder's functions wait for pointer of Constant. and some of get() functions
>> in ConstantExpr have a additional argument "DoFold" which decide to enable or
>> disable fold. This implementation may be inefficient because there are additional
>> arguments and 'if' statements. Please comment about this.
>>
>> In this patch, bitcast is folded unconditionally due to a little of problems.
>> For example, EmitSWITCH_EXPR() on llvm-gcc-4.2 changes case's low
>> and high values to ConstantInt which is folded to integer constant.and
>> The part of codes does not accept unfolded bitcast ConstantExpr.
>> (Because ConvertINTEGER_CST() uses Type::getInt64Ty(Context) to make
>> ConstantInt, bitcast is generated on types with other bitwidths)
>>
>> I completed compilation of llvm-gcc-4.2 on x86-32bit and ARM using this patch
>> and tested test-suite. There are not compilation and execution errors on x86-32bit.
>>
>> Patch files are as following:
>> 1. llvm patch
>> 2. llvm-gcc test patch - insertion of define "LLVM_TEST_NOFOLDER"
>>
>> Please review these patches.
>>
>> Thanks,
>> Jin-Gu Kang
>> ________________________________________
>> From: Duncan Sands [baldrick at free.fr]
>> Sent: Wednesday, February 23, 2011 5:57 PM
>> To: Jin Gu Kang
>> Cc: Chris Lattner; llvm-commits at cs.uiuc.edu
>> Subject: Re: [llvm-commits] Fold() function on TargetFolder class - removal of redundant constantexpr
>>
>> Hi Jin Gu Kang,
>>
>>> while processing "kang.temp.e = 1" in TreeToLLVM::EmitLV_COMPONENT_REF() function on llvm-gcc-4.2,
>>> StructAddrLV.Ptr which is returned from EmitLV() is bitcast ConstantExpr because type of kang's initializer is different
>>> from type of struct test and this StructAddrLV.Ptr is used as an argument by CreateStructGEP().
>>> Later, this bitcast ConstantExpr is disappeared by SymbolicallyEvaluateGEP() like above ll code for getelementptr.
>>>
>>> so I suggested to keep this bitcast ConstantExpr in order to represent meaning of conversion.
>>
>> it is true that disabling all constant folding may be helpful for beginners who
>> want to read the bitcode and understand how it relates to the original C.  This
>> is not so easy to achieve however because a minimal amount of constant folding
>> always occurs.  For example you can change llvm-gcc from using TargetFolder to
>> ConstantFolder.  This will result in less constant folding because it will no
>> longer use information about the target when constant folding.  However the fold
>> you mentioned (bitcast turned into deeper GEP) will occur anyway because doing
>> it does not require knowing the target.  There is also a NoFolder class which
>> does no folding whatsoever, by creating instructions rather than constants, but
>> I hear that it doesn't work anymore (it used to, but perhaps bitrotted because
>> no one uses it).  If you want to disable all constant folding in llvm-gcc then
>> I suggest you change all instances of TargetFolder to NoFolder in
>> llvm-internal.h and llvm-backend.cpp.  This line
>>      TheFolder = new TargetFolder(TheTarget->getTargetData());
>> probably needs to become something like this:
>>      TheFolder = new NoFolder(getGlobalContext());
>> However as I mentioned it probably will require some work on IRBuilder and
>> NoFolder to get it working properly.  If you succeed please send in a patch.
>>
>> Ciao, Duncan.
>>
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits