[llvm] r229029 - [IC] Fix a bug with the instcombine canonicalizing of loads and

[Chandler Carruth]

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>