From bd9b22ee76fe1e6f9bbab3a364850e44bf4a0965 Mon Sep 17 00:00:00 2001
From: Matt Arsenault <Matthew.Arsenault@amd.com>
Date: Thu, 25 Jun 2020 19:12:55 -0400
Subject: [PATCH] ValueTracking: Fix isKnownNonZero for non-0 null pointers for
 byval

The IR doesn't have a proper concept of invalid pointers, and "null"
constants are just all zeros (though it really needs one).

I think it's not possible to break this for AMDGPU due to the copy
semantics of byval. If you have an original stack object at 0, the
byval copy will be placed above it so I don't think it's really
possible to hit a 0 address.
---
 lib/Analysis/ValueTracking.cpp                | 13 ++++++----
 .../null-ptr-is-valid-attribute.ll            | 20 ++++++++++++++++
 .../InstSimplify/null-ptr-is-valid.ll         | 24 +++++++++++++++++++
 3 files changed, 53 insertions(+), 4 deletions(-)
 create mode 100644 test/Transforms/InstSimplify/null-ptr-is-valid-attribute.ll
 create mode 100644 test/Transforms/InstSimplify/null-ptr-is-valid.ll

diff --git a/lib/Analysis/ValueTracking.cpp b/lib/Analysis/ValueTracking.cpp
index 43caaa62c2e..306f6a4a35c 100644
--- a/lib/Analysis/ValueTracking.cpp
+++ b/lib/Analysis/ValueTracking.cpp
@@ -2353,15 +2353,20 @@ bool isKnownNonZero(const Value *V, const APInt &DemandedElts, unsigned Depth,
     return false;
 
   // Check for pointer simplifications.
-  if (V->getType()->isPointerTy()) {
+
+  if (PointerType *PtrTy = dyn_cast<PointerType>(V->getType())) {
     // Alloca never returns null, malloc might.
     if (isa<AllocaInst>(V) && Q.DL.getAllocaAddrSpace() == 0)
       return true;
 
-    // A byval, inalloca, or nonnull argument is never null.
-    if (const Argument *A = dyn_cast<Argument>(V))
-      if (A->hasPassPointeeByValueAttr() || A->hasNonNullAttr())
+    // A byval, inalloca may not be null in a non-default addres space. A
+    // nonnull argument is assumed never 0.
+    if (const Argument *A = dyn_cast<Argument>(V)) {
+      if (((A->hasPassPointeeByValueAttr() &&
+            !NullPointerIsDefined(A->getParent(), PtrTy->getAddressSpace())) ||
+           A->hasNonNullAttr()))
         return true;
+    }
 
     // A Load tagged with nonnull metadata is never null.
     if (const LoadInst *LI = dyn_cast<LoadInst>(V))
diff --git a/test/Transforms/InstSimplify/null-ptr-is-valid-attribute.ll b/test/Transforms/InstSimplify/null-ptr-is-valid-attribute.ll
new file mode 100644
index 00000000000..8ce04e7d3a9
--- /dev/null
+++ b/test/Transforms/InstSimplify/null-ptr-is-valid-attribute.ll
@@ -0,0 +1,20 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -S -instsimplify < %s | FileCheck %s
+
+; A 0 valued byval pointer may be valid
+define i1 @byval_may_be_zero(i32* byval(i32) %ptr) null_pointer_is_valid {
+; CHECK-LABEL: @byval_may_be_zero(
+; CHECK-NEXT:    [[CMP:%.*]] = icmp eq i32* [[PTR:%.*]], null
+; CHECK-NEXT:    ret i1 [[CMP]]
+;
+  %cmp = icmp eq i32* %ptr, null
+  ret i1 %cmp
+}
+
+define i1 @nonnull_may_be_zero(i32* nonnull %ptr) null_pointer_is_valid {
+; CHECK-LABEL: @nonnull_may_be_zero(
+; CHECK-NEXT:    ret i1 false
+;
+  %cmp = icmp eq i32* %ptr, null
+  ret i1 %cmp
+}
diff --git a/test/Transforms/InstSimplify/null-ptr-is-valid.ll b/test/Transforms/InstSimplify/null-ptr-is-valid.ll
new file mode 100644
index 00000000000..7b7fb140e8c
--- /dev/null
+++ b/test/Transforms/InstSimplify/null-ptr-is-valid.ll
@@ -0,0 +1,24 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -S -instsimplify < %s | FileCheck %s
+
+target datalayout = "A5"
+
+; A 0 valued byval pointer may be valid
+define i1 @byval_may_be_zero(i32 addrspace(5)* byval(i32) %ptr) {
+; CHECK-LABEL: @byval_may_be_zero(
+; CHECK-NEXT:    [[CMP:%.*]] = icmp eq i32 addrspace(5)* [[PTR:%.*]], null
+; CHECK-NEXT:    ret i1 [[CMP]]
+;
+  %cmp = icmp eq i32 addrspace(5)* %ptr, null
+  ret i1 %cmp
+}
+
+; FIXME: The interpretation of nonnull assumes a 0 pointer value, so
+; this really is an incorrect fold.
+define i1 @nonnull_may_be_zero(i32 addrspace(5)* nonnull %ptr) {
+; CHECK-LABEL: @nonnull_may_be_zero(
+; CHECK-NEXT:    ret i1 false
+;
+  %cmp = icmp eq i32 addrspace(5)* %ptr, null
+  ret i1 %cmp
+}