[SROA] Teach SROA to perform no-op pointer conversion.

[hliao]

Summary

• When promoting a pointer from memory to register, SROA skips pointers from different address spaces. However, as ptrtoint and inttoptr are defined as no-op casts if that integer type has the same as the pointer value, generate the pair of ptrtoint/inttoptr (no-op cast) sequence to convert pointers from different address spaces if they have the same size.

[arsenm]

llvm/lib/Transforms/Scalar/SROA.cpp

1706–1713

cast<PointerType>->getPointerAddressSpace is redundant. You can drop the pionter in the getter. Also, in this case I would probably prefer dyn_cast and check the pointer since this is immediately after an isPointerTy check

1784

Isn't there a create-noop or create reinterpret cast somewhere?

This also probably needs more comments

[hliao]

remove redundant code.

[hliao]

llvm/lib/Transforms/Scalar/SROA.cpp

1784

so far, there is no cast creating that pair.

[hliao]

PING

[hliao]

PING

[hliao]

ping for code review

[efriedma]

llvm/lib/Transforms/Scalar/SROA.cpp

1709

Do you need to check isNonIntegralPointerType()?

1786

Probably this comment should be clarified to explain why this isn't an addrspacecast.

[hliao]

Check non-integral pointers.

[hliao]

llvm/lib/Transforms/Scalar/SROA.cpp

1709

good point

[hliao]

fix typo!

[hliao]

Add a new test on non-integral pointers.

[efriedma]

LGTM with one minor comment

llvm/lib/Transforms/Scalar/SROA.cpp

1709

I think "different non-integral" is backwards from the code?

llvm/lib/Transforms/Scalar/SROA.cpp

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 cast<PointerType>(NewTy)->getPointerAddressSpace() ==
-        cast<PointerType>(OldTy)->getPointerAddressSpace();
+      unsigned OldAS = OldTy->getPointerAddressSpace();
+      unsigned NewAS = NewTy->getPointerAddressSpace();
+      // Convert pointers if they are pointers from the same address space or
+      // different non-integral address spaces with the same pointer size.
+      return OldAS == NewAS ||
+             (!DL.isNonIntegralPointerType(OldTy) &&
+              !DL.isNonIntegralPointerType(NewTy) &&
+              DL.getPointerSize(OldAS) == DL.getPointerSize(NewAS));
     }
 
     // We can convert integers to integral pointers, but not to non-integral
     // pointers.
     if (OldTy->isIntegerTy())
       return !DL.isNonIntegralPointerType(NewTy);
 
     // We can convert integral pointers to integers, but non-integral pointers
if (OldTy->isPtrOrPtrVectorTy() && NewTy->isIntOrIntVectorTy()) {
     // Expand i8* to <2 x i32> --> i8* to i64 to <2 x i32>
     if (!OldTy->isVectorTy() && NewTy->isVectorTy())
       return IRB.CreateBitCast(IRB.CreatePtrToInt(V, DL.getIntPtrType(OldTy)),
                                NewTy);
 
     return IRB.CreatePtrToInt(V, NewTy);
   }
 
+  if (OldTy->isPtrOrPtrVectorTy() && NewTy->isPtrOrPtrVectorTy()) {
+    unsigned OldAS = OldTy->getPointerAddressSpace();
+    unsigned NewAS = NewTy->getPointerAddressSpace();
+    // To convert pointers with different address spaces (they are already
+    // checked convertible, i.e. they have the same pointer size), so far we
+    // cannot use `bitcast` (which has restrict on the same address space) or
+    // `addrspacecast` (which is not always no-op casting). Instead, use a pair
+    // of no-op `ptrtoint`/`inttoptr` casts through an integer with the same bit
+    // size.
+    if (OldAS != NewAS) {
+      assert(DL.getPointerSize(OldAS) == DL.getPointerSize(NewAS));
+      return IRB.CreateIntToPtr(IRB.CreatePtrToInt(V, DL.getIntPtrType(OldTy)),
+                                NewTy);
+    }
+  }
+
   return IRB.CreateBitCast(V, NewTy);
 }
 
 /// Test whether the given slice use can be promoted to a vector.
 ///
 /// This function is called to test each entry in a partition which is slated
 /// for a single slice.
 static bool isVectorPromotionViableForSlice(Partition &P, const Slice &S,

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

 ; RUN: opt < %s -sroa -S | FileCheck %s
-target datalayout = "e-p:64:64:64-p1:16:16:16-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:32:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-n8:16:32:64"
+target datalayout = "e-p:64:64:64-p1:16:16:16-p3:32:32:32-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:32:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-n8:16:32:64"
 
 declare void @llvm.memcpy.p0i8.p0i8.i32(i8* nocapture, i8* nocapture readonly, i32, i1)
 declare void @llvm.memcpy.p1i8.p0i8.i32(i8 addrspace(1)* nocapture, i8* nocapture readonly, i32, i1)
 declare void @llvm.memcpy.p0i8.p1i8.i32(i8* nocapture, i8 addrspace(1)* nocapture readonly, i32, i1)
 declare void @llvm.memcpy.p1i8.p1i8.i32(i8 addrspace(1)* nocapture, i8 addrspace(1)* nocapture readonly, i32, i1)
 
 
 ; Make sure an illegal bitcast isn't introduced
for.end:
   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
+; If pointers from different address spaces have different sizes, 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
 }
 
