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

[Jin Gu Kang]

Hi Duncan

> First, I thought index of getelementptr is confused.
> %1 = load i32* bitcast (i8* getelementptr inbounds (%0* @kang, i32 0, i32 2, i32 0) to i32*)
> On above statement, there is not bitcast which decribes kang's sub struct type(%1) is mapped to struct.test.

On above statement, I meant it may be hard to understand that bitcast from i8* to i32* is 
conversion from first field of %1(kang's sub struct type) to first field struct test.

> int main(void) {
> kang.temp.e = 1;
> return 0;
>}

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.

Thanks,
Jin-Gu Kang
________________________________________
From: Duncan Sands [baldrick at free.fr]
Sent: Wednesday, February 23, 2011 4:59 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,

> %struct.kist = type { i8, i32, %struct.test, i16, i32 }
> %struct.test = type { i32, i32, double }
...
>    %1 = load i32* bitcast (i8* getelementptr inbounds (%0* @kang, i32 0, i32 2, i32 0) to i32*), align 8

the getelementptr access the first field (i32) of %struct.test.  You
can read it as follows: ignore the first index (i32 0); the second index
(i32 2) indexes the third field in %struct.kist, i.e. the %struct.test field;
the third index (i32 0) indexes the first field in %struct.test, which is an
i32 (it contains the three C fields c, d and e).

> First, I thought index of getelementptr is confused.

See above explanation.

> %1 = load i32* bitcast (i8* getelementptr inbounds (%0* @kang, i32 0, i32 2, i32 0) to i32*)
> On above statement, there is not bitcast which decribes kang's sub struct type(%1) is mapped to struct.test.

That's because it is not getting a pointer to temp, it is getting a pointer to
the first field of temp, see above.  It may be that the front-end originally
got a pointer to temp and bitcast it to a pointer to the first field of the
struct, and the constant folder turned this into a GEP that goes one deeper.

> The reason why bitcast disappears that SymbolicallyEvaluateGEP() function under CreateStructGEP()
> function changes bitcast ConstantExpr to bitcast ConstantExpr's operand(0) using stripPointerCasts()
> and makes new GEP using the operand(0).
>
> I would like to know what do you think about keeping bitcast ConstantExpr about above code?

What for?  Also, if you want less constant folding, use ConstantFolder rather
than TargetFolder.

> Second, I thought redundant information is generated while processing CreateStructGEP().
>    Constant *CreateInBoundsGetElementPtr(Constant *C, Value* const *IdxList,
>                                          unsigned NumIdx) const {
>      return Fold(ConstantExpr::getInBoundsGetElementPtr(C, IdxList, NumIdx));
>    }
> As I said on previous e-mail, I thought if fold() function's argument is different with fold() function's return value,
> fold() function's argument which was newly created  is redundant. This redundant ConstantExpr
> increases Use count of its operand and Increased Use Count may have effect on functions which use Use inforamation
> like replaceAllUsesWith(). so I would like to remove this redundant ConstantExprs. :)

I'm sure more time has been spent discussing this point with you than would ever
be saved during the history of the universe by doing this optimization.  I don't
intend to reply any more on this point.

Ciao, Duncan.