[llvm] r367443 - [AMDGPU] Fix for vectorizer crash with pointers of different size

[Benjamin Kramer via llvm-commits]

On Wed, Jul 31, 2019 at 6:32 PM Stanislav Mekhanoshin via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> Author: rampitec
> Date: Wed Jul 31 09:33:11 2019
> New Revision: 367443
>
> URL: http://llvm.org/viewvc/llvm-project?rev=367443&view=rev
> Log:
> [AMDGPU] Fix for vectorizer crash with pointers of different size
>
> When vectorizer strips pointers it can eventually end up with
> pointers of two different sizes, then SCEV will crash.
>
> Differential Revision: https://reviews.llvm.org/D65480
>
> Added:
>
> llvm/trunk/test/Transforms/LoadStoreVectorizer/AMDGPU/vect-ptr-ptr-size-mismatch.ll
> Modified:
>     llvm/trunk/lib/Transforms/Vectorize/LoadStoreVectorizer.cpp
>
> Modified: llvm/trunk/lib/Transforms/Vectorize/LoadStoreVectorizer.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Vectorize/LoadStoreVectorizer.cpp?rev=367443&r1=367442&r2=367443&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Vectorize/LoadStoreVectorizer.cpp (original)
> +++ llvm/trunk/lib/Transforms/Vectorize/LoadStoreVectorizer.cpp Wed Jul 31
> 09:33:11 2019
> @@ -339,11 +339,16 @@ bool Vectorizer::areConsecutivePointers(
>                                          const APInt &PtrDelta,
>                                          unsigned Depth) const {
>    unsigned PtrBitWidth = DL.getPointerTypeSizeInBits(PtrA->getType());
> +  unsigned PtrAS = PtrA->getType()->getPointerAddressSpace();
>    APInt OffsetA(PtrBitWidth, 0);
>    APInt OffsetB(PtrBitWidth, 0);
>    PtrA = PtrA->stripAndAccumulateInBoundsConstantOffsets(DL, OffsetA);
>    PtrB = PtrB->stripAndAccumulateInBoundsConstantOffsets(DL, OffsetB);
>
> +  if (PtrA->getType()->getPointerAddressSpace() != PtrAS ||
> +      PtrB->getType()->getPointerAddressSpace() != PtrAS)
> +    return false;
>

This is too aggressive, it will prevent any pointer pair that's
addrspacecast'ed from ever being recognized as consecutive. What's the
reason for not checking the size? I'm seeing regressions where pointers are
getting loaded from memory and then casted to the right addresspace.

- Ben


> +
>    APInt OffsetDelta = OffsetB - OffsetA;
>
>    // Check if they are based on the same pointer. That makes the offsets
>
> Added:
> llvm/trunk/test/Transforms/LoadStoreVectorizer/AMDGPU/vect-ptr-ptr-size-mismatch.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/LoadStoreVectorizer/AMDGPU/vect-ptr-ptr-size-mismatch.ll?rev=367443&view=auto
>
> ==============================================================================
> ---
> llvm/trunk/test/Transforms/LoadStoreVectorizer/AMDGPU/vect-ptr-ptr-size-mismatch.ll
> (added)
> +++
> llvm/trunk/test/Transforms/LoadStoreVectorizer/AMDGPU/vect-ptr-ptr-size-mismatch.ll
> Wed Jul 31 09:33:11 2019
> @@ -0,0 +1,18 @@
> +; RUN: opt -mtriple=amdgcn-amd-amdhsa -load-store-vectorizer -S < %s |
> FileCheck %s
> +
> +target datalayout =
> "e-p:64:64-p1:64:64-p2:32:32-p3:32:32-p4:64:64-p5:32:32"
> +
> +; CHECK-LABEL: @test
> +; CHECK: store i32* undef, i32** %tmp9, align 8
> +; CHECK: store i32* undef, i32** %tmp7, align 8
> +define amdgpu_kernel void @test() {
> +entry:
> +  %a10.ascast.i = addrspacecast i32* addrspace(5)* null to i32**
> +  %tmp4 = icmp eq i32 undef, 0
> +  %tmp6 = select i1 false, i32** undef, i32** undef
> +  %tmp7 = select i1 %tmp4, i32** null, i32** %tmp6
> +  %tmp9 = select i1 %tmp4, i32** %a10.ascast.i, i32** null
> +  store i32* undef, i32** %tmp9, align 8
> +  store i32* undef, i32** %tmp7, align 8
> +  unreachable
> +}
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190801/6ed60de1/attachment.html>