[llvm] r320953 - Re-commit "Properly handle multi-element and dynamically sized allocas in getPointerDereferenceableBytes()""

[Friedman, Eli via llvm-commits]

On 12/17/2017 1:20 PM, Bjorn Steinbrink via llvm-commits wrote:
> Author: bsteinbr
> Date: Sun Dec 17 13:20:16 2017
> New Revision: 320953
>
> URL: http://llvm.org/viewvc/llvm-project?rev=320953&view=rev
> Log:
> Re-commit "Properly handle multi-element and dynamically sized allocas in getPointerDereferenceableBytes()""
>
> llvm-clang-x86_64-expensive-checks-win is still broken, so the failure
> seems unrelated.
>
> Modified:
>      llvm/trunk/include/llvm/IR/Value.h
>      llvm/trunk/lib/IR/Value.cpp
>      llvm/trunk/test/Analysis/ValueTracking/memory-dereferenceable.ll
>
> Modified: llvm/trunk/include/llvm/IR/Value.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/Value.h?rev=320953&r1=320952&r2=320953&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/IR/Value.h (original)
> +++ llvm/trunk/include/llvm/IR/Value.h Sun Dec 17 13:20:16 2017
> @@ -570,7 +570,7 @@ public:
>     ///
>     /// 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(const DataLayout &DL,
> +  uint64_t getPointerDereferenceableBytes(const DataLayout &DL,
>                                             bool &CanBeNull) const;
>   
>     /// \brief Returns an alignment of the pointer value.
>
> Modified: llvm/trunk/lib/IR/Value.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Value.cpp?rev=320953&r1=320952&r2=320953&view=diff
> ==============================================================================
> --- llvm/trunk/lib/IR/Value.cpp (original)
> +++ llvm/trunk/lib/IR/Value.cpp Sun Dec 17 13:20:16 2017
> @@ -619,11 +619,11 @@ const Value *Value::stripInBoundsOffsets
>     return stripPointerCastsAndOffsets<PSK_InBounds>(this);
>   }
>   
> -unsigned Value::getPointerDereferenceableBytes(const DataLayout &DL,
> +uint64_t Value::getPointerDereferenceableBytes(const DataLayout &DL,
>                                                  bool &CanBeNull) const {
>     assert(getType()->isPointerTy() && "must be pointer");
>   
> -  unsigned DerefBytes = 0;
> +  uint64_t DerefBytes = 0;
>     CanBeNull = false;
>     if (const Argument *A = dyn_cast<Argument>(this)) {
>       DerefBytes = A->getDereferenceableBytes();
> @@ -655,8 +655,10 @@ unsigned Value::getPointerDereferenceabl
>         CanBeNull = true;
>       }
>     } else if (auto *AI = dyn_cast<AllocaInst>(this)) {
> -    if (AI->getAllocatedType()->isSized()) {
> -      DerefBytes = DL.getTypeStoreSize(AI->getAllocatedType());
> +    const ConstantInt *ArraySize = dyn_cast<ConstantInt>(AI->getArraySize());
> +    if (ArraySize && AI->getAllocatedType()->isSized()) {
> +      DerefBytes = DL.getTypeStoreSize(AI->getAllocatedType()) *
> +        ArraySize->getZExtValue();

The getZExtValue() call could crash (the ArraySize can be any integer, 
including ones which don't fit into a uint64_t).

I'd suggest simplifying the code by just ignoring allocas where 
isArrayAllocation() returns true. (instcombine will transform array 
allocations with a constant size to allocations of array type, so you 
don't really lose any optimization power).

-Eli

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project