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 ↗

(On Diff #49073)

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 ↗

(On Diff #49073)

I'd expect this to call isDereferenceableFromAttribute.

[apilipenko]

Check CheckForNonNull.

[apilipenko]

lib/IR/Value.cpp

556 ↗

(On Diff #49073)

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.

llvm/trunk/include/llvm/IR/Value.h

#include "llvm/IR/Value.def"
 
   /// \brief Returns the number of bytes known to be dereferenceable for the
   /// pointer value.
   ///
   /// If CanBeNull is set by this function the pointer can either be null or be
   /// 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
   /// align attribute of a function argument, or guaranteed by DataLayout.
   unsigned getPointerAlignment(const DataLayout &DL) const;
 
   /// \brief Translate PHI node to its predecessor from the given basic block.
   ///

llvm/trunk/lib/Analysis/Loads.cpp

 /// a simple load or store.
 static bool isDereferenceableAndAlignedPointer(
     const Value *V, unsigned Align, const DataLayout &DL,
     const Instruction *CtxI, const DominatorTree *DT,
     const TargetLibraryInfo *TLI, SmallPtrSetImpl<const Value *> &Visited) {
   // 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 CheckForNonNull;
+  if (V->isPointerDereferenceable(CheckForNonNull)) {
+    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*
   // would result in a 4-byte load from a 1-byte alloca. However,
   // if we're casting from a pointer from a type of larger size
   // to a type of smaller size (or the same size), and the alignment
   // is at least as large as for the resulting pointer type, then
   // we can look through the bitcast.
   if (const BitCastOperator *BC = dyn_cast<BitCastOperator>(V)) {
     Type *STy = BC->getSrcTy()->getPointerElementType(),
          *DTy = BC->getDestTy()->getPointerElementType();
     if (STy->isSized() && DTy->isSized() &&
         (DL.getTypeStoreSize(STy) >= DL.getTypeStoreSize(DTy)) &&
         (DL.getABITypeAlignment(STy) >= DL.getABITypeAlignment(DTy)))
       return isDereferenceableAndAlignedPointer(BC->getOperand(0), Align, DL,
                                                 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);
 
   // For GEPs, determine if the indexing lands within the allocated object.
   if (const GEPOperator *GEP = dyn_cast<GEPOperator>(V)) {
     Type *Ty = GEP->getResultElementType();
     const Value *Base = GEP->getPointerOperand();
 

llvm/trunk/lib/IR/Value.cpp

if (DerefBytes == 0) {
         DerefBytes = CI->getLimitedValue();
       }
       CanBeNull = true;
     }
   }
   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");
 
   unsigned Align = 0;
   if (auto *GO = dyn_cast<GlobalObject>(this)) {
     Align = GO->getAlignment();
     if (Align == 0) {
       if (auto *GVar = dyn_cast<GlobalVariable>(GO)) {