[llvm-commits] [llvm] r61980 - in /llvm/trunk: lib/Transforms/Scalar/InstructionCombining.cpp test/Transforms/InstCombine/bitcast-gep.ll
Chris Lattner
sabre at nondot.org
Fri Jan 9 10:31:21 PST 2009
On Jan 9, 2009, at 1:19 AM, Duncan Sands wrote:
> 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?
Consider trying to get to offset 4 in this. It will form a gep 0, 0,
1. Just because the array has size zero it doesn't mean its elements
have size 0.
>
>> + // Handle silly modulus not returning values values [0..TySize).
>
> values values -> values
Fixed.
>
> Also, you would have to do something here anyway to correct FirstIdx,
> but I see you don't complain about silly division :)
What do you mean? Note that this code already existed in instcombine,
I just moved/refactored it.
>
>> + assert((uint64_t)Offset < (uint64_t)TySize && "Out of range
>> offset");
>
> How about checking that FirstIdx * TySize + Offset == original offset?
why?
>> + unsigned Elt = SL->getElementContainingOffset(Offset);
>
> Does this work correct if the offset is pointing into alignment
> padding?
Yep.
>> + 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?
No, see above.
>
>> + // 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.
Right, it doesn't (necessarily) remove the bitcast, this is a
canonicalization. This happens a lot with union cases.
-Chris
More information about the llvm-commits
mailing list