Allow ptrtoint/inttoptr of non-integral pointer types in IR

preames · preames · commit ac81cb7e6dde · 2021-06-11T13:38:32.000-07:00
I don't like landing this change, but it's an acknowledgement of a practical reality.  Despite not having well specified semantics for inttoptr and ptrtoint involving non-integral pointer types, they are used in practice.  Here's a quick summary of the current pragmatic reality:
* I happen to know that the main external user of non-integral pointers has effectively disabled the verifier rules.
* RS4GC (the lowering pass for abstract GC machine model which is the key motivation for non-integral pointers), even supports them.  We just have all the tests using an integral pointer space to let the verifier run.
* Certain idioms (such as alignment checks for alignment N, where any relocation is guaranteed to be N byte aligned) are fine in practice.
* As implemented, inttoptr/ptrtoint are CSEd and are not control dependent.  This means that any code which is intending to check a particular bit pattern at site of use must be wrapped in an intrinsic or external function call.

This change allows them in the Verifier, and updates the LangRef to specific them as implementation dependent.  This allows us to acknowledge current reality while still leaving ourselves room to punt on figuring out "good" semantics until the future.
diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst
@@ -601,10 +601,11 @@ Non-integral pointer types represent pointers that have an *unspecified* bitwise
 representation; that is, the integral representation may be target dependent or
 unstable (not backed by a fixed integer).
 
-``inttoptr`` instructions converting integers to non-integral pointer types are
-ill-typed, and so are ``ptrtoint`` instructions converting values of
-non-integral pointer types to integers.  Vector versions of said instructions
-are ill-typed as well.
+``inttoptr`` and ``ptrtoint`` instructions converting integers to non-integral
+pointer types or vice versa are implementation defined, and subject to likely
+future revision in semantics. Vector versions of said instructions are as well.
+Users of non-integral-pointer types are advised not to design around current
+semantics as they may very well change in the nearish future.
 
 .. _globalvars:
 
