[PATCH] D50010: [VNCoercion] Disallow coercion between different ni addrspaces

[Keno Fischer via Phabricator via llvm-commits]

loladiro created this revision.
loladiro added reviewers: sanjoy, reames, dberlin.

I'm not sure if it would be legal by the IR reference to introduce
an addrspacecast here, since the IR reference is a bit vague on
the exact semantics, but at least for our usage of it (and I
suspect for many other's usage) it is not. For us, addrspacecasts
between non-integral address spaces carry frontend information that the
optimizer cannot deduce afterwards in a generic way (though we
have frontend specific passes in our pipline that do propagate
these). In any case, I'm sure nobody is using it this way at
the moment, since it would have introduced inttoptrs, which
are definitely illegal.

Fixes PR38375


Repository:
  rL LLVM

https://reviews.llvm.org/D50010

Files:
  lib/Transforms/Utils/VNCoercion.cpp
  test/Transforms/GVN/non-integral-pointers.ll


Index: test/Transforms/GVN/non-integral-pointers.ll
===================================================================
--- test/Transforms/GVN/non-integral-pointers.ll
+++ test/Transforms/GVN/non-integral-pointers.ll
@@ -1,6 +1,6 @@
 ; RUN: opt -gvn -S < %s | FileCheck %s
 
-target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128-ni:4"
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128-ni:4:5"
 target triple = "x86_64-unknown-linux-gnu"
 
 define void @f0(i1 %alwaysFalse, i64 %val, i64* %loc) {
@@ -37,3 +37,21 @@
  alwaysTaken:
   ret i64 42
 }
+
+ define i8 addrspace(5)* @multini(i1 %alwaysFalse, i8 addrspace(4)* %val, i8 addrspace(4)** %loc) {
+ ; CHECK-LABEL: @multini(
+ ; CHECK-NOT: inttoptr
+ ; CHECK-NOT: ptrtoint
+ ; CHECK-NOT: addrspacecast
+  entry:
+   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 i8 addrspace(5)**
+   %differentas = load i8 addrspace(5)*, i8 addrspace(5)** %loc.bc
+   ret i8 addrspace(5)* %differentas
+
+  alwaysTaken:
+   ret i8 addrspace(5)* null
+ }
Index: lib/Transforms/Utils/VNCoercion.cpp
===================================================================
--- lib/Transforms/Utils/VNCoercion.cpp
+++ lib/Transforms/Utils/VNCoercion.cpp
@@ -20,7 +20,8 @@
       StoredVal->getType()->isStructTy() || StoredVal->getType()->isArrayTy())
     return false;
 
-  uint64_t StoreSize = DL.getTypeSizeInBits(StoredVal->getType());
+  Type *StoredValTy = StoredVal->getType();
+  uint64_t StoreSize = DL.getTypeSizeInBits(StoredValTy);
 
   // The store size must be byte-aligned to support future type casts.
   if (llvm::alignTo(StoreSize, 8) != StoreSize)
@@ -30,10 +31,15 @@
   if (StoreSize < DL.getTypeSizeInBits(LoadTy))
     return false;
 
-  // Don't coerce non-integral pointers to integers or vice versa.
-  if (DL.isNonIntegralPointerType(StoredVal->getType()) !=
-      DL.isNonIntegralPointerType(LoadTy))
+  bool StoredNI = DL.isNonIntegralPointerType(StoredValTy);
+  bool LoadNI = DL.isNonIntegralPointerType(LoadTy);
+  if (StoredNI != LoadNI) {
+    return false;
+  } else if (StoredNI && LoadNI &&
+             cast<PointerType>(StoredValTy)->getAddressSpace() !=
+                 cast<PointerType>(LoadTy)->getAddressSpace()) {
     return false;
+  }
 
   return true;
 }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D50010.158063.patch
Type: text/x-patch
Size: 2384 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180730/5b8e4674/attachment-0001.bin>