[llvm] cf41616 - [Instcombine] Disable memcpy of alloca bypass for instruction sources

[Roman Lebedev via llvm-commits]

Thank you.
I think we will see similar issues get exposed in other places that
use `isDereferenceableAndAlignedPointer()`.

Roman

On Wed, Apr 14, 2021 at 5:52 PM Benjamin Kramer via llvm-commits
<llvm-commits at lists.llvm.org> wrote:
>
>
> Author: Benjamin Kramer
> Date: 2021-04-14T16:52:09+02:00
> New Revision: cf4161673c7e7c7c57d8115468bfcc9988f43d36
>
> URL: https://github.com/llvm/llvm-project/commit/cf4161673c7e7c7c57d8115468bfcc9988f43d36
> DIFF: https://github.com/llvm/llvm-project/commit/cf4161673c7e7c7c57d8115468bfcc9988f43d36.diff
>
> LOG: [Instcombine] Disable memcpy of alloca bypass for instruction sources
>
> This transformation is fundamentally broken when it comes to dominance,
> it just happened to work when the source of the memcpy can be moved into
> the place of the alloca. The bug shows up a lot more often since
> 077bff39d46364035a5dcfa32fc69910ad0975d0 allows the source to be a
> switch.
>
> It would be possible to check dominance of the source and all its
> operands, but that seems very heavy for instcombine.
>
> Added:
>
>
> Modified:
>     llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
>     llvm/test/Transforms/InstCombine/tmp-alloca-bypass.ll
>
> Removed:
>
>
>
> ################################################################################
> diff  --git a/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp b/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
> index 3ca54958cc79..dd4c15e4cdb1 100644
> --- a/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
> +++ b/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
> @@ -407,10 +407,9 @@ Instruction *InstCombinerImpl::visitAllocaInst(AllocaInst &AI) {
>        TheSrc, AllocaAlign, DL, &AI, &AC, &DT);
>      if (AllocaAlign <= SourceAlign &&
>          isDereferenceableForAllocaSize(TheSrc, &AI, DL) &&
> -        (!isa<Instruction>(TheSrc) || all_of(AI.users(), [&](const User *U) {
> -          return DT.dominates(cast<Instruction>(TheSrc), cast<Instruction>(U));
> -        }))) {
> -      // FIXME: can the dominance restriction be relaxed by sinking instrns?
> +        !isa<Instruction>(TheSrc)) {
> +      // FIXME: Can we sink instructions without violating dominance when TheSrc
> +      // is an instruction instead of a constant or argument?
>        LLVM_DEBUG(dbgs() << "Found alloca equal to global: " << AI << '\n');
>        LLVM_DEBUG(dbgs() << "  memcpy = " << *Copy << '\n');
>        unsigned SrcAddrSpace = TheSrc->getType()->getPointerAddressSpace();
>
> diff  --git a/llvm/test/Transforms/InstCombine/tmp-alloca-bypass.ll b/llvm/test/Transforms/InstCombine/tmp-alloca-bypass.ll
> index 9a87e8a68f8e..7fb531d7fa08 100644
> --- a/llvm/test/Transforms/InstCombine/tmp-alloca-bypass.ll
> +++ b/llvm/test/Transforms/InstCombine/tmp-alloca-bypass.ll
> @@ -31,5 +31,29 @@ define void @test(i8*%out) {
>    ret void
>  }
>
> +define void @test2() {
> +; CHECK-LABEL: @test2(
> +; CHECK-NEXT:  bb:
> +; CHECK-NEXT:    [[I:%.*]] = alloca [[T0:%.*]], align 8
> +; CHECK-NEXT:    [[I1:%.*]] = call i32 @func(%t0* undef)
> +; CHECK-NEXT:    [[I2:%.*]] = icmp eq i32 [[I1]], 2503
> +; CHECK-NEXT:    [[I3:%.*]] = select i1 [[I2]], i8* bitcast (%t0* @g0 to i8*), i8* bitcast (%t0* @g1 to i8*)
> +; CHECK-NEXT:    [[I4:%.*]] = bitcast %t0* [[I]] to i8*
> +; CHECK-NEXT:    call void @llvm.memcpy.p0i8.p0i8.i64(i8* noundef nonnull align 8 dereferenceable(16) [[I4]], i8* noundef nonnull align 8 dereferenceable(16) [[I3]], i64 16, i1 false)
> +; CHECK-NEXT:    [[I5:%.*]] = call i32 @func(%t0* nonnull byval([[T0]]) [[I]])
> +; CHECK-NEXT:    unreachable
> +;
> +bb:
> +  %i = alloca %t0, align 8
> +  %i1 = call i32 @func(%t0* undef)
> +  %i2 = icmp eq i32 %i1, 2503
> +  %i3 = select i1 %i2, i8* bitcast (%t0* @g0 to i8*), i8* bitcast (%t0* @g1 to i8*)
> +  %i4 = bitcast %t0* %i to i8*
> +  call void @llvm.memcpy.p0i8.p0i8.i64(i8* noundef nonnull align 8 dereferenceable(16) %i4, i8* noundef nonnull align 8 dereferenceable(16) %i3, i64 16, i1 false)
> +  %i5 = call i32 @func(%t0* nonnull byval(%t0) %i)
> +  unreachable
> +}
> +
> +declare i32 @func(%t0*)
>  declare i1 @get_cond()
>  declare void @llvm.memcpy.p0i8.p0i8.i64(i8*, i8*, i64, i1)
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits