[llvm] r226781 - [canonicalize] Teach InstCombine to canonicalize loads which are only
Duncan P. N. Exon Smith
dexonsmith at apple.com
Mon Feb 9 12:06:48 PST 2015
FYI, just filed PR22524. Looks like this commit exposes a problem
in DAGCombiner.
> On 2015-Jan-21, at 21:08, Chandler Carruth <chandlerc at gmail.com> wrote:
>
> Author: chandlerc
> Date: Wed Jan 21 23:08:12 2015
> New Revision: 226781
>
> URL: http://llvm.org/viewvc/llvm-project?rev=226781&view=rev
> Log:
> [canonicalize] Teach InstCombine to canonicalize loads which are only
> ever stored to always use a legal integer type if one is available.
>
> Regardless of whether this particular type is good or bad, it ensures we
> don't get weird differences in generated code (and resulting
> performance) from "equivalent" patterns that happen to end up using
> a slightly different type.
>
> After some discussion on llvmdev it seems everyone generally likes this
> canonicalization. However, there may be some parts of LLVM that handle
> it poorly and need to be fixed. I have at least verified that this
> doesn't impede GVN and instcombine's store-to-load forwarding powers in
> any obvious cases. Subtle cases are exactly what we need te flush out if
> they remain.
>
> Also note that this IR pattern should already be hitting LLVM from Clang
> at least because it is exactly the IR which would be produced if you
> used memcpy to copy a pointer or floating point between memory instead
> of a variable.
>
> Modified:
> llvm/trunk/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
> llvm/trunk/test/Transforms/InstCombine/load.ll
> llvm/trunk/test/Transforms/InstCombine/struct-assign-tbaa.ll
>
> Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp?rev=226781&r1=226780&r2=226781&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp (original)
> +++ llvm/trunk/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp Wed Jan 21 23:08:12 2015
> @@ -417,6 +417,35 @@ static Instruction *combineLoadToOperati
> if (LI.use_empty())
> return nullptr;
>
> + Type *Ty = LI.getType();
> +
> + // Try to canonicalize loads which are only ever stored to operate over
> + // integers instead of any other type. We only do this when the loaded type
> + // is sized and has a size exactly the same as its store size and the store
> + // size is a legal integer type.
> + const DataLayout *DL = IC.getDataLayout();
> + if (!Ty->isIntegerTy() && Ty->isSized() && DL &&
> + DL->isLegalInteger(DL->getTypeStoreSizeInBits(Ty)) &&
> + DL->getTypeStoreSizeInBits(Ty) == DL->getTypeSizeInBits(Ty)) {
> + if (std::all_of(LI.user_begin(), LI.user_end(), [&LI](User *U) {
> + auto *SI = dyn_cast<StoreInst>(U);
> + return SI && SI->getPointerOperand() != &LI;
> + })) {
> + LoadInst *NewLoad = combineLoadToNewType(
> + IC, LI,
> + Type::getIntNTy(LI.getContext(), DL->getTypeStoreSizeInBits(Ty)));
> + // Replace all the stores with stores of the newly loaded value.
> + for (auto UI = LI.user_begin(), UE = LI.user_end(); UI != UE;) {
> + auto *SI = cast<StoreInst>(*UI++);
> + IC.Builder->SetInsertPoint(SI);
> + combineStoreToNewValue(IC, *SI, NewLoad);
> + IC.EraseInstFromFunction(*SI);
> + }
> + assert(LI.use_empty() && "Failed to remove all users of the load!");
> + // Return the old load so the combiner can delete it safely.
> + return &LI;
> + }
> + }
>
> // Fold away bit casts of the loaded value by loading the desired type.
> if (LI.hasOneUse())
>
> Modified: llvm/trunk/test/Transforms/InstCombine/load.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/load.ll?rev=226781&r1=226780&r2=226781&view=diff
> ==============================================================================
> --- llvm/trunk/test/Transforms/InstCombine/load.ll (original)
> +++ llvm/trunk/test/Transforms/InstCombine/load.ll Wed Jan 21 23:08:12 2015
> @@ -2,7 +2,7 @@
>
> ; This test makes sure that these instructions are properly eliminated.
>
> -target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
> +target datalayout = "e-m:e-p:64:64:64-i64:64-f80:128-n8:16:32:64-S128"
>
> @X = constant i32 42 ; <i32*> [#uses=2]
> @X2 = constant i32 47 ; <i32*> [#uses=1]
> @@ -150,3 +150,53 @@ define i8 @test15(i8 %x, i32 %y) {
> %r = load i8* %g.i8
> ret i8 %r
> }
> +
> +define void @test16(i8* %x, i8* %a, i8* %b, i8* %c) {
> +; Check that we canonicalize loads which are only stored to use integer types
> +; when there is a valid integer type.
> +; CHECK-LABEL: @test16(
> +; CHECK: %[[L1:.*]] = load i32*
> +; CHECK-NOT: load
> +; CHECK: store i32 %[[L1]], i32*
> +; CHECK: store i32 %[[L1]], i32*
> +; CHECK-NOT: store
> +; CHECK: %[[L1:.*]] = load i32*
> +; CHECK-NOT: load
> +; CHECK: store i32 %[[L1]], i32*
> +; CHECK: store i32 %[[L1]], i32*
> +; CHECK-NOT: store
> +; CHECK: ret
> +
> +entry:
> + %x.cast = bitcast i8* %x to float*
> + %a.cast = bitcast i8* %a to float*
> + %b.cast = bitcast i8* %b to float*
> + %c.cast = bitcast i8* %c to i32*
> +
> + %x1 = load float* %x.cast
> + store float %x1, float* %a.cast
> + store float %x1, float* %b.cast
> +
> + %x2 = load float* %x.cast
> + store float %x2, float* %b.cast
> + %x2.cast = bitcast float %x2 to i32
> + store i32 %x2.cast, i32* %c.cast
> +
> + ret void
> +}
> +
> +define void @test17(i8** %x, i8 %y) {
> +; Check that in cases similar to @test16 we don't try to rewrite a load when
> +; its only use is a store but it is used as the pointer to that store rather
> +; than the value.
> +;
> +; CHECK-LABEL: @test17(
> +; CHECK: %[[L:.*]] = load i8**
> +; CHECK: store i8 %y, i8* %[[L]]
> +
> +entry:
> + %x.load = load i8** %x
> + store i8 %y, i8* %x.load
> +
> + ret void
> +}
>
> Modified: llvm/trunk/test/Transforms/InstCombine/struct-assign-tbaa.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/struct-assign-tbaa.ll?rev=226781&r1=226780&r2=226781&view=diff
> ==============================================================================
> --- llvm/trunk/test/Transforms/InstCombine/struct-assign-tbaa.ll (original)
> +++ llvm/trunk/test/Transforms/InstCombine/struct-assign-tbaa.ll Wed Jan 21 23:08:12 2015
> @@ -10,8 +10,8 @@ declare void @llvm.memcpy.p0i8.p0i8.i64(
> %struct.test1 = type { float }
>
> ; CHECK: @test
> -; CHECK: %2 = load float* %0, align 4, !tbaa !0
> -; CHECK: store float %2, float* %1, align 4, !tbaa !0
> +; CHECK: %[[LOAD:.*]] = load i32* %{{.*}}, align 4, !tbaa !0
> +; CHECK: store i32 %[[LOAD:.*]], i32* %{{.*}}, align 4, !tbaa !0
> ; CHECK: ret
> define void @test1(%struct.test1* nocapture %a, %struct.test1* nocapture %b) {
> entry:
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list