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.

[sanjoy]

lib/Transforms/Scalar/SROA.cpp

1638–1641

This line is too long -- can you wrap it to 80 chars?

http://llvm.org/docs/CodingStandards.html#source-code-width

test/Transforms/SROA/address-spaces-1.ll

1 ↗

(On Diff #55454)

Given that you've filed a PR, it is better to name the test as prXXXX.ll. Also don't use -O3 here -- you should just invoke the pass you're unit testing (SROA in this case).

An easy way to do this is to use -print-before-all to get the IR right before SROA crashed.

13 ↗

(On Diff #55454)

Can you shrink this test case to be more to the point?

[liu12295]

This revision addresses the raised concerns.

[chandlerc]

Note that this patch didn't actually go to the mailing list when you created it, so now the mailing list is missing context on what this patch is even doing. The first email is from Sanjoy.

test/Transforms/SROA/address-spaces-1.ll

1 ↗

(On Diff #55454)

Please don't name the file 'prXXXX.ll' or add a new file at all.

Naming test files after 'pr....' encourages to have more smaller test files when each file adds substantial overhead.

Instead we should be trying to find an existing overarching test file and add the tests to it. Perhaps the 'address-spaces.ll'?

[liu12295]

Will start a new one.

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/pr27557.ll

+; RUN: opt -O1 < %s -S
+
+; ModuleID = '<origin>'
+target datalayout = "e-m:w-i64:64-f80:128-n8:16:32:64-S128"
+
+%union.anon = type { i32 addrspace(1)* }
+
+@g = common addrspace(1) global i32 0, align 4
+@l = common addrspace(3) global i32 0, align 4
+
+; Function Attrs: nounwind uwtable
+define void @testKernel() #0 {
+  %1 = alloca %union.anon, align 8
+  %2 = bitcast %union.anon* %1 to i32 addrspace(1)**
+  store i32 addrspace(1)* @g, i32 addrspace(1)** %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
+}