[GVN] Don't coerce non-integral pointers to integers or vice versa

[sanjoy]

Summary

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

The NewGVN test does not fail without these changes (perhaps it does
try to coerce pointers <-> integers to begin with?), but I added the
test case anyway.

[davide]

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

25–28 ↗

(On Diff #95668)

I think a test made only of CHECK-NOT is a little brittle. Can you expand it a bit?

[sanjoy]

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

25–28 ↗

(On Diff #95668)

I could expand it to CHECK for what GVN generates here (IIRC it does not do anything), but that would mislead the reader about the purpose of this test. This test is really intended to check that GVN did not generate inttoptr or ptrtoint, so I think a CHECK-NOT based test is appropriate here.

[davide]

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

25–28 ↗

(On Diff #95668)

Yes, it shouldn't do anything. Probably load coercion tests should cover other cases so, after a second look, I guess your CHECK lines are fine.

llvm/trunk/lib/Transforms/Utils/VNCoercion.cpp

bool canCoerceMustAliasedValueToLoad(Value *StoredVal, Type *LoadTy,
   if (LoadTy->isStructTy() || LoadTy->isArrayTy() ||
       StoredVal->getType()->isStructTy() || StoredVal->getType()->isArrayTy())
     return false;
 
   // The store has to be at least as big as the load.
   if (DL.getTypeSizeInBits(StoredVal->getType()) < DL.getTypeSizeInBits(LoadTy))
     return false;
 
+  // Don't coerce non-integral pointers to integers or vice versa.
+  if (DL.isNonIntegralPointerType(StoredVal->getType()) !=
+      DL.isNonIntegralPointerType(LoadTy))
+    return false;
+
   return true;
 }
 
 template <class T, class HelperClass>
 static T *coerceAvailableValueToLoadTypeHelper(T *StoredVal, Type *LoadedTy,
                                                HelperClass &Helper,
                                                const DataLayout &DL) {
   assert(canCoerceMustAliasedValueToLoad(StoredVal, LoadedTy, DL) &&

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

+; RUN: opt -gvn -S < %s | FileCheck %s
+
+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, i64* %loc) {
+; CHECK-LABEL: @f0(
+; CHECK-NOT: inttoptr
+; CHECK-NOT: ptrtoint
+ entry:
+  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, i8 addrspace(4)** %loc) {
+; CHECK-LABEL: @f1(
+; CHECK-NOT: inttoptr
+; CHECK-NOT: ptrtoint
+ 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 i64*
+  %int = load i64, i64* %loc.bc
+  ret i64 %int
+
+ alwaysTaken:
+  ret i64 42
+}

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

+; RUN: opt -newgvn -S < %s | FileCheck %s
+
+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, i64* %loc) {
+; CHECK-LABEL: @f0(
+; CHECK-NOT: inttoptr
+; CHECK-NOT: ptrtoint
+ entry:
+  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, i8 addrspace(4)** %loc) {
+; CHECK-LABEL: @f1(
+; CHECK-NOT: inttoptr
+; CHECK-NOT: ptrtoint
+ 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 i64*
+  %int = load i64, i64* %loc.bc
+  ret i64 %int
+
+ alwaysTaken:
+  ret i64 42
+}