[llvm-commits] [llvm] r61980 - in /llvm/trunk: lib/Transforms/Scalar/InstructionCombining.cpp test/Transforms/InstCombine/bitcast-gep.ll

[Duncan Sands]

Hi Chris,

> +  // Start with the index over the outer type.  Note that the type size
> +  // might be zero (even if the offset isn't zero) if the indexed type
> +  // is something like [0 x {int, int}]

if the type size is zero, I don't see how you can do anything unless
Offset is zero.  So how about handling the zero size case entirely
here and bailing out?

> +    // Handle silly modulus not returning values values [0..TySize).

values values -> values

Also, you would have to do something here anyway to correct FirstIdx,
but I see you don't complain about silly division :)

> +    assert((uint64_t)Offset < (uint64_t)TySize && "Out of range offset");

How about checking that FirstIdx * TySize + Offset == original offset?

> +      unsigned Elt = SL->getElementContainingOffset(Offset);

Does this work correct if the offset is pointing into alignment padding?

> +      if (uint64_t EltSize = TD->getABITypeSize(STy->getElementType())) {
> +        NewIndices.push_back(ConstantInt::get(IntPtrTy,Offset/EltSize));
> +        Offset %= EltSize;
> +      } else {
> +        NewIndices.push_back(ConstantInt::get(IntPtrTy, 0));

If EltSize is zero then aren't you dead unless Offset = 0?

> +          // If we were able to index down into an element, create the GEP
> +          // and bitcast the result.  This eliminates one bitcast, potentially
> +          // two.

Doesn't it turn GEP(bitcast) into bitcast(GEP)?  If so, it doesn't remove a
bitcast.

Ciao,

Duncan.