[llvm-commits] [llvm] r62045 - /llvm/trunk/lib/Transforms/Scalar/InstructionCombining.cpp

[Nicolas Geoffray]

Hi Chris,

Quick question folllowing this patch. Getting the size of a type is 
platform dependent, and since InstCombine makes lots of uses of 
getTypeSize, InstCombine is therefore platform dependent. Is this OK? I 
know this patch did not introduce the platform-dependency, but I was 
just curious if that's OK, and if LLVM considers transformation passes 
strongly platform dependent.

Thanks!
Nicolas

Chris Lattner wrote:
> Author: lattner
> Date: Sun Jan 11 14:15:20 2009
> New Revision: 62045
>
> URL: http://llvm.org/viewvc/llvm-project?rev=62045&view=rev
> Log:
> Make a couple of cleanups to the instcombine bitcast/gep 
> canonicalization transform based on duncan's comments:
>
> 1) improve the comment about %.
> 2) within our index loop make sure the offset stays 
>    within the *type size*, instead of within the *abi size*.
>    This allows us to reason explicitly about landing in tail
>    padding and means that issues like non-zero offsets into
>    [0 x foo] types don't occur anymore.
>
>
> Modified:
>     llvm/trunk/lib/Transforms/Scalar/InstructionCombining.cpp
>
> Modified: llvm/trunk/lib/Transforms/Scalar/InstructionCombining.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/InstructionCombining.cpp?rev=62045&r1=62044&r2=62045&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Scalar/InstructionCombining.cpp (original)
> +++ llvm/trunk/lib/Transforms/Scalar/InstructionCombining.cpp Sun Jan 11 14:15:20 2009
> @@ -7712,7 +7712,7 @@
>      FirstIdx = Offset/TySize;
>      Offset %= TySize;
>      
> -    // Handle silly modulus not returning values [0..TySize).
> +    // Handle hosts where % returns negative instead of values [0..TySize).
>      if (Offset < 0) {
>        --FirstIdx;
>        Offset += TySize;
> @@ -7725,12 +7725,15 @@
>      
>    // Index into the types.  If we fail, set OrigBase to null.
>    while (Offset) {
> +    // Indexing into tail padding between struct/array elements.
> +    if (uint64_t(Offset*8) >= TD->getTypeSizeInBits(Ty))
> +      return false;
> +    
>      if (const StructType *STy = dyn_cast<StructType>(Ty)) {
>        const StructLayout *SL = TD->getStructLayout(STy);
> -      if (Offset >= (int64_t)SL->getSizeInBytes()) {
> -        // We can't index into this, bail out.
> -        return false;
> -      }
> +      assert(Offset < (int64_t)SL->getSizeInBytes() &&
> +             "Offset must stay within the indexed type");
> +      
>        unsigned Elt = SL->getElementContainingOffset(Offset);
>        NewIndices.push_back(ConstantInt::get(Type::Int32Ty, Elt));
>        
> @@ -7738,15 +7741,13 @@
>        Ty = STy->getElementType(Elt);
>      } else if (isa<ArrayType>(Ty) || isa<VectorType>(Ty)) {
>        const SequentialType *STy = cast<SequentialType>(Ty);
> -      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));
> -      }
> +      uint64_t EltSize = TD->getABITypeSize(STy->getElementType());
> +      assert(EltSize && "Cannot index into a zero-sized array");
> +      NewIndices.push_back(ConstantInt::get(IntPtrTy,Offset/EltSize));
> +      Offset %= EltSize;
>        Ty = STy->getElementType();
>      } else {
> -      // Otherwise, we can't index into this, bail out.
> +      // Otherwise, we can't index into the middle of this atomic type, bail.
>        return false;
>      }
>    }
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>