[ValueTracking] Fix crash in GetPointerBaseWithConstantOffset()

[• tstellarAMD]

Summary

While walking defs of pointer operands we were assuming that the pointer
size would remain constant. This is not true, because addresspacecast
instructions may cast the pointer to an address space with a different
pointer width.

[sanjoy]

lgtm

Btw, this looks like a good reason to change accumulateConstantOffset to return an Optional<APInt> of the right bitwidth.

lib/Analysis/ValueTracking.cpp

Value *llvm::GetPointerBaseWithConstantOffset(Value *Ptr, int64_t &Offset,
   // that case, we stop after accumulating the cycle once (not that it
   // matters).
   SmallPtrSet<Value *, 16> Visited;
   while (Visited.insert(Ptr).second) {
     if (Ptr->getType()->isVectorTy())
       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 ||
                Operator::getOpcode(Ptr) == Instruction::AddrSpaceCast) {
       Ptr = cast<Operator>(Ptr)->getOperand(0);
     } else if (GlobalAlias *GA = dyn_cast<GlobalAlias>(Ptr)) {
       if (GA->isInterposable())
         break;

test/Analysis/ValueTracking/get-pointer-base-with-const-off.ll

+; 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
+}