constant folding doesn't simplify constantexpr gep to constantint

[lattner]

Bugzilla Link	8883
Resolution	FIXED
Resolved on	Jan 06, 2011 00:20
Version	1.0
OS	All
CC	@sunfishcode

Extended Description

This code:

void fill(Iterator first, Iterator last, T value) {
while (first != last) *first++ = value;
}
char X[1000];
void foo() {
fill(X, X+1000, 0);
}

Compiles with: clang t.cpp -S -o - -emit-llvm -O3

into:

...

while.body.i: ; preds = %while.body.i, %entry
%indvar.i = phi i64 [ 0, %entry ], [ %indvar.next.i, %while.body.i ]
%first.addr.05.i = getelementptr [1000 x i8]* @​X, i64 0, i64 %indvar.i
store i8 0, i8* %first.addr.05.i, align 1, !tbaa !​0
%indvar.next.i = add i64 %indvar.i, 1
%exitcond = icmp eq i64 %indvar.next.i, ptrtoint (i8* getelementptr ([1000 x i8]* @​X, i64 1, i64 sub (i64 0, i64 ptrtoint ([1000 x i8]* @​X to i64))) to i64)
br i1 %exitcond, label %Z4fillIPciEvT_S1_T0.exit, label %while.body.i

Notice the poor icmp, which should be comparing the indvar against 1000. SCEV should simplify this away. This can also be seen in simple_types_constant_folding.llvm.bc in Nehalem:SingleSource/Benchmarks/Adobe-C++ where SCEV is expanding an expression for the loop-idiom pass, resulting in this crazy memset:

call void @​llvm.memset.p0i8.i64(i8* getelementptr inbounds ([8000 x i8]* @​data8, i64 0, i64 0), i8 %11, i64 ptrtoint (i8* getelementptr ([8000 x i8]* @​data8, i64 1, i64 sub (i64 0, i64 ptrtoint ([8000 x i8]* @​data8 to i64))) to i64), i32 16, i1 false)

Codegen is able to simplify the constant to "8000", but the expression simplification logic in SCEV should do this. This may be just a matter of using lib/Analysis/ConstantFolding APIs with target data in scev.

[sunfishcode]

The crazy constant is not coming from indvars. This kind of thing should be simplified by instcombine.

[lattner]

This is coming from loop idiom, which is using SCEV. SCEV is the one that should fold this (e.g. using IRBuilder with a TargetData-enabled folder).

[sunfishcode]

reduced testcase
The attached testcase does not have foldable constant exprs. Run it through just opt -inline and the big foldable constant expr appears. Taking opt's given pass sequence literally is a common pitfall.

So maybe the inliner that needs to be doing TargetData-enabled folding. But an argument can be made that instcombine ought to be able to simplify things like this either way.

[lattner]

Ah, very interesting. Investigating.

[lattner]

Inliner is doing the right thing, constantfolding.cpp just didn't know how to handle this case. Simple enough, implemented in r122950. Thanks for tracking down what the root issue is Dan!