[PATCH] Fix always creating GEP with i32 indices

[Eli Friedman]

LGTM.

-Eli

On Tue, Aug 13, 2013 at 12:14 PM, Matt Arsenault
<Matthew.Arsenault at amd.com>wrote:

>   Catch a few more cases of this. Some of these were inside the
> visitGetElementPtrInst itself, so that is more useful, since it should
> prevent re-visiting the newly created GEP.
>
> Hi eli.friedman,
>
> http://llvm-reviews.chandlerc.com/D1372
>
> CHANGE SINCE LAST DIFF
>   http://llvm-reviews.chandlerc.com/D1372?vs=3400&id=3442#toc
>
> Files:
>   lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
>   lib/Transforms/InstCombine/InstructionCombining.cpp
>   test/Transforms/InstCombine/alloca.ll
>
> Index: lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
> ===================================================================
> --- lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
> +++ lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
> @@ -180,7 +180,10 @@
>        // Now that I is pointing to the first non-allocation-inst in the
> block,
>        // insert our getelementptr instruction...
>        //
> -      Value *NullIdx =
> Constant::getNullValue(Type::getInt32Ty(AI.getContext()));
> +      Type *IdxTy = TD
> +                  ? TD->getIntPtrType(AI.getContext())
> +                  : Type::getInt64Ty(AI.getContext());
> +      Value *NullIdx = Constant::getNullValue(IdxTy);
>        Value *Idx[2] = { NullIdx, NullIdx };
>        Instruction *GEP =
>          GetElementPtrInst::CreateInBounds(New, Idx, New->getName() +
> ".sub");
> @@ -300,9 +303,11 @@
>        if (ArrayType *ASrcTy = dyn_cast<ArrayType>(SrcPTy))
>          if (Constant *CSrc = dyn_cast<Constant>(CastOp))
>            if (ASrcTy->getNumElements() != 0) {
> -            Value *Idxs[2];
> -            Idxs[0] =
> Constant::getNullValue(Type::getInt32Ty(LI.getContext()));
> -            Idxs[1] = Idxs[0];
> +            Type *IdxTy = TD
> +                        ? TD->getIntPtrType(LI.getContext())
> +                        : Type::getInt64Ty(LI.getContext());
> +            Value *Idx = Constant::getNullValue(IdxTy);
> +            Value *Idxs[2] = { Idx, Idx };
>              CastOp = ConstantExpr::getGetElementPtr(CSrc, Idxs);
>              SrcTy = cast<PointerType>(CastOp->getType());
>              SrcPTy = SrcTy->getElementType();
> Index: lib/Transforms/InstCombine/InstructionCombining.cpp
> ===================================================================
> --- lib/Transforms/InstCombine/InstructionCombining.cpp
> +++ lib/Transforms/InstCombine/InstructionCombining.cpp
> @@ -1235,9 +1235,8 @@
>        if (TD && SrcElTy->isArrayTy() &&
>
>  TD->getTypeAllocSize(cast<ArrayType>(SrcElTy)->getElementType()) ==
>            TD->getTypeAllocSize(ResElTy)) {
> -        Value *Idx[2];
> -        Idx[0] =
> Constant::getNullValue(Type::getInt32Ty(GEP.getContext()));
> -        Idx[1] = GEP.getOperand(1);
> +        TYpe *IdxType = TD->getIntPtrType(GEP.getContext());
> +        Value *Idx[2] = { Constant::getNullValue(IdxType),
> GEP.getOperand(1) };
>          Value *NewGEP = GEP.isInBounds() ?
>            Builder->CreateInBoundsGEP(StrippedPtr, Idx, GEP.getName()) :
>            Builder->CreateGEP(StrippedPtr, Idx, GEP.getName());
> @@ -1305,7 +1304,7 @@
>              // If the multiplication NewIdx * Scale may overflow then the
> new
>              // GEP may not be "inbounds".
>              Value *Off[2];
> -            Off[0] =
> Constant::getNullValue(Type::getInt32Ty(GEP.getContext()));
> +            Off[0] =
> Constant::getNullValue(TD->getIntPtrType(GEP.getContext());
>              Off[1] = NewIdx;
>              Value *NewGEP = GEP.isInBounds() && NSW ?
>                Builder->CreateInBoundsGEP(StrippedPtr, Off, GEP.getName())
> :
> Index: test/Transforms/InstCombine/alloca.ll
> ===================================================================
> --- test/Transforms/InstCombine/alloca.ll
> +++ test/Transforms/InstCombine/alloca.ll
> @@ -1,7 +1,7 @@
> -target datalayout =
> "E-p:64:64:64-a0:0:8-f32:32:32-f64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:32:64-v64:64:64-v128:128:128"
> +; RUN: opt < %s -instcombine -S
> -default-data-layout="E-p:64:64:64-a0:0:8-f32:32:32-f64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:32:64-v64:64:64-v128:128:128"
> | FileCheck %s
> +; RUN: opt < %s -instcombine -S
> -default-data-layout="E-p:32:32:32-a0:0:8-f32:32:32-f64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:32:64-v64:64:64-v128:128:128"
> | FileCheck %s -check-prefix=P32
> +; RUN: opt < %s -instcombine -S | FileCheck %s -check-prefix=NODL
>
> -; RUN: opt < %s -instcombine -S | FileCheck %s
> -; END.
>
>  declare void @use(...)
>
> @@ -110,3 +110,22 @@
>  }
>
>  declare void @llvm.memcpy.p0i8.p0i8.i32(i8* nocapture, i8* nocapture,
> i32, i32, i1) nounwind
> +
> +
> +; Check that the GEP indices use the pointer size, or 64 if unknown
> +define void @test8() {
> +; CHECK-LABEL: @test8(
> +; CHECK: alloca [100 x i32]
> +; CHECK: getelementptr inbounds [100 x i32]* %x1, i64 0, i64 0
> +
> +; P32-LABEL: @test8(
> +; P32: alloca [100 x i32]
> +; P32: getelementptr inbounds [100 x i32]* %x1, i32 0, i32 0
> +
> +; NODL-LABEL: @test8(
> +; NODL: alloca [100 x i32]
> +; NODL: getelementptr inbounds [100 x i32]* %x1, i64 0, i64 0
> +  %x = alloca i32, i32 100
> +  call void (...)* @use(i32* %x)
> +  ret void
> +}
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130813/2bb81fce/attachment.html>