NFC. Introduce Value::isPointerDereferenceable

[apilipenko]

Summary

This is analogous to D17572 (and the diff is based upon it). Extract a part of isDereferenceableAndAlignedPointer functionality to Value::isPointerDereferenceable. The difference between those function is that isPointerDereferenceable can be used for opaque types.

[hfinkel]

lib/Analysis/Loads.cpp

96

Leaving CheckForNull unimplemented seems prone to confusion, and this assert text does not say that this is not yet implemented. Do you just need to call isKnownNonNull? If so, do that.

[majnemer]

lib/IR/Value.cpp

556

I'd expect this to call isDereferenceableFromAttribute.

[apilipenko]

Check CheckForNonNull.

[apilipenko]

lib/IR/Value.cpp

556

Can you elaborate? isDereferenceableFromAttribute is an internal function in Loads.cpp, while this is a public API to get inherent property of the value.

[sanjoy]

The overall idea seems reasonable, but isn't this increasing our dependence on pointee types?

[apilipenko]

@sanjoy, it has nothing to do with pointer types. There are some "kinds" of pointers (e.g. allocas, some globals) which are inherently dereferenceable.

[sanjoy]

In D17611#417343, @apilipenko wrote:

@sanjoy, it has nothing to do with pointer types. There are some "kinds" of pointers (e.g. allocas, some globals) which are inherently dereferenceable.

I should've been clearer: this is in context of removing pointee types
from LLVM, and having only on pointer type per address space. In that
world, the current trick of "remembering" the number of bytes we want
dereferenceable implicitly via the type of the pointer won't work.

Perhaps a slightly more future proof API would be:

bool isPointerDereferenceable(bool &CanBeNull, unsigned SizeInBytes) const;

so that it can work with (made up syntax):

%ptr = alloca i32
...  ;; ptr is of type pointer addrspace(1), no i32 involved
%val = select <cond>, ..., %ptr
load i16, %val ;; dereferenceable

and

%ptr = alloca i32
...  ;; ptr is of type pointer addrspace(1), no i32 involved
%val = select <cond>, ..., %ptr
load i64, %val  ;; not dereferenceable

However, your patch does not make things any worse, so this is a minor
nit at best, fix at your own discretion.

include/llvm/IR/Value.h

   /// dereferenceable up to the returned number of bytes.
   unsigned getPointerDereferenceableBytes(bool &CanBeNull) const;
 
+  /// \brief Returns true if the pointer value is fully dereferenceable.
+  ///
+  /// Sets CanBeNull to true if the pointer is either null or can be fully
+  /// dereferenceable.
+  bool isPointerDereferenceable(bool &CanBeNull) const;
+
   /// \brief Returns an alignment of the pointer value.
   ///
   /// Returns an alignment which is either specified explicitly, e.g. via

lib/Analysis/Loads.cpp

   // Note that it is not safe to speculate into a malloc'd region because
   // malloc may return null.
 
-  // These are obviously ok if aligned.
-  if (isa<AllocaInst>(V))
+  bool CheckForNull;
+  if (V->isPointerDereferenceable(CheckForNull)) {
+    if (CheckForNonNull && !isKnownNonNullAt(V, CtxI, DT, TLI))
+      return false;
     return isAligned(V, Align, DL);
+  }
 
   // It's not always safe to follow a bitcast, for example:
   //   bitcast i8* (alloca i8) to i32*

                                                 CtxI, DT, TLI, Visited);
   }
 
-  // Global variables which can't collapse to null are ok.
-  if (const GlobalVariable *GV = dyn_cast<GlobalVariable>(V))
-    if (!GV->hasExternalWeakLinkage())
-      return isAligned(V, Align, DL);
-
-  // byval arguments are okay.
-  if (const Argument *A = dyn_cast<Argument>(V))
-    if (A->hasByValAttr())
-      return isAligned(V, Align, DL);
-
   if (isDereferenceableFromAttribute(V, DL, CtxI, DT, TLI))
     return isAligned(V, Align, DL);
 

lib/IR/Value.cpp

   return DerefBytes;
 }
 
+bool Value::isPointerDereferenceable(bool &CanBeNull) const {
+  assert(getType()->isPointerTy() && "must be pointer");
+
+  CanBeNull = false;
+
+  // These are obviously ok.
+  if (isa<AllocaInst>(this))
+    return true;
+
+  // Global variables which can't collapse to null are ok.
+  // TODO: return true for those but set CanBeNull flag
+  if (const GlobalVariable *GV = dyn_cast<GlobalVariable>(this))
+    if (!GV->hasExternalWeakLinkage())
+      return true;
+
+  // byval arguments are okay.
+  if (const Argument *A = dyn_cast<Argument>(this))
+    if (A->hasByValAttr())
+      return true;
+
+  return false;
+}
+
 unsigned Value::getPointerAlignment(const DataLayout &DL) const {
   assert(getType()->isPointerTy() && "must be pointer");