From 0b37b7bf52b92f9e1e7ffaff4063fd1184d2eec3 Mon Sep 17 00:00:00 2001 From: Hans Wennborg Date: Fri, 13 Feb 2015 03:19:15 +0000 Subject: [PATCH] Merging r229029, minus the test which didn't work on the branch: ------------------------------------------------------------------------ r229029 | chandlerc | 2015-02-12 18:30:01 -0800 (Thu, 12 Feb 2015) | 16 lines [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. ------------------------------------------------------------------------ git-svn-id: https://llvm.org/svn/llvm-project/llvm/branches/release_36@229036 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../InstCombine/InstCombineLoadStoreAlloca.cpp | 11 +++++++++-- test/Transforms/InstCombine/loadstore-metadata.ll | 2 ++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp b/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp index af1694d3453..6230c000cf5 100644 --- a/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp +++ b/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp @@ -330,11 +330,17 @@ static LoadInst *combineLoadToNewType(InstCombiner &IC, LoadInst &LI, Type *NewT 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. @@ -548,13 +554,14 @@ static bool combineStoreToValueType(InstCombiner &IC, StoreInst &SI) { 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; } } diff --git a/test/Transforms/InstCombine/loadstore-metadata.ll b/test/Transforms/InstCombine/loadstore-metadata.ll index ad6a11cf6eb..3d18ac0e334 100644 --- a/test/Transforms/InstCombine/loadstore-metadata.ll +++ b/test/Transforms/InstCombine/loadstore-metadata.ll @@ -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( -- 2.50.1