[PATCH] D24772: [ValueTracking] Fix crash in GetPointerBaseWithConstantOffset()

[Phabricator via llvm-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL283557: [ValueTracking] Fix crash in GetPointerBaseWithConstantOffset() (authored by tstellar).

Changed prior to commit:
  https://reviews.llvm.org/D24772?vs=71955&id=73949#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D24772

Files:
  llvm/trunk/lib/Analysis/ValueTracking.cpp
  llvm/trunk/test/Analysis/ValueTracking/get-pointer-base-with-const-off.ll
  llvm/trunk/test/Transforms/GVN/addrspace-cast.ll


Index: llvm/trunk/test/Analysis/ValueTracking/get-pointer-base-with-const-off.ll
===================================================================
--- llvm/trunk/test/Analysis/ValueTracking/get-pointer-base-with-const-off.ll
+++ llvm/trunk/test/Analysis/ValueTracking/get-pointer-base-with-const-off.ll
@@ -0,0 +1,26 @@
+; RUN: opt -gvn -S < %s | FileCheck %s
+
+; Make sure we don't crash when analyzing an addrspacecast in
+; GetPointerBaseWithConstantOffset()
+
+target datalayout = "e-p:32:32-p4:64:64"
+
+define i32 @addrspacecast-crash() {
+; CHECK-LABEL: @addrspacecast-crash
+; CHECK: %tmp = alloca [25 x i64]
+; CHECK: %tmp1 = getelementptr inbounds [25 x i64], [25 x i64]* %tmp, i32 0, i32 0
+; CHECK: %tmp2 = addrspacecast i64* %tmp1 to <8 x i64> addrspace(4)*
+; CHECK: store <8 x i64> zeroinitializer, <8 x i64> addrspace(4)* %tmp2
+; CHECK-NOT: load
+bb:
+  %tmp = alloca [25 x i64]
+  %tmp1 = getelementptr inbounds [25 x i64], [25 x i64]* %tmp, i32 0, i32 0
+  %tmp2 = addrspacecast i64* %tmp1 to <8 x i64> addrspace(4)*
+  %tmp3 = getelementptr inbounds <8 x i64>, <8 x i64> addrspace(4)* %tmp2, i64 0
+  store <8 x i64> zeroinitializer, <8 x i64> addrspace(4)* %tmp3
+  %tmp4 = getelementptr inbounds [25 x i64], [25 x i64]* %tmp, i32 0, i32 0
+  %tmp5 = addrspacecast i64* %tmp4 to i32 addrspace(4)*
+  %tmp6 = getelementptr inbounds i32, i32 addrspace(4)* %tmp5, i64 10
+  %tmp7 = load i32, i32 addrspace(4)* %tmp6, align 4
+  ret i32 %tmp7
+}
Index: llvm/trunk/test/Transforms/GVN/addrspace-cast.ll
===================================================================
--- llvm/trunk/test/Transforms/GVN/addrspace-cast.ll
+++ llvm/trunk/test/Transforms/GVN/addrspace-cast.ll
@@ -1,21 +0,0 @@
-; RUN: opt < %s -gvn -S | FileCheck %s
-target datalayout = "e-m:e-p:16:16-p1:32:16-i32:16-i64:16-n8:16"
-
-; In cases where two address spaces do not have the same size pointer, the
-; input for the addrspacecast should not be used as a substitute for itself
-; when manipulating the pointer.
-
-; Check that we don't hit the assert in this scenario
-define i8 @test(i32 %V, i32* %P) {
-; CHECK-LABEL: @test(
-; CHECK: load
-  %P1 = getelementptr inbounds i32, i32* %P, i16 16
-
-  store i32 %V, i32* %P1
-
-  %P2 = addrspacecast i32* %P1 to i8 addrspace(1)*
-  %P3 = getelementptr i8, i8 addrspace(1)* %P2, i32 2
-
-  %A = load i8, i8 addrspace(1)* %P3
-  ret i8 %A
-}
Index: llvm/trunk/lib/Analysis/ValueTracking.cpp
===================================================================
--- llvm/trunk/lib/Analysis/ValueTracking.cpp
+++ llvm/trunk/lib/Analysis/ValueTracking.cpp
@@ -2855,21 +2855,22 @@
       break;
 
     if (GEPOperator *GEP = dyn_cast<GEPOperator>(Ptr)) {
-      APInt GEPOffset(BitWidth, 0);
+      // If one of the values we have visited is an addrspacecast, then
+      // the pointer type of this GEP may be different from the type
+      // of the Ptr parameter which was passed to this function.  This
+      // means when we construct GEPOffset, we need to use the size
+      // of GEP's pointer type rather than the size of the original
+      // pointer type.
+      APInt GEPOffset(DL.getPointerTypeSizeInBits(Ptr->getType()), 0);
       if (!GEP->accumulateConstantOffset(DL, GEPOffset))
         break;
 
-      ByteOffset += GEPOffset;
+      ByteOffset += GEPOffset.getSExtValue();
 
       Ptr = GEP->getPointerOperand();
-    } else if (Operator::getOpcode(Ptr) == Instruction::BitCast) {
+    } else if (Operator::getOpcode(Ptr) == Instruction::BitCast ||
+               Operator::getOpcode(Ptr) == Instruction::AddrSpaceCast) {
       Ptr = cast<Operator>(Ptr)->getOperand(0);
-    } else if (AddrSpaceCastInst *ASCI = dyn_cast<AddrSpaceCastInst>(Ptr)) {
-      Value *SourcePtr = ASCI->getPointerOperand();
-      // Don't look through addrspace cast which changes pointer size
-      if (BitWidth != DL.getPointerTypeSizeInBits(SourcePtr->getType()))
-        break;
-      Ptr = SourcePtr;
     } else if (GlobalAlias *GA = dyn_cast<GlobalAlias>(Ptr)) {
       if (GA->isInterposable())
         break;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D24772.73949.patch
Type: text/x-patch
Size: 4062 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161007/9ca36443/attachment.bin>