+@l2 = common addrspace(2) global i32 0, align 4
+
+; If pointers from different address spaces have the same size, that pointer
+; should be promoted through the pair of `ptrtoint`/`inttoptr`.
+define i32* @pr27557.alt() {
+; CHECK-LABEL: @pr27557.alt(
+; CHECK: ret i32* inttoptr (i64 ptrtoint (i32 addrspace(2)* @l2 to i64) to i32*)
+  %1 = alloca %union.anon, align 8
+  %2 = bitcast %union.anon* %1 to i32 addrspace(2)**
+  store i32 addrspace(2)* @l2, i32 addrspace(2)** %2, align 8
+  %3 = bitcast %union.anon* %1 to i32**
+  %4 = load i32*, i32** %3, align 8
+  ret i32* %4
+}
+
 ; Make sure pre-splitting doesn't try to introduce an illegal bitcast
 define float @presplit(i64 addrspace(1)* %p) {
 entry:
 ; CHECK-LABEL: @presplit(
 ; CHECK: %[[CAST:.*]] = bitcast i64 addrspace(1)* {{.*}} to i32 addrspace(1)*
 ; CHECK: load i32, i32 addrspace(1)* %[[CAST]]
    %b = alloca i64
    %b.cast = bitcast i64* %b to [2 x float]*

llvm/test/Transforms/SROA/alloca-address-space.ll

 ; RUN: opt < %s -sroa -S | FileCheck %s
-target datalayout = "e-p:64:64:64-p1:16:16:16-p2:32:32-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:32:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-n8:16:32:64-A2"
+target datalayout = "e-p:64:64:64-p1:16:16:16-p2:32:32-p3:32:32:32-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:32:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-n8:16:32:64-A2"
 
 declare void @llvm.memcpy.p2i8.p2i8.i32(i8 addrspace(2)* nocapture, i8 addrspace(2)* nocapture readonly, i32, i1)
 declare void @llvm.memcpy.p1i8.p2i8.i32(i8 addrspace(1)* nocapture, i8 addrspace(2)* nocapture readonly, i32, i1)
 declare void @llvm.memcpy.p2i8.p1i8.i32(i8 addrspace(2)* nocapture, i8 addrspace(1)* nocapture readonly, i32, i1)
 declare void @llvm.memcpy.p1i8.p1i8.i32(i8 addrspace(1)* nocapture, i8 addrspace(1)* nocapture readonly, i32, i1)
 
 
 
for.end:
   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
+; If pointers from different address spaces have different sizes, make sure an
+; illegal bitcast isn't introduced
 ; CHECK-LABEL: @pr27557(
 ; CHECK: %[[CAST:.*]] = bitcast i32* addrspace(2)* {{.*}} to i32 addrspace(3)* addrspace(2)*
 ; CHECK: store i32 addrspace(3)* @l, i32 addrspace(3)* addrspace(2)* %[[CAST]]
 define void @pr27557() {
   %1 = alloca %union.anon, align 8, addrspace(2)
   %2 = bitcast %union.anon addrspace(2)* %1 to i32* addrspace(2)*
   store i32* @g, i32* addrspace(2)* %2, align 8
   %3 = bitcast %union.anon addrspace(2)* %1 to i32 addrspace(3)* addrspace(2)*
   store i32 addrspace(3)* @l, i32 addrspace(3)* addrspace(2)* %3, align 8
   ret void
 }
 
+@l4 = common addrspace(4) global i32 0, align 4
+
+; If pointers from different address spaces have the same size, that pointer
+; should be promoted through the pair of `ptrtoint`/`inttoptr`.
+define i32* @pr27557.alt() {
+; CHECK-LABEL: @pr27557.alt(
+; CHECK: ret i32* inttoptr (i64 ptrtoint (i32 addrspace(4)* @l4 to i64) to i32*)
+  %1 = alloca %union.anon, align 8, addrspace(2)
+  %2 = bitcast %union.anon addrspace(2)* %1 to i32 addrspace(4)* addrspace(2)*
+  store i32 addrspace(4)* @l4, i32 addrspace(4)* addrspace(2)* %2, align 8
+  %3 = bitcast %union.anon addrspace(2)* %1 to i32* addrspace(2)*
+  %4 = load i32*, i32* addrspace(2)* %3, align 8
+  ret i32* %4
+}
+
 ; Test load from and store to non-zero address space.
 define void @test_load_store_diff_addr_space([2 x float] addrspace(1)* %complex1, [2 x float] addrspace(1)* %complex2) {
 ; CHECK-LABEL: @test_load_store_diff_addr_space
 ; CHECK-NOT: alloca
 ; CHECK: load i32, i32 addrspace(1)*
 ; CHECK: load i32, i32 addrspace(1)*
 ; CHECK: store i32 %{{.*}}, i32 addrspace(1)*
 ; CHECK: store i32 %{{.*}}, i32 addrspace(1)*

llvm/test/Transforms/SROA/non-integral-pointers.ll

 neverTaken:
   %x.field.ld.0 = load i64 addrspace(4)*, i64 addrspace(4)** %x
   ret i64 addrspace(4)* %x.field.ld.0
   
 alwaysTaken:
   ret i64 addrspace(4)* null
 }
 
+%union.anon = type { i32* }
+
+; CHECK-LABEL: @f2(
+; CHECK-NOT: ptr2int
+; CHECK-NOT: int2ptr
+define i8 *@f2(i8 addrspace(4)* %p) {
+  %1 = alloca %union.anon, align 8
+  %2 = bitcast %union.anon* %1 to i8 addrspace(4)**
+  store i8 addrspace(4)* %p, i8 addrspace(4)** %2, align 8
+  %3 = bitcast %union.anon* %1 to i8**
+  %4 = load i8*, i8** %3, align 8
+  ret i8* %4
+}
+
 declare void @llvm.memset.p0i8.i64(i8*, i8, i64, i1)