<div dir="ltr"><div>LGTM.</div><div><br></div><div>-Eli</div><div><br></div>On Tue, Aug 13, 2013 at 12:14 PM, Matt Arsenault <span dir="ltr"><<a href="mailto:Matthew.Arsenault@amd.com" target="_blank">Matthew.Arsenault@amd.com</a>></span> wrote:<br>
<div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> 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.<br>
<br>
Hi eli.friedman,<br>
<br>
<a href="http://llvm-reviews.chandlerc.com/D1372" target="_blank">http://llvm-reviews.chandlerc.com/D1372</a><br>
<br>
CHANGE SINCE LAST DIFF<br>
<a href="http://llvm-reviews.chandlerc.com/D1372?vs=3400&id=3442#toc" target="_blank">http://llvm-reviews.chandlerc.com/D1372?vs=3400&id=3442#toc</a><br>
<br>
Files:<br>
lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp<br>
lib/Transforms/InstCombine/InstructionCombining.cpp<br>
<div class="im"> test/Transforms/InstCombine/alloca.ll<br>
<br>
Index: lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp<br>
===================================================================<br>
--- lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp<br>
+++ lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp<br>
@@ -180,7 +180,10 @@<br>
// Now that I is pointing to the first non-allocation-inst in the block,<br>
// insert our getelementptr instruction...<br>
//<br>
- Value *NullIdx = Constant::getNullValue(Type::getInt32Ty(AI.getContext()));<br>
+ Type *IdxTy = TD<br>
+ ? TD->getIntPtrType(AI.getContext())<br>
+ : Type::getInt64Ty(AI.getContext());<br>
+ Value *NullIdx = Constant::getNullValue(IdxTy);<br>
Value *Idx[2] = { NullIdx, NullIdx };<br>
Instruction *GEP =<br>
GetElementPtrInst::CreateInBounds(New, Idx, New->getName() + ".sub");<br>
</div>@@ -300,9 +303,11 @@<br>
if (ArrayType *ASrcTy = dyn_cast<ArrayType>(SrcPTy))<br>
if (Constant *CSrc = dyn_cast<Constant>(CastOp))<br>
if (ASrcTy->getNumElements() != 0) {<br>
- Value *Idxs[2];<br>
- Idxs[0] = Constant::getNullValue(Type::getInt32Ty(LI.getContext()));<br>
- Idxs[1] = Idxs[0];<br>
<div class="im">+ Type *IdxTy = TD<br>
</div>+ ? TD->getIntPtrType(LI.getContext())<br>
+ : Type::getInt64Ty(LI.getContext());<br>
+ Value *Idx = Constant::getNullValue(IdxTy);<br>
+ Value *Idxs[2] = { Idx, Idx };<br>
CastOp = ConstantExpr::getGetElementPtr(CSrc, Idxs);<br>
SrcTy = cast<PointerType>(CastOp->getType());<br>
SrcPTy = SrcTy->getElementType();<br>
Index: lib/Transforms/InstCombine/InstructionCombining.cpp<br>
===================================================================<br>
--- lib/Transforms/InstCombine/InstructionCombining.cpp<br>
+++ lib/Transforms/InstCombine/InstructionCombining.cpp<br>
@@ -1235,9 +1235,8 @@<br>
if (TD && SrcElTy->isArrayTy() &&<br>
TD->getTypeAllocSize(cast<ArrayType>(SrcElTy)->getElementType()) ==<br>
TD->getTypeAllocSize(ResElTy)) {<br>
- Value *Idx[2];<br>
- Idx[0] = Constant::getNullValue(Type::getInt32Ty(GEP.getContext()));<br>
- Idx[1] = GEP.getOperand(1);<br>
+ TYpe *IdxType = TD->getIntPtrType(GEP.getContext());<br>
+ Value *Idx[2] = { Constant::getNullValue(IdxType), GEP.getOperand(1) };<br>
Value *NewGEP = GEP.isInBounds() ?<br>
Builder->CreateInBoundsGEP(StrippedPtr, Idx, GEP.getName()) :<br>
Builder->CreateGEP(StrippedPtr, Idx, GEP.getName());<br>
@@ -1305,7 +1304,7 @@<br>
// If the multiplication NewIdx * Scale may overflow then the new<br>
// GEP may not be "inbounds".<br>
Value *Off[2];<br>
- Off[0] = Constant::getNullValue(Type::getInt32Ty(GEP.getContext()));<br>
+ Off[0] = Constant::getNullValue(TD->getIntPtrType(GEP.getContext());<br>
Off[1] = NewIdx;<br>
Value *NewGEP = GEP.isInBounds() && NSW ?<br>
Builder->CreateInBoundsGEP(StrippedPtr, Off, GEP.getName()) :<br>
<div class="HOEnZb"><div class="h5">Index: test/Transforms/InstCombine/alloca.ll<br>
===================================================================<br>
--- test/Transforms/InstCombine/alloca.ll<br>
+++ test/Transforms/InstCombine/alloca.ll<br>
@@ -1,7 +1,7 @@<br>
-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"<br>
+; 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<br>
+; 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<br>
+; RUN: opt < %s -instcombine -S | FileCheck %s -check-prefix=NODL<br>
<br>
-; RUN: opt < %s -instcombine -S | FileCheck %s<br>
-; END.<br>
<br>
declare void @use(...)<br>
<br>
@@ -110,3 +110,22 @@<br>
}<br>
<br>
declare void @llvm.memcpy.p0i8.p0i8.i32(i8* nocapture, i8* nocapture, i32, i32, i1) nounwind<br>
+<br>
+<br>
+; Check that the GEP indices use the pointer size, or 64 if unknown<br>
+define void @test8() {<br>
+; CHECK-LABEL: @test8(<br>
+; CHECK: alloca [100 x i32]<br>
+; CHECK: getelementptr inbounds [100 x i32]* %x1, i64 0, i64 0<br>
+<br>
+; P32-LABEL: @test8(<br>
+; P32: alloca [100 x i32]<br>
+; P32: getelementptr inbounds [100 x i32]* %x1, i32 0, i32 0<br>
+<br>
+; NODL-LABEL: @test8(<br>
+; NODL: alloca [100 x i32]<br>
+; NODL: getelementptr inbounds [100 x i32]* %x1, i64 0, i64 0<br>
+ %x = alloca i32, i32 100<br>
+ call void (...)* @use(i32* %x)<br>
+ ret void<br>
+}<br>
</div></div></blockquote></div><br></div></div>