SROA assertion: creating bitcast between ptr types with different addr spaces

[liu12295]

Summary

In SROA.cpp's canConvertValue() function, it will return "true" if both NewTy and OldTy are ptr type. Probably we need to check whether those two pointers are pointing to the same addr space; otherwise this will trigger an assertion later when SROA tries to create a bitcast between pointers with different addr spaces.

[liu12295]

Add FileCheck, so that we can see the problem even with release build. The original test will trigger assertion only on debug build.

[sanjoy]

The code looks fine, have a few minor nits on the test case.

test/Transforms/SROA/pr27557.ll

1 ↗

(On Diff #55608)

@chandlerc 's suggestion of putting this in address-spaces.ll still stands. To make the PR number obvious, I'd suggest renaming @testKernel to @pr27557 instead.

3 ↗

(On Diff #55608)

You can probably remove the ModuleID bit.

11 ↗

(On Diff #55608)

These show up inside the the optimized testKernel, right? If so, I'd suggest:

; CHECK-LABEL: @testKernel(
< your CHECK: lines >

[liu12295]

Add unit test into existing address-spaces.ll

[liu12295]

r268409

Function canConvertValue needs to check whether both NewTy and OldTy pointers are
pointing to the same addr space. This can prevent SROA from creating a bitcast
between pointers with different addr spaces.

Differential Revision: http://reviews.llvm.org/D19697

lib/Transforms/Scalar/SROA.cpp

static bool canConvertValue(const DataLayout &DL, Type *OldTy, Type *NewTy) {
   if (!NewTy->isSingleValueType() || !OldTy->isSingleValueType())
     return false;
 
   // We can convert pointers to integers and vice-versa. Same for vectors
   // of pointers and integers.
   OldTy = OldTy->getScalarType();
   NewTy = NewTy->getScalarType();
   if (NewTy->isPointerTy() || OldTy->isPointerTy()) {
-    if (NewTy->isPointerTy() && OldTy->isPointerTy())
-      return true;
+    if (NewTy->isPointerTy() && OldTy->isPointerTy()) {
+      return cast<PointerType>(NewTy)->getPointerAddressSpace() ==
+        cast<PointerType>(OldTy)->getPointerAddressSpace();
+    }
     if (NewTy->isIntegerTy() || OldTy->isIntegerTy())
       return true;
     return false;
   }
 
   return true;
 }
 

test/Transforms/SROA/address-spaces.ll

for.end:
   %0 = bitcast %struct.struct_test_27.0.13* %in to [5 x i64]*
   store [5 x i64] %in.coerce, [5 x i64]* %0, align 8
   %scevgep9 = getelementptr %struct.struct_test_27.0.13, %struct.struct_test_27.0.13* %in, i32 0, i32 4, i32 0
   %scevgep910 = bitcast i32* %scevgep9 to i8*
   call void @llvm.memcpy.p1i8.p0i8.i32(i8 addrspace(1)* undef, i8* %scevgep910, i32 16, i32 4, i1 false)
   ret void
 }
  
+%union.anon = type { i32* }
+
+@g = common global i32 0, align 4
+@l = common addrspace(3) global i32 0, align 4
+
+; Make sure an illegal bitcast isn't introduced
+define void @pr27557() {
+; CHECK-LABEL: @pr27557(
+; CHECK: %[[CAST:.*]] = bitcast i32** {{.*}} to i32 addrspace(3)**
+; CHECK: store i32 addrspace(3)* @l, i32 addrspace(3)** %[[CAST]]
+  %1 = alloca %union.anon, align 8
+  %2 = bitcast %union.anon* %1 to i32**
+  store i32* @g, i32** %2, align 8
+  %3 = bitcast %union.anon* %1 to i32 addrspace(3)**
+  store i32 addrspace(3)* @l, i32 addrspace(3)** %3, align 8
+  ret void
+}