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

[Duncan Sands]

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.