[opaque pointer types] [NFC] isDereferenceable{,AndAligned}Pointer: take the accessed size (and alignment) as arguments.

[mjacob]

lib/Transforms/IPO/ArgumentPromotion.cpp

432

Can we pass the size instead of pointee type here?

[eddyb]

lib/Transforms/IPO/ArgumentPromotion.cpp

432

Yes, but the method is local to this file and all the callers already have a type.
Passing a size means using DL.getTypeStoreSize at each call site, which is anti-DRY IMO.

[dblaikie]

include/llvm/Analysis/ValueTracking.h

244

Do we still need to pass the DataLyaout to these functions if they have the size & alignment?

lib/Analysis/ValueTracking.cpp

3260

Where did this case come from?

3308

Where'd this check go?

lib/Transforms/InstCombine/InstCombineCalls.cpp

1779

Why is it only '1', rather than 'Bytes' used below?

[eddyb]

include/llvm/Analysis/ValueTracking.h

244

Yes, it's used all over the place in ValueTracking (to get the sizes of other types found during the process).

lib/Analysis/ValueTracking.cpp

3260

Previously, if Align was 0, then BaseAlign would take over.
Instead, I'm taking the maximum alignment required by either, but I'm not 100% sure it does the exact same thing, only that it passes the tests.

3308

Based on the fact that you can't get the Size necessary to call this function if you have an unsized type.

But I can add the check back, as if (Size > 0), if you want.

lib/Transforms/InstCombine/InstCombineCalls.cpp

1779

I recall this being i8*, but if that is not the case, I'll use Bytes instead.

[mjacob]

lib/Transforms/IPO/ArgumentPromotion.cpp

432

This is a tradeoff we have to make anyway. E.g. in this revision you introduce more calls to DL.getTypeStoreSize() than you remove. IMHO there is no way to avoid this if we want to make pointers opaque.

Anyway, isSafeToPromoteArgument() has only two callers, one of these is the method itself.

[eddyb]

lib/Transforms/IPO/ArgumentPromotion.cpp

432

Odd, I must've misread something else, I thought I saw half a dozen calls. Fair enough then.

[eddyb]

Sorry @dblaikie, but I almost missed these comments as they weren't showing up in Differential.

I've tried to manually quote them here so that the review makes sense.

lib/Analysis/ValueTracking.cpp

3260

@dblaikie (via email)
I think it'd be best to preserve the old behavior. Feel free to add a
// FIXME: Is this really the right thing, shouldn't we use the blah blah blah
or the like.

There are only 4 possible cases, for a GEP getting a T field in S:

• No explicit align: before was alignof(S)
  • alignof(S) >= alignof(T): now alignof(S) (same)
  • alignof(S) < alignof(T): now alignof(T) (larger, i.e. overly conservative)
• Explicit align (A): before was A
  • alignof(S) <= A: now A (same)
  • alignof(S) > A: now alignof(S) (larger, i.e. overly conservative)

The alignof(S) < alignof(T) case seems interesting to me: can an aggregate have a smaller alignment than one of its fields?

And I'm not sure what, if any, bounds are placed on explicit alignments.

So let's say I want to preserve the old behavior.
I could pass *both* the ABI alignment and the explicit alignment, but that would be even noisier.
Or I could bundle the alignment value and a boolean flag indicating whether it's ABI or explicit.

But we need to discuss which semantic is *correct*:

If Base is aligned to Align and Offset is a multiple of Align (checked below), then the resulting pointer should still be aligned.

For this to have been safe in the old implementation, alignof(S) >= alignof(T) must hold, otherwise an aligned Base might not result in an aligned field.

However, if alignof(S) > alignof(T), and no explicit alignment was provided, the old implementation was overly conservative itself and required that Base be aligned to alignof(S) even though alignof(T) would suffice.

3308

@dblaikie (via email)
So was this check unnecessary all along? Or are callers doing an equivalent check now?

All callers I've encountered would now trip an assert were this case to occur, so at least it wasn't tested against.
It might be interesting to see which callers could actually end up with a non-sized type.

