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

[Chris Lattner]

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