[llvm] r351325 - Assertion in isAllocaPromotable due to extra bitcast goes into lifetime marker

[Hans Wennborg via llvm-commits]

This landed very soon after the 8.0 branch was created. Should it be
merged over? I don't know anything about this, just saw that it fixes
an assert.

On Wed, Jan 16, 2019 at 1:10 PM Gabor Buella via llvm-commits
<llvm-commits at lists.llvm.org> wrote:
>
> Author: gbuella
> Date: Wed Jan 16 04:06:17 2019
> New Revision: 351325
>
> URL: http://llvm.org/viewvc/llvm-project?rev=351325&view=rev
> Log:
> Assertion in isAllocaPromotable due to extra bitcast goes into lifetime marker
>
> For the given test SROA detects possible replacement and creates a correct alloca. After that SROA is adding lifetime markers for this new alloca. The function getNewAllocaSlicePtr is trying to deduce the pointer type based on the original alloca, which is split, to use it later in lifetime intrinsic.
>
> For the test we ended up with such code (rA is initial alloca [10 x float], which is split, and rA.sroa.0.0 is a new split allocation)
>
> ```
> %rA.sroa.0.0.rA.sroa_cast = bitcast i32* %rA.sroa.0 to [10 x float]*    <----- this one causing the assertion and is an extra bitcast
> %5 = bitcast [10 x float]* %rA.sroa.0.0.rA.sroa_cast to i8*
> call void @llvm.lifetime.start.p0i8(i64 4, i8* %5)
> ```
>
> isAllocaPromotable code assumes that a user of alloca may go into lifetime marker through bitcast but it must be the only one bitcast to i8* type. In the test it's not a i8* type, return false and throw the assertion.
>
> As we are creating a pointer, which will be used in lifetime markers only, the proposed fix is to create a bitcast to i8* immediately to avoid extra bitcast creation.
>
> The test is a greatly simplified to just reproduce the assertion.
>
> Author: Igor Tsimbalist <igor.v.tsimbalist at intel.com>
>
> Reviewers: chandlerc, craig.topper
>
> Reviewed By: chandlerc
>
> Differential Revision: https://reviews.llvm.org/D55934
>
> Modified:
>     llvm/trunk/lib/Transforms/Scalar/SROA.cpp
>     llvm/trunk/test/Transforms/SROA/basictest.ll
>
> Modified: llvm/trunk/lib/Transforms/Scalar/SROA.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/SROA.cpp?rev=351325&r1=351324&r2=351325&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Scalar/SROA.cpp (original)
> +++ llvm/trunk/lib/Transforms/Scalar/SROA.cpp Wed Jan 16 04:06:17 2019
> @@ -3031,7 +3031,10 @@ private:
>      ConstantInt *Size =
>          ConstantInt::get(cast<IntegerType>(II.getArgOperand(0)->getType()),
>                           NewEndOffset - NewBeginOffset);
> -    Value *Ptr = getNewAllocaSlicePtr(IRB, OldPtr->getType());
> +    // Lifetime intrinsics always expect an i8* so directly get such a pointer
> +    // for the new alloca slice.
> +    Type *PointerTy = IRB.getInt8PtrTy(OldPtr->getType()->getPointerAddressSpace());
> +    Value *Ptr = getNewAllocaSlicePtr(IRB, PointerTy);
>      Value *New;
>      if (II.getIntrinsicID() == Intrinsic::lifetime_start)
>        New = IRB.CreateLifetimeStart(Ptr, Size);
>
> Modified: llvm/trunk/test/Transforms/SROA/basictest.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/SROA/basictest.ll?rev=351325&r1=351324&r2=351325&view=diff
> ==============================================================================
> --- llvm/trunk/test/Transforms/SROA/basictest.ll (original)
> +++ llvm/trunk/test/Transforms/SROA/basictest.ll Wed Jan 16 04:06:17 2019
> @@ -1745,6 +1745,55 @@ entry:
>    ret void
>  }
>
> +declare void @llvm.lifetime.start.isVoid.i64.p0i8(i64, [10 x float]* nocapture)
> +declare void @llvm.lifetime.end.isVoid.i64.p0i8(i64, [10 x float]* nocapture)
> + at array = dso_local global [10 x float] undef, align 4
> +
> +define void @test29(i32 %num, i32 %tid) {
> +; CHECK-LABEL: @test29(
> +; CHECK-NOT: alloca [10 x float]
> +; CHECK: ret void
> +
> +entry:
> +  %ra = alloca [10 x float], align 4
> +  call void @llvm.lifetime.start.isVoid.i64.p0i8(i64 40, [10 x float]* nonnull %ra)
> +
> +  %cmp1 = icmp sgt i32 %num, 0
> +  br i1 %cmp1, label %bb1, label %bb7
> +
> +bb1:
> +  %tobool = icmp eq i32 %tid, 0
> +  %conv.i = zext i32 %tid to i64
> +  %0 = bitcast [10 x float]* %ra to i32*
> +  %1 = load i32, i32* %0, align 4
> +  %arrayidx5 = getelementptr inbounds [10 x float], [10 x float]* @array, i64 0, i64 %conv.i
> +  %2 = bitcast float* %arrayidx5 to i32*
> +  br label %bb2
> +
> +bb2:
> +  %i.02 = phi i32 [ %num, %bb1 ], [ %sub, %bb5 ]
> +  br i1 %tobool, label %bb3, label %bb4
> +
> +bb3:
> +  br label %bb5
> +
> +bb4:
> +  store i32 %1, i32* %2, align 4
> +  br label %bb5
> +
> +bb5:
> +  %sub = add i32 %i.02, -1
> +  %cmp = icmp sgt i32 %sub, 0
> +  br i1 %cmp, label %bb2, label %bb6
> +
> +bb6:
> +  br label %bb7
> +
> +bb7:
> +  call void @llvm.lifetime.end.isVoid.i64.p0i8(i64 40, [10 x float]* nonnull %ra)
> +  ret void
> +}
> +
>  !0 = !{!1, !1, i64 0, i64 1}
>  !1 = !{!2, i64 1, !"type_0"}
>  !2 = !{!"root"}
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits