[SROA] Add support for non-integral pointers

[sanjoy]

Summary

C.f. http://llvm.org/docs/LangRef.html#non-integral-pointer-type

[sanjoy]

ping!

[sanjoy]

ping

[chandlerc]

relatively minor stuff here

lib/Transforms/Scalar/SROA.cpp

1640–1641 ↗

(On Diff #95659)

I think these need to be in separate tests with comments, otherwise its too confusing. I thought at first this was a bug because we did *NewTy*->isIntegerTy() and isNonIntegralPointerType(*OldTy*)... But I think that is actually correct?

A comment here saying "if we can convert from pointer to integer" and then "the reverse conversion from integer to pointer" on two separate tests would be easier to read I think.

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

1–4 ↗

(On Diff #95659)

Comment to help the reader out by explaining what this is testing and where this shows up in the datalayout?

[sanjoy]

• address review

[sanjoy]

ping!

[reames]

Chandler, is this ready to go in? Or do you want to see further changes?

[loladiro]

LGTM, FWIW. I encountered this issue as well and this seems like the correct solution.

llvm/trunk/lib/Transforms/Scalar/SROA.cpp

static bool canConvertValue(const DataLayout &DL, Type *OldTy, Type *NewTy) {
   // 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();
     }
-    if (NewTy->isIntegerTy() || OldTy->isIntegerTy())
-      return true;
+
+    // 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
+    // need to remain pointers.
+    if (!DL.isNonIntegralPointerType(OldTy))
+      return NewTy->isIntegerTy();
+
     return false;
   }
 
   return true;
 }
 
 /// \brief Generic routine to convert an SSA value to a value of a different
 /// type.

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

+; RUN: opt -sroa -S < %s | FileCheck %s
+
+; This test checks that SROA does not introduce ptrtoint and inttoptr
+; casts from and to non-integral pointers.  The "ni:4" bit in the
+; datalayout states that pointers of address space 4 are to be
+; considered "non-integral".
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128-ni:4"
+target triple = "x86_64-unknown-linux-gnu"
+
+define void @f0(i1 %alwaysFalse, i64 %val) {
+; CHECK-LABEL: @f0(
+; CHECK-NOT: inttoptr
+; CHECK-NOT: ptrtoint
+entry:
+  %loc = alloca i64
+  store i64 %val, i64* %loc
+  br i1 %alwaysFalse, label %neverTaken, label %alwaysTaken
+
+neverTaken:
+  %loc.bc = bitcast i64* %loc to i8 addrspace(4)**
+  %ptr = load i8 addrspace(4)*, i8 addrspace(4)** %loc.bc
+  store i8 5, i8 addrspace(4)* %ptr
+  ret void
+
+alwaysTaken:
+  ret void
+}
+
+define i64 @f1(i1 %alwaysFalse, i8 addrspace(4)* %val) {
+; CHECK-LABEL: @f1(
+; CHECK-NOT: inttoptr
+; CHECK-NOT: ptrtoint
+entry:
+  %loc = alloca i8 addrspace(4)*
+  store i8 addrspace(4)* %val, i8 addrspace(4)** %loc
+  br i1 %alwaysFalse, label %neverTaken, label %alwaysTaken
+
+neverTaken:
+  %loc.bc = bitcast i8 addrspace(4)** %loc to i64*
+  %int = load i64, i64* %loc.bc
+  ret i64 %int
+
+alwaysTaken:
+  ret i64 42
+}