diff --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp
@@ -2178,19 +2178,6 @@ void Verifier::visitConstantExpr(const ConstantExpr *CE) {
     Assert(CastInst::castIsValid(Instruction::BitCast, CE->getOperand(0),
                                  CE->getType()),
            "Invalid bitcast", CE);
-
-  if (CE->getOpcode() == Instruction::IntToPtr ||
-      CE->getOpcode() == Instruction::PtrToInt) {
-    auto *PtrTy = CE->getOpcode() == Instruction::IntToPtr
-                      ? CE->getType()
-                      : CE->getOperand(0)->getType();
-    StringRef Msg = CE->getOpcode() == Instruction::IntToPtr
-                        ? "inttoptr not supported for non-integral pointers"
-                        : "ptrtoint not supported for non-integral pointers";
-    Assert(
-        !DL.isNonIntegralPointerType(cast<PointerType>(PtrTy->getScalarType())),
-        Msg);
-  }
 }
 
 bool Verifier::verifyAttributeCount(AttributeList Attrs, unsigned Params) {
@@ -3041,10 +3028,6 @@ void Verifier::visitPtrToIntInst(PtrToIntInst &I) {
 
   Assert(SrcTy->isPtrOrPtrVectorTy(), "PtrToInt source must be pointer", &I);
 
-  if (auto *PTy = dyn_cast<PointerType>(SrcTy->getScalarType()))
-    Assert(!DL.isNonIntegralPointerType(PTy),
-           "ptrtoint not supported for non-integral pointers");
-
   Assert(DestTy->isIntOrIntVectorTy(), "PtrToInt result must be integral", &I);
   Assert(SrcTy->isVectorTy() == DestTy->isVectorTy(), "PtrToInt type mismatch",
          &I);
@@ -3068,10 +3051,6 @@ void Verifier::visitIntToPtrInst(IntToPtrInst &I) {
          "IntToPtr source must be an integral", &I);
   Assert(DestTy->isPtrOrPtrVectorTy(), "IntToPtr result must be a pointer", &I);
 
-  if (auto *PTy = dyn_cast<PointerType>(DestTy->getScalarType()))
-    Assert(!DL.isNonIntegralPointerType(PTy),
-           "inttoptr not supported for non-integral pointers");
-
   Assert(SrcTy->isVectorTy() == DestTy->isVectorTy(), "IntToPtr type mismatch",
          &I);
   if (SrcTy->isVectorTy()) {
@@ -4539,12 +4518,9 @@ void Verifier::visitInstruction(Instruction &I) {
       Assert(CBI && &CBI->getCalledOperandUse() == &I.getOperandUse(i),
              "Cannot take the address of an inline asm!", &I);
     } else if (ConstantExpr *CE = dyn_cast<ConstantExpr>(I.getOperand(i))) {
-      if (CE->getType()->isPtrOrPtrVectorTy() ||
-          !DL.getNonIntegralAddressSpaces().empty()) {
+      if (CE->getType()->isPtrOrPtrVectorTy()) {
         // If we have a ConstantExpr pointer, we need to see if it came from an
-        // illegal bitcast.  If the datalayout string specifies non-integral
-        // address spaces then we also need to check for illegal ptrtoint and
-        // inttoptr expressions.
+        // illegal bitcast.
         visitConstantExprsRecursively(CE);
       }
     }
diff --git a/llvm/test/Transforms/RewriteStatepointsForGC/base-inttoptr.ll b/llvm/test/Transforms/RewriteStatepointsForGC/base-inttoptr.ll
@@ -2,6 +2,7 @@
 ; RUN: opt -S -rewrite-statepoints-for-gc < %s | FileCheck %s
 
 target triple = "x86_64-unknown-linux-gnu"
+target datalayout = "e-ni:1:6"
 
 declare void @foo()
 
diff --git a/llvm/test/Transforms/RewriteStatepointsForGC/constants.ll b/llvm/test/Transforms/RewriteStatepointsForGC/constants.ll
@@ -1,6 +1,8 @@
 ; RUN: opt -S -rewrite-statepoints-for-gc < %s | FileCheck %s
 ; RUN: opt -S -passes=rewrite-statepoints-for-gc < %s | FileCheck %s
 
+target datalayout = "e-ni:1:6"
+
 ; constants don't get relocated.
 @G = addrspace(1) global i8 5
 
diff --git a/llvm/test/Verifier/non-integral-pointers.ll b/llvm/test/Verifier/non-integral-pointers.ll
@@ -1,84 +1,118 @@
-; RUN: not opt -verify < %s 2>&1 | FileCheck %s
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -verify -S < %s 2>&1 | FileCheck %s
 
 target datalayout = "e-ni:4:6"
 
 define i64 @f_0(i8 addrspace(4)* %ptr) {
-; CHECK: ptrtoint not supported for non-integral pointers
+; CHECK-LABEL: @f_0(
+; CHECK-NEXT:    [[VAL:%.*]] = ptrtoint i8 addrspace(4)* [[PTR:%.*]] to i64
+; CHECK-NEXT:    ret i64 [[VAL]]
+;
   %val = ptrtoint i8 addrspace(4)* %ptr to i64
   ret i64 %val
 }
 
 define <4 x i64> @f_1(<4 x i8 addrspace(4)*> %ptr) {
-; CHECK: ptrtoint not supported for non-integral pointers
+; CHECK-LABEL: @f_1(
+; CHECK-NEXT:    [[VAL:%.*]] = ptrtoint <4 x i8 addrspace(4)*> [[PTR:%.*]] to <4 x i64>
+; CHECK-NEXT:    ret <4 x i64> [[VAL]]
+;
   %val = ptrtoint <4 x i8 addrspace(4)*> %ptr to <4 x i64>
   ret <4 x i64> %val
 }
 
 define i64 @f_2(i8 addrspace(3)* %ptr) {
-; Negative test
+; CHECK-LABEL: @f_2(
+; CHECK-NEXT:    [[VAL:%.*]] = ptrtoint i8 addrspace(3)* [[PTR:%.*]] to i64
+; CHECK-NEXT:    ret i64 [[VAL]]
+;
   %val = ptrtoint i8 addrspace(3)* %ptr to i64
   ret i64 %val
 }
 
 define i8 addrspace(4)* @f_3(i64 %integer) {
-; CHECK: inttoptr not supported for non-integral pointers
+; CHECK-LABEL: @f_3(
+; CHECK-NEXT:    [[VAL:%.*]] = inttoptr i64 [[INTEGER:%.*]] to i8 addrspace(4)*
+; CHECK-NEXT:    ret i8 addrspace(4)* [[VAL]]
+;
   %val = inttoptr i64 %integer to i8 addrspace(4)*
   ret i8 addrspace(4)* %val
 }
 
 define <4 x i8 addrspace(4)*> @f_4(<4 x i64> %integer) {
-; CHECK: inttoptr not supported for non-integral pointers
+; CHECK-LABEL: @f_4(
+; CHECK-NEXT:    [[VAL:%.*]] = inttoptr <4 x i64> [[INTEGER:%.*]] to <4 x i8 addrspace(4)*>
+; CHECK-NEXT:    ret <4 x i8 addrspace(4)*> [[VAL]]
+;
   %val = inttoptr <4 x i64> %integer to <4 x i8 addrspace(4)*>
   ret <4 x i8 addrspace(4)*> %val
 }
 
 define i8 addrspace(3)* @f_5(i64 %integer) {
-; Negative test
+; CHECK-LABEL: @f_5(
+; CHECK-NEXT:    [[VAL:%.*]] = inttoptr i64 [[INTEGER:%.*]] to i8 addrspace(3)*
+; CHECK-NEXT:    ret i8 addrspace(3)* [[VAL]]
+;
   %val = inttoptr i64 %integer to i8 addrspace(3)*
   ret i8 addrspace(3)* %val
 }
 
 define i64 @f_6(i8 addrspace(6)* %ptr) {
-; CHECK: ptrtoint not supported for non-integral pointers
+; CHECK-LABEL: @f_6(
+; CHECK-NEXT:    [[VAL:%.*]] = ptrtoint i8 addrspace(6)* [[PTR:%.*]] to i64
+; CHECK-NEXT:    ret i64 [[VAL]]
+;
   %val = ptrtoint i8 addrspace(6)* %ptr to i64
   ret i64 %val
 }
 
 define i8 addrspace(4)* @f_7() {
-; CHECK: inttoptr not supported for non-integral pointers
+; CHECK-LABEL: @f_7(
+; CHECK-NEXT:    ret i8 addrspace(4)* inttoptr (i64 50 to i8 addrspace(4)*)
+;
   ret i8 addrspace(4)* inttoptr (i64 50 to i8 addrspace(4)*)
 }
 
 @global0 = addrspace(4) constant i8 42
 
 define i64 @f_8() {
-; CHECK: ptrtoint not supported for non-integral pointers
+; CHECK-LABEL: @f_8(
+; CHECK-NEXT:    ret i64 ptrtoint (i8 addrspace(4)* @global0 to i64)
+;
   ret i64 ptrtoint (i8 addrspace(4)* @global0 to i64)
 }
 
 define i8 addrspace(4)* @f_9() {
-; CHECK: inttoptr not supported for non-integral pointers
+; CHECK-LABEL: @f_9(
+; CHECK-NEXT:    ret i8 addrspace(4)* getelementptr (i8, i8 addrspace(4)* inttoptr (i64 55 to i8 addrspace(4)*), i32 100)
+;
   ret i8 addrspace(4)* getelementptr (i8, i8 addrspace(4)* inttoptr (i64 55 to i8 addrspace(4)*), i32 100)
 }
 
 @global1 = addrspace(4) constant i8 42
 
 define i8 addrspace(4)* @f_10() {
-; CHECK: ptrtoint not supported for non-integral pointers
+; CHECK-LABEL: @f_10(
+; CHECK-NEXT:    ret i8 addrspace(4)* getelementptr (i8, i8 addrspace(4)* @global0, i64 ptrtoint (i8 addrspace(4)* @global1 to i64))
+;
   ret i8 addrspace(4)* getelementptr (i8, i8 addrspace(4)* @global0, i64 ptrtoint (i8 addrspace(4)* @global1 to i64))
 }
 
 @cycle_0 = addrspace(4) constant i64 ptrtoint (i64 addrspace(4)* addrspace(4)* @cycle_1 to i64)
 @cycle_1 = addrspace(4) constant i64 addrspace(4) * @cycle_0
 
 define i64 addrspace(4)* addrspace(4)* @f_11() {
-; CHECK: ptrtoint not supported for non-integral pointers
+; CHECK-LABEL: @f_11(
+; CHECK-NEXT:    ret i64 addrspace(4)* addrspace(4)* @cycle_1
+;
   ret i64 addrspace(4)* addrspace(4)* @cycle_1
 }
 
 @cycle_self = addrspace(4) constant i64 ptrtoint (i64 addrspace(4)* @cycle_self to i64)
 
 define i64 addrspace(4)* @f_12() {
-; CHECK: ptrtoint not supported for non-integral pointers
+; CHECK-LABEL: @f_12(
+; CHECK-NEXT:    ret i64 addrspace(4)* @cycle_self
+;
   ret i64 addrspace(4)* @cycle_self
 }