lib/Transforms/InstCombine/InstCombineCalls.cpp

1779

@dblaikie (via email)
If it's obviously i8* in the code, that's OK (could you tell me where to look - even if it's a few lines nearby, that'd be helpful), might be worth a comment or assertion.

Ah, I remember now, the check here is actually that if isDereferenceablePointer returns true, and we have an Argument, the information must come from a dereferenceable attribute.

I suppose it might be better to just skip isDereferenceablePointer and just copy the dereferenceable attribute.

This intrinsic (experimental.gc.relocate) is one of the ones which happen to keep all of their pointee information behind a pointer type and will need to be redesigned (slightly).

include/llvm/Analysis/ValueTracking.h

namespace llvm {
   /// onlyUsedByLifetimeMarkers - Return true if the only users of this pointer
   /// are lifetime markers.
   bool onlyUsedByLifetimeMarkers(const Value *V);
 
   /// isDereferenceablePointer - Return true if this is always a dereferenceable
   /// pointer. If the context instruction is specified perform context-sensitive
   /// analysis and return true if the pointer is dereferenceable at the
   /// specified instruction.
-  bool isDereferenceablePointer(const Value *V, const DataLayout &DL,
+  bool isDereferenceablePointer(const Value *V, uint64_t Size,
+                                const DataLayout &DL,
                                 const Instruction *CtxI = nullptr,
                                 const DominatorTree *DT = nullptr,
                                 const TargetLibraryInfo *TLI = nullptr);
 
   /// Returns true if V is always a dereferenceable pointer with alignment
   /// greater or equal than requested. If the context instruction is specified
   /// performs context-sensitive analysis and returns true if the pointer is
   /// dereferenceable at the specified instruction.
-  bool isDereferenceableAndAlignedPointer(const Value *V, unsigned Align,
-                                          const DataLayout &DL,
+  bool isDereferenceableAndAlignedPointer(const Value *V, uint64_t Size,
+                                          unsigned Align, const DataLayout &DL,
                                           const Instruction *CtxI = nullptr,
                                           const DominatorTree *DT = nullptr,
                                           const TargetLibraryInfo *TLI = nullptr);
 
   /// isSafeToSpeculativelyExecute - Return true if the instruction does not
   /// have any effects besides calculating the result and does not have
   /// undefined behavior.
   ///

lib/Analysis/Loads.cpp

 /// if the address is already accessed.
 ///
 /// This uses the pointee type to determine how many bytes need to be safe to
 /// load from the pointer.
 bool llvm::isSafeToLoadUnconditionally(Value *V, unsigned Align,
                                        Instruction *ScanFrom) {
   const DataLayout &DL = ScanFrom->getModule()->getDataLayout();
 
+  Type *ElemTy = V->getType()->getPointerElementType();
+  uint64_t Size = DL.getTypeStoreSize(ElemTy);
+
   // Zero alignment means that the load has the ABI alignment for the target
   if (Align == 0)
-    Align = DL.getABITypeAlignment(V->getType()->getPointerElementType());
+    Align = DL.getABITypeAlignment(ElemTy);
   assert(isPowerOf2_32(Align));
 
-  if (isDereferenceableAndAlignedPointer(V, Align, DL))
+  if (isDereferenceableAndAlignedPointer(V, Size, Align, DL))
     return true;
 
   int64_t ByteOffset = 0;
   Value *Base = V;
   Base = GetPointerBaseWithConstantOffset(V, ByteOffset, DL);
 
   if (ByteOffset < 0) // out of bounds
     return false;

lib/Analysis/MemDerefPrinter.cpp

FunctionPass *llvm::createMemDerefPrinter() {
   return new MemDerefPrinter();
 }
 
 bool MemDerefPrinter::runOnFunction(Function &F) {
   const DataLayout &DL = F.getParent()->getDataLayout();
   for (auto &I: instructions(F)) {
     if (LoadInst *LI = dyn_cast<LoadInst>(&I)) {
       Value *PO = LI->getPointerOperand();
-      if (isDereferenceablePointer(PO, DL))
+      uint64_t Size = DL.getTypeStoreSize(LI->getType());
+      unsigned Align = LI->getAlignment();
+      if (Align == 0)
+        Align = DL.getABITypeAlignment(LI->getType());
+      if (isDereferenceablePointer(PO, Size, DL))
         Deref.push_back(PO);
-      if (isDereferenceableAndAlignedPointer(PO, LI->getAlignment(), DL))
+      if (isDereferenceableAndAlignedPointer(PO, Size, Align, DL))
         DerefAndAligned.insert(PO);
     }
   }
   return false;
 }
 
 void MemDerefPrinter::print(raw_ostream &OS, const Module *M) const {
   OS << "The following are dereferenceable:\n";

lib/Analysis/ValueTracking.cpp

for (const User *U : V->users()) {
     if (II->getIntrinsicID() != Intrinsic::lifetime_start &&
         II->getIntrinsicID() != Intrinsic::lifetime_end)
       return false;
   }
   return true;
 }
 
 static bool isDereferenceableFromAttribute(const Value *BV, APInt Offset,
-                                           Type *Ty, const DataLayout &DL,
+                                           uint64_t Size,
                                            const Instruction *CtxI,
                                            const DominatorTree *DT,
                                            const TargetLibraryInfo *TLI) {
   assert(Offset.isNonNegative() && "offset can't be negative");
-  assert(Ty->isSized() && "must be sized");
   
   APInt DerefBytes(Offset.getBitWidth(), 0);
   bool CheckForNonNull = false;
   if (const Argument *A = dyn_cast<Argument>(BV)) {
     DerefBytes = A->getDereferenceableBytes();
     if (!DerefBytes.getBoolValue()) {
       DerefBytes = A->getDereferenceableOrNullBytes();
       CheckForNonNull = true;
if (!DerefBytes.getBoolValue()) {
         ConstantInt *CI = mdconst::extract<ConstantInt>(MD->getOperand(0));
         DerefBytes = CI->getLimitedValue();
       }
       CheckForNonNull = true;
     }
   }
   
   if (DerefBytes.getBoolValue())
-    if (DerefBytes.uge(Offset + DL.getTypeStoreSize(Ty)))
+    if (DerefBytes.uge(Offset + Size))
       if (!CheckForNonNull || isKnownNonNullAt(BV, CtxI, DT, TLI))
         return true;
 
   return false;
 }
 
-static bool isDereferenceableFromAttribute(const Value *V, const DataLayout &DL,
-                                           const Instruction *CtxI,
-                                           const DominatorTree *DT,
-                                           const TargetLibraryInfo *TLI) {
-  Type *VTy = V->getType();
-  Type *Ty = VTy->getPointerElementType();
-  if (!Ty->isSized())
-    return false;
-  
-  APInt Offset(DL.getTypeStoreSizeInBits(VTy), 0);
-  return isDereferenceableFromAttribute(V, Offset, Ty, DL, CtxI, DT, TLI);
-}
-
 static bool isAligned(const Value *Base, APInt Offset, unsigned Align,
                       const DataLayout &DL) {
   APInt BaseAlign(Offset.getBitWidth(), getAlignment(Base, DL));
 
   if (!BaseAlign) {
     Type *Ty = Base->getType()->getPointerElementType();
     if (!Ty->isSized())
       return false;
static bool isAligned(const Value *Base, unsigned Align, const DataLayout &DL) {
   assert(Ty->isSized() && "must be sized");
   APInt Offset(DL.getTypeStoreSizeInBits(Ty), 0);
   return isAligned(Base, Offset, Align, DL);
 }
 
 /// Test if V is always a pointer to allocated and suitably aligned memory for
 /// a simple load or store.
 static bool isDereferenceableAndAlignedPointer(
-    const Value *V, unsigned Align, const DataLayout &DL,
-    const Instruction *CtxI, const DominatorTree *DT,
+    const Value *V, uint64_t Size, 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))
     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);
+    if (STy->isSized() && DTy->isSized()) {
+      uint64_t SSize = DL.getTypeStoreSize(STy);
+      uint64_t DSize = DL.getTypeStoreSize(DTy);
+      unsigned SAlign = DL.getABITypeAlignment(STy);
+      unsigned DAlign = DL.getABITypeAlignment(DTy);
+      if (SSize >= DSize && SAlign >= DAlign) {
+        if (Align == DAlign)
+          Align = SAlign;
+        return isDereferenceableAndAlignedPointer(BC->getOperand(0), SSize, 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))
+  Type *VTy = V->getType();
+  APInt Offset(DL.getTypeStoreSizeInBits(VTy), 0);
+  if(isDereferenceableFromAttribute(V, Offset, Size, 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();
 
     // Conservatively require that the base pointer be fully dereferenceable
     // and aligned.
     if (!Visited.insert(Base).second)
       return false;
-    if (!isDereferenceableAndAlignedPointer(Base, Align, DL, CtxI, DT, TLI,
-                                            Visited))
+
+    Type *BaseType = GEP->getSourceElementType();
+    uint64_t BaseSize = DL.getTypeAllocSize(BaseType);
+    unsigned BaseAlign = DL.getABITypeAlignment(BaseType);
+    if (BaseAlign < Align)
+      BaseAlign = Align;
+    if (!isDereferenceableAndAlignedPointer(Base, BaseSize, BaseAlign,
+                                            DL, CtxI, DT, TLI, Visited))
       return false;
 
-    APInt Offset(DL.getPointerTypeSizeInBits(GEP->getType()), 0);
     if (!GEP->accumulateConstantOffset(DL, Offset))
       return false;
 
     // Check if the load is within the bounds of the underlying object
     // and offset is aligned.
-    uint64_t LoadSize = DL.getTypeStoreSize(Ty);
-    Type *BaseType = GEP->getSourceElementType();
     assert(isPowerOf2_32(Align) && "must be a power of 2!");
-    return (Offset + LoadSize).ule(DL.getTypeAllocSize(BaseType)) && 
+    return (Offset + Size).ule(BaseSize) &&
            !(Offset & APInt(Offset.getBitWidth(), Align-1));
   }
 
   // For gc.relocate, look through relocations
   if (const GCRelocateInst *RelocateInst = dyn_cast<GCRelocateInst>(V))
     return isDereferenceableAndAlignedPointer(
-        RelocateInst->getDerivedPtr(), Align, DL, CtxI, DT, TLI, Visited);
+        RelocateInst->getDerivedPtr(), Size, Align,
+        DL, CtxI, DT, TLI, Visited);
 
   if (const AddrSpaceCastInst *ASC = dyn_cast<AddrSpaceCastInst>(V))
-    return isDereferenceableAndAlignedPointer(ASC->getOperand(0), Align, DL,
-                                              CtxI, DT, TLI, Visited);
+    return isDereferenceableAndAlignedPointer(ASC->getOperand(0), Size, Align,
+                                              DL, CtxI, DT, TLI, Visited);
 
   // If we don't know, assume the worst.
   return false;
 }
 
-bool llvm::isDereferenceableAndAlignedPointer(const Value *V, unsigned Align,
+bool llvm::isDereferenceableAndAlignedPointer(const Value *V, uint64_t Size,
+                                              unsigned Align,
                                               const DataLayout &DL,
                                               const Instruction *CtxI,
                                               const DominatorTree *DT,
                                               const TargetLibraryInfo *TLI) {
   // When dereferenceability information is provided by a dereferenceable
   // attribute, we know exactly how many bytes are dereferenceable. If we can
   // determine the exact offset to the attributed variable, we can use that
   // information here.
-  Type *VTy = V->getType();
-  Type *Ty = VTy->getPointerElementType();
-
-  // Require ABI alignment for loads without alignment specification
-  if (Align == 0)
-    Align = DL.getABITypeAlignment(Ty);
-
-  if (Ty->isSized()) {
-    APInt Offset(DL.getTypeStoreSizeInBits(VTy), 0);
-    const Value *BV = V->stripAndAccumulateInBoundsConstantOffsets(DL, Offset);
+  APInt Offset(DL.getTypeStoreSizeInBits(V->getType()), 0);
 
-    if (Offset.isNonNegative())
-      if (isDereferenceableFromAttribute(BV, Offset, Ty, DL, CtxI, DT, TLI) &&
-          isAligned(BV, Offset, Align, DL))
-        return true;
-  }
+    if (isDereferenceableFromAttribute(BV, Offset, Size, CtxI, DT, TLI) &&
 
   SmallPtrSet<const Value *, 32> Visited;
-  return ::isDereferenceableAndAlignedPointer(V, Align, DL, CtxI, DT, TLI,
-                                              Visited);
+  return ::isDereferenceableAndAlignedPointer(V, Size, Align, DL,
+                                              CtxI, DT, TLI, Visited);
 }
 
-bool llvm::isDereferenceablePointer(const Value *V, const DataLayout &DL,
+bool llvm::isDereferenceablePointer(const Value *V, uint64_t Size,
+                                    const DataLayout &DL,
                                     const Instruction *CtxI,
                                     const DominatorTree *DT,
                                     const TargetLibraryInfo *TLI) {
-  return isDereferenceableAndAlignedPointer(V, 1, DL, CtxI, DT, TLI);
+  return isDereferenceableAndAlignedPointer(V, Size, 1, DL, CtxI, DT, TLI);
 }
 
 bool llvm::isSafeToSpeculativelyExecute(const Value *V,
                                         const Instruction *CtxI,
                                         const DominatorTree *DT,
                                         const TargetLibraryInfo *TLI) {
   const Operator *Inst = dyn_cast<Operator>(V);
   if (!Inst)
if (!LI->isUnordered() ||
         // Speculative load may create a race that did not exist in the source.
         LI->getParent()->getParent()->hasFnAttribute(
             Attribute::SanitizeThread) ||
         // Speculative load may load data from dirty regions.
         LI->getParent()->getParent()->hasFnAttribute(
             Attribute::SanitizeAddress))
       return false;
     const DataLayout &DL = LI->getModule()->getDataLayout();
+    uint64_t Size = DL.getTypeStoreSize(LI->getType());
+    unsigned Align = LI->getAlignment();
+    if (Align == 0)
+      Align = DL.getABITypeAlignment(LI->getType());
     return isDereferenceableAndAlignedPointer(
-        LI->getPointerOperand(), LI->getAlignment(), DL, CtxI, DT, TLI);
+        LI->getPointerOperand(), Size, Align, DL, CtxI, DT, TLI);
   }
   case Instruction::Call: {
     if (const IntrinsicInst *II = dyn_cast<IntrinsicInst>(Inst)) {
       switch (II->getIntrinsicID()) {
       // These synthetic intrinsics have no side-effects and just mark
       // information about their operands.
       // FIXME: There are other no-op synthetic instructions that potentially
       // should be considered at least *safe* to speculate...

lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

 void SelectionDAGBuilder::visitLoad(const LoadInst &I) {
   if (I.isAtomic())
     return visitAtomicLoad(I);
 
   const Value *SV = I.getOperand(0);
   SDValue Ptr = getValue(SV);
 
   Type *Ty = I.getType();
+  uint64_t Size = DAG.getDataLayout().getTypeStoreSize(Ty);
 
   bool isVolatile = I.isVolatile();
   bool isNonTemporal = I.getMetadata(LLVMContext::MD_nontemporal) != nullptr;
 
   // The IR notion of invariant_load only guarantees that all *non-faulting*
   // invariant loads result in the same value.  The MI notion of invariant load
   // guarantees that the load can be legally moved to any location within its
   // containing function.  The MI notion of invariant_load is stronger than the
   // IR notion of invariant_load -- an MI invariant_load is an IR invariant_load
   // with a guarantee that the location being loaded from is dereferenceable
   // throughout the function's lifetime.
 
   bool isInvariant = I.getMetadata(LLVMContext::MD_invariant_load) != nullptr &&
-                     isDereferenceablePointer(SV, DAG.getDataLayout());
+                     isDereferenceablePointer(SV, Size, DAG.getDataLayout());
   unsigned Alignment = I.getAlignment();
 
   AAMDNodes AAInfo;
   I.getAAMetadata(AAInfo);
   const MDNode *Ranges = I.getMetadata(LLVMContext::MD_range);
 
   const TargetLowering &TLI = DAG.getTargetLoweringInfo();
   SmallVector<EVT, 4> ValueVTs;
   SmallVector<uint64_t, 4> Offsets;
   ComputeValueVTs(TLI, DAG.getDataLayout(), Ty, ValueVTs, &Offsets);
   unsigned NumValues = ValueVTs.size();
   if (NumValues == 0)
     return;
 
   SDValue Root;
   bool ConstantMemory = false;
   if (isVolatile || NumValues > MaxParallelChains)
     // Serialize volatile loads with other side effects.
     Root = getRoot();
-  else if (AA->pointsToConstantMemory(MemoryLocation(
-               SV, DAG.getDataLayout().getTypeStoreSize(Ty), AAInfo))) {
+  else if (AA->pointsToConstantMemory(MemoryLocation(SV, Size, AAInfo))) {
     // Do not serialize (non-volatile) loads of constant memory with anything.
     Root = DAG.getEntryNode();
     ConstantMemory = true;
   } else {
     // Do not serialize non-volatile loads against each other.
     Root = DAG.getRoot();
   }
 

lib/Transforms/IPO/ArgumentPromotion.cpp

struct ArgPromotion : public CallGraphSCCPass {
 
     /// A vector used to hold the indices of a single GEP instruction
     typedef std::vector<uint64_t> IndicesVector;
 
   private:
     bool isDenselyPacked(Type *type, const DataLayout &DL);
     bool canPaddingBeAccessed(Argument *Arg);
     CallGraphNode *PromoteArguments(CallGraphNode *CGN);
-    bool isSafeToPromoteArgument(Argument *Arg, bool isByVal,
+    bool isSafeToPromoteArgument(Argument *Arg, Type *ElemTy, bool isByVal,
                                  AAResults &AAR) const;
     CallGraphNode *DoPromotion(Function *F,
                               SmallPtrSetImpl<Argument*> &ArgsToPromote,
                               SmallPtrSetImpl<Argument*> &ByValArgsToTransform);
     
     using llvm::Pass::doInitialization;
     bool doInitialization(CallGraph &CG) override;
     /// The maximum number of elements to expand, or 0 for unlimited.
if (isSelfRecursive) {
           }
         }
         if (RecursiveType)
           continue;
       }
     }
     
     // Otherwise, see if we can promote the pointer to its value.
-    if (isSafeToPromoteArgument(PtrArg, PtrArg->hasByValOrInAllocaAttr(), AAR))
+    if (isSafeToPromoteArgument(PtrArg, AgTy, PtrArg->hasByValOrInAllocaAttr(), AAR))
       ArgsToPromote.insert(PtrArg);
   }
 
   // No promotable pointer arguments.
   if (ArgsToPromote.empty() && ByValArgsToTransform.empty()) 
     return nullptr;
 
   return DoPromotion(F, ArgsToPromote, ByValArgsToTransform);
 }
 
 /// AllCallersPassInValidPointerForArgument - Return true if we can prove that
 /// all callees pass in a valid pointer for the specified function argument.
-static bool AllCallersPassInValidPointerForArgument(Argument *Arg) {
+static bool AllCallersPassInValidPointerForArgument(Argument *Arg, Type *ElemTy) {
   Function *Callee = Arg->getParent();
   const DataLayout &DL = Callee->getParent()->getDataLayout();
 
   unsigned ArgNo = Arg->getArgNo();
+  uint64_t Size = DL.getTypeStoreSize(ElemTy);
 
   // Look at all call sites of the function.  At this pointer we know we only
   // have direct callees.
   for (User *U : Callee->users()) {
     CallSite CS(U);
     assert(CS && "Should only have direct calls!");
 
-    if (!isDereferenceablePointer(CS.getArgument(ArgNo), DL))
+    if (!isDereferenceablePointer(CS.getArgument(ArgNo), Size, DL))
       return false;
   }
   return true;
 }
 
 /// Returns true if Prefix is a prefix of longer. That means, Longer has a size
 /// that is greater than or equal to the size of prefix, and each of the
 /// elements in Prefix is the same as the corresponding elements in Longer.
static void MarkIndicesSafe(const ArgPromotion::IndicesVector &ToMark,
   }
 }
 
 /// isSafeToPromoteArgument - As you might guess from the name of this method,
 /// it checks to see if it is both safe and useful to promote the argument.
 /// This method limits promotion of aggregates to only promote up to three
 /// elements of the aggregate in order to avoid exploding the number of
 /// arguments passed in.
-bool ArgPromotion::isSafeToPromoteArgument(Argument *Arg,
+bool ArgPromotion::isSafeToPromoteArgument(Argument *Arg, Type *ElemTy,
                                            bool isByValOrInAlloca,
                                            AAResults &AAR) const {
   typedef std::set<IndicesVector> GEPIndicesSet;
 
   // Quick exit for unused arguments
   if (Arg->use_empty())
     return true;
 
bool ArgPromotion::isSafeToPromoteArgument(Argument *Arg, Type *ElemTy,
   // that the address isn't captured.
   GEPIndicesSet SafeToUnconditionallyLoad;
 
   // This set contains all the sets of indices that we are planning to promote.
   // This makes it possible to limit the number of arguments added.
   GEPIndicesSet ToPromote;
 
   // If the pointer is always valid, any load with first index 0 is valid.
-  if (isByValOrInAlloca || AllCallersPassInValidPointerForArgument(Arg))
+  if (isByValOrInAlloca || AllCallersPassInValidPointerForArgument(Arg, ElemTy))
     SafeToUnconditionallyLoad.insert(IndicesVector(1, 0));
 
   // First, iterate the entry block and mark loads of (geps of) arguments as
   // safe.
   BasicBlock &EntryBlock = Arg->getParent()->front();
   // Declare this here so we can reuse it
   IndicesVector Indices;
   for (Instruction &I : EntryBlock)
for (Use &U : Arg->uses()) {
     } else if (GetElementPtrInst *GEP = dyn_cast<GetElementPtrInst>(UR)) {
       if (GEP->use_empty()) {
         // Dead GEP's cause trouble later.  Just remove them if we run into
         // them.
         GEP->eraseFromParent();
         // TODO: This runs the above loop over and over again for dead GEPs
         // Couldn't we just do increment the UI iterator earlier and erase the
         // use?
-        return isSafeToPromoteArgument(Arg, isByValOrInAlloca, AAR);
+        return isSafeToPromoteArgument(Arg, ElemTy, isByValOrInAlloca, AAR);
       }
 
       // Ensure that all of the indices are constants.
       for (User::op_iterator i = GEP->idx_begin(), e = GEP->idx_end();
         i != e; ++i)
         if (ConstantInt *C = dyn_cast<ConstantInt>(*i))
           Operands.push_back(C->getSExtValue());
         else

lib/Transforms/InstCombine/InstCombineCalls.cpp

if (isa<ConstantPointerNull>(DerivedPtr)) {
       return ReplaceInstUsesWith(*II, ConstantPointerNull::get(GCRelocateType));
     }
 
     // isKnownNonNull -> nonnull attribute
     if (isKnownNonNullAt(DerivedPtr, II, DT, TLI))
       II->addAttribute(AttributeSet::ReturnIndex, Attribute::NonNull);
 
     // isDereferenceablePointer -> deref attribute
-    if (isDereferenceablePointer(DerivedPtr, DL)) {
+    if (isDereferenceablePointer(DerivedPtr, 1, DL)) {
       if (Argument *A = dyn_cast<Argument>(DerivedPtr)) {
         uint64_t Bytes = A->getDereferenceableBytes();
         II->addDereferenceableAttr(AttributeSet::ReturnIndex, Bytes);
       }
     }
 
     // TODO: bitcast(relocate(p)) -> relocate(bitcast(p))
     // Canonicalize on the type from the uses to the defs