[llvm] r220165 - Do a better and more complete job of preserving metadata when combining
Chandler Carruth
chandlerc at gmail.com
Sun Oct 19 03:46:47 PDT 2014
Author: chandlerc
Date: Sun Oct 19 05:46:46 2014
New Revision: 220165
URL: http://llvm.org/viewvc/llvm-project?rev=220165&view=rev
Log:
Do a better and more complete job of preserving metadata when combining
loads.
This handles many more cases than just the AA metadata, some of them
suggested by Hal in his review of the AA metadata handling patch. I've
tried to test this behavior where tractable to do so.
I'll point out that I have specifically *not* included a test for
debuginfo because it was going to require 2 or 3 times as much work to
craft some input which would survive the "helpful" stripping of debug
info metadata that doesn't match the desired schema. This is another
good example of why the current state of write-ability for our debug
info metadata is unacceptable. I spent over 30 minutes trying to conjure
some test case that would survive, even copying from other debug info
tests, but it always failed to survive with no explanation of why or how
I might fix it. =[
Added:
llvm/trunk/test/Transforms/InstCombine/loadstore-metadata.ll
Removed:
llvm/trunk/test/Transforms/InstCombine/loadstore-aa-metadata.ll
Modified:
llvm/trunk/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp?rev=220165&r1=220164&r2=220165&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp (original)
+++ llvm/trunk/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp Sun Oct 19 05:46:46 2014
@@ -15,6 +15,7 @@
#include "llvm/ADT/Statistic.h"
#include "llvm/Analysis/Loads.h"
#include "llvm/IR/DataLayout.h"
+#include "llvm/IR/LLVMContext.h"
#include "llvm/IR/IntrinsicInst.h"
#include "llvm/Transforms/Utils/BasicBlockUtils.h"
#include "llvm/Transforms/Utils/Local.h"
@@ -291,6 +292,62 @@ Instruction *InstCombiner::visitAllocaIn
return visitAllocSite(AI);
}
+/// \brief Helper to combine a load to a new type.
+///
+/// This just does the work of combining a load to a new type. It handles
+/// metadata, etc., and returns the new instruction. The \c NewTy should be the
+/// loaded *value* type. This will convert it to a pointer, cast the operand to
+/// that pointer type, load it, etc.
+///
+/// Note that this will create all of the instructions with whatever insert
+/// point the \c InstCombiner currently is using.
+static LoadInst *combineLoadToNewType(InstCombiner &IC, LoadInst &LI, Type *NewTy) {
+ Value *Ptr = LI.getPointerOperand();
+ unsigned AS = LI.getPointerAddressSpace();
+ SmallVector<std::pair<unsigned, MDNode *>, 8> MD;
+ LI.getAllMetadata(MD);
+
+ LoadInst *NewLoad = IC.Builder->CreateAlignedLoad(
+ IC.Builder->CreateBitCast(Ptr, NewTy->getPointerTo(AS)),
+ LI.getAlignment(), LI.getName());
+ for (const auto &MDPair : MD) {
+ unsigned ID = MDPair.first;
+ MDNode *N = MDPair.second;
+ // Note, essentially every kind of metadata should be preserved here! This
+ // routine is supposed to clone a load instruction changing *only its type*.
+ // The only metadata it makes sense to drop is metadata which is invalidated
+ // when the pointer type changes. This should essentially never be the case
+ // in LLVM, but we explicitly switch over only known metadata to be
+ // conservatively correct. If you are adding metadata to LLVM which pertains
+ // to loads, you almost certainly want to add it here.
+ switch (ID) {
+ case LLVMContext::MD_dbg:
+ case LLVMContext::MD_tbaa:
+ case LLVMContext::MD_prof:
+ case LLVMContext::MD_fpmath:
+ case LLVMContext::MD_tbaa_struct:
+ case LLVMContext::MD_invariant_load:
+ case LLVMContext::MD_alias_scope:
+ case LLVMContext::MD_noalias:
+ // All of these directly apply.
+ 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.
+ break;
+ }
+ }
+ // FIXME: These metadata nodes should really have enumerators and be handled
+ // above.
+ if (MDNode *N = LI.getMetadata("nontemporal"))
+ NewLoad->setMetadata("nontemporal", N);
+ if (MDNode *N = LI.getMetadata("llvm.mem.parallel_loop_access"))
+ NewLoad->setMetadata("llvm.mem.parallel_loop_access", N);
+ return NewLoad;
+}
+
/// \brief Combine loads to match the type of value their uses after looking
/// through intervening bitcasts.
///
@@ -317,18 +374,11 @@ static Instruction *combineLoadToOperati
if (LI.use_empty())
return nullptr;
- Value *Ptr = LI.getPointerOperand();
- unsigned AS = LI.getPointerAddressSpace();
- AAMDNodes AAInfo;
- LI.getAAMetadata(AAInfo);
// Fold away bit casts of the loaded value by loading the desired type.
if (LI.hasOneUse())
if (auto *BC = dyn_cast<BitCastInst>(LI.user_back())) {
- LoadInst *NewLoad = IC.Builder->CreateAlignedLoad(
- IC.Builder->CreateBitCast(Ptr, BC->getDestTy()->getPointerTo(AS)),
- LI.getAlignment(), LI.getName());
- NewLoad->setAAMetadata(AAInfo);
+ LoadInst *NewLoad = combineLoadToNewType(IC, LI, BC->getDestTy());
BC->replaceAllUsesWith(NewLoad);
IC.EraseInstFromFunction(*BC);
return &LI;
Removed: llvm/trunk/test/Transforms/InstCombine/loadstore-aa-metadata.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/loadstore-aa-metadata.ll?rev=220164&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/InstCombine/loadstore-aa-metadata.ll (original)
+++ llvm/trunk/test/Transforms/InstCombine/loadstore-aa-metadata.ll (removed)
@@ -1,25 +0,0 @@
-; RUN: opt -instcombine -S < %s | FileCheck %s
-
-define i32 @test_load_cast_combine_tbaa(float* %ptr) {
-; Ensure (cast (load (...))) -> (load (cast (...))) preserves TBAA.
-; CHECK-LABEL: @test_load_cast_combine_tbaa(
-; CHECK: load i32* %{{.*}}, !tbaa !0
-entry:
- %l = load float* %ptr, !tbaa !0
- %c = bitcast float %l to i32
- ret i32 %c
-}
-
-define i32 @test_load_cast_combine_noalias(float* %ptr) {
-; Ensure (cast (load (...))) -> (load (cast (...))) preserves no-alias metadata.
-; CHECK-LABEL: @test_load_cast_combine_noalias(
-; CHECK: load i32* %{{.*}}, !alias.scope !2, !noalias !1
-entry:
- %l = load float* %ptr, !alias.scope !2, !noalias !1
- %c = bitcast float %l to i32
- ret i32 %c
-}
-
-!0 = metadata !{ metadata !1, metadata !1, i64 0 }
-!1 = metadata !{ metadata !1 }
-!2 = metadata !{ metadata !2, metadata !1 }
Added: llvm/trunk/test/Transforms/InstCombine/loadstore-metadata.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/loadstore-metadata.ll?rev=220165&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/InstCombine/loadstore-metadata.ll (added)
+++ llvm/trunk/test/Transforms/InstCombine/loadstore-metadata.ll Sun Oct 19 05:46:46 2014
@@ -0,0 +1,86 @@
+; RUN: opt -instcombine -S < %s | FileCheck %s
+
+define i32 @test_load_cast_combine_tbaa(float* %ptr) {
+; Ensure (cast (load (...))) -> (load (cast (...))) preserves TBAA.
+; CHECK-LABEL: @test_load_cast_combine_tbaa(
+; CHECK: load i32* %{{.*}}, !tbaa !0
+entry:
+ %l = load float* %ptr, !tbaa !0
+ %c = bitcast float %l to i32
+ ret i32 %c
+}
+
+define i32 @test_load_cast_combine_noalias(float* %ptr) {
+; Ensure (cast (load (...))) -> (load (cast (...))) preserves no-alias metadata.
+; CHECK-LABEL: @test_load_cast_combine_noalias(
+; CHECK: load i32* %{{.*}}, !alias.scope !2, !noalias !1
+entry:
+ %l = load float* %ptr, !alias.scope !2, !noalias !1
+ %c = bitcast float %l to i32
+ ret i32 %c
+}
+
+define float @test_load_cast_combine_range(i32* %ptr) {
+; Ensure (cast (load (...))) -> (load (cast (...))) drops range metadata. It
+; would be nice to preserve or update it somehow but this is hard when moving
+; between types.
+; CHECK-LABEL: @test_load_cast_combine_range(
+; CHECK: load float* %{{.*}}
+; CHECK-NOT: !range
+; CHECK: ret float
+entry:
+ %l = load i32* %ptr, !range !5
+ %c = bitcast i32 %l to float
+ ret float %c
+}
+
+define i32 @test_load_cast_combine_invariant(float* %ptr) {
+; Ensure (cast (load (...))) -> (load (cast (...))) preserves invariant metadata.
+; CHECK-LABEL: @test_load_cast_combine_invariant(
+; CHECK: load i32* %{{.*}}, !invariant.load !3
+entry:
+ %l = load float* %ptr, !invariant.load !3
+ %c = bitcast float %l to i32
+ ret i32 %c
+}
+
+define i32 @test_load_cast_combine_nontemporal(float* %ptr) {
+; Ensure (cast (load (...))) -> (load (cast (...))) preserves nontemporal
+; metadata.
+; CHECK-LABEL: @test_load_cast_combine_nontemporal(
+; CHECK: load i32* %{{.*}}, !nontemporal !4
+entry:
+ %l = load float* %ptr, !nontemporal !4
+ %c = bitcast float %l to i32
+ ret i32 %c
+}
+
+define void @test_load_cast_combine_loop(float* %src, i32* %dst, i32 %n) {
+; Ensure (cast (load (...))) -> (load (cast (...))) preserves loop access
+; metadata.
+; CHECK-LABEL: @test_load_cast_combine_loop(
+; CHECK: load i32* %{{.*}}, !llvm.mem.parallel_loop_access !1
+entry:
+ br label %loop
+
+loop:
+ %i = phi i32 [ 0, %entry ], [ %i.next, %loop ]
+ %src.gep = getelementptr inbounds float* %src, i32 %i
+ %dst.gep = getelementptr inbounds i32* %dst, i32 %i
+ %l = load float* %src.gep, !llvm.mem.parallel_loop_access !1
+ %c = bitcast float %l to i32
+ store i32 %c, i32* %dst.gep
+ %i.next = add i32 %i, 1
+ %cmp = icmp slt i32 %i.next, %n
+ br i1 %cmp, label %loop, label %exit, !llvm.loop !1
+
+exit:
+ ret void
+}
+
+!0 = metadata !{ metadata !1, metadata !1, i64 0 }
+!1 = metadata !{ metadata !1 }
+!2 = metadata !{ metadata !2, metadata !1 }
+!3 = metadata !{ }
+!4 = metadata !{ i32 1 }
+!5 = metadata !{ i32 0, i32 42 }
More information about the llvm-commits
mailing list