[llvm] r229029 - [IC] Fix a bug with the instcombine canonicalizing of loads and
Chandler Carruth
chandlerc at gmail.com
Thu Feb 12 18:41:51 PST 2015
I already mentioned this to Hans, but for completeness, this also needs to
go into 3.6.
On Thu, Feb 12, 2015 at 6:30 PM, Chandler Carruth <chandlerc at gmail.com>
wrote:
> Author: chandlerc
> Date: Thu Feb 12 20:30:01 2015
> New Revision: 229029
>
> URL: http://llvm.org/viewvc/llvm-project?rev=229029&view=rev
> Log:
> [IC] Fix a bug with the instcombine canonicalizing of loads and
> propagating of metadata.
>
> We were propagating !nonnull metadata even when the newly formed load is
> no longer of a pointer type. This is clearly broken and results in LLVM
> failing the verifier and aborting. This patch just restricts the
> propagation of !nonnull metadata to when we actually have a pointer
> type.
>
> This bug report and the initial version of this patch was provided by
> Charles Davis! Many thanks for finding this!
>
> We still need to add logic to round-trip the metadata correctly if we
> combine from pointer types to integer types and then back by using range
> metadata for the integer type loads. But this is the minimal and safe
> version of the patch, which is important so we can backport it into 3.6.
>
> Modified:
> llvm/trunk/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
> llvm/trunk/test/Transforms/InstCombine/loadstore-metadata.ll
>
> Modified:
> llvm/trunk/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp?rev=229029&r1=229028&r2=229029&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
> (original)
> +++ llvm/trunk/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
> Thu Feb 12 20:30:01 2015
> @@ -330,11 +330,17 @@ static LoadInst *combineLoadToNewType(In
> case LLVMContext::MD_noalias:
> case LLVMContext::MD_nontemporal:
> case LLVMContext::MD_mem_parallel_loop_access:
> - case LLVMContext::MD_nonnull:
> // All of these directly apply.
> NewLoad->setMetadata(ID, N);
> break;
>
> + case LLVMContext::MD_nonnull:
> + // FIXME: We should translate this into range metadata for integer
> types
> + // and vice versa.
> + if (NewTy->isPointerTy())
> + NewLoad->setMetadata(ID, N);
> + break;
> +
> case LLVMContext::MD_range:
> // FIXME: It would be nice to propagate this in some way, but the
> type
> // conversions make it hard.
> @@ -377,13 +383,14 @@ static StoreInst *combineStoreToNewValue
> case LLVMContext::MD_noalias:
> case LLVMContext::MD_nontemporal:
> case LLVMContext::MD_mem_parallel_loop_access:
> - case LLVMContext::MD_nonnull:
> // All of these directly apply.
> NewStore->setMetadata(ID, N);
> break;
>
> case LLVMContext::MD_invariant_load:
> + case LLVMContext::MD_nonnull:
> case LLVMContext::MD_range:
> + // These don't apply for stores.
> break;
> }
> }
>
> Modified: llvm/trunk/test/Transforms/InstCombine/loadstore-metadata.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/loadstore-metadata.ll?rev=229029&r1=229028&r2=229029&view=diff
>
> ==============================================================================
> --- llvm/trunk/test/Transforms/InstCombine/loadstore-metadata.ll (original)
> +++ llvm/trunk/test/Transforms/InstCombine/loadstore-metadata.ll Thu Feb
> 12 20:30:01 2015
> @@ -1,5 +1,7 @@
> ; RUN: opt -instcombine -S < %s | FileCheck %s
>
> +target datalayout = "e-m:e-p:64:64:64-i64:64-f80:128-n8:16:32:64-S128"
> +
> define i32 @test_load_cast_combine_tbaa(float* %ptr) {
> ; Ensure (cast (load (...))) -> (load (cast (...))) preserves TBAA.
> ; CHECK-LABEL: @test_load_cast_combine_tbaa(
> @@ -78,6 +80,23 @@ exit:
> ret void
> }
>
> +define void @test_load_cast_combine_nonnull(float** %ptr) {
> +; We can't preserve nonnull metadata when converting a load of a pointer
> to
> +; a load of an integer.
> +; FIXME: We should really transform this into range metadata and vice
> versa.
> +;
> +; CHECK-LABEL: @test_load_cast_combine_nonnull(
> +; CHECK: %[[V:.*]] = load i64* %{{.*}}
> +; CHECK-NOT: !nonnull
> +; CHECK: store i64 %[[V]], i64*
> +entry:
> + %p = load float** %ptr, !nonnull !3
> + %gep = getelementptr float** %ptr, i32 42
> + store float* %p, float** %gep
> +
> + ret void
> +}
> +
> !0 = !{ !1, !1, i64 0 }
> !1 = !{ !1 }
> !2 = !{ !2, !1 }
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150212/dad19e06/attachment.html>
More information about the llvm-commits
mailing list