[llvm] r276358 - GVH-hoist: only clone GEPs (PR28606)

[Hans Wennborg via llvm-commits]

Merged this and the two follow-ups r276364 and r276368 to 3.9 in r276420.

Cheers,
Hans

On Thu, Jul 21, 2016 at 7:22 PM, Sebastian Pop via llvm-commits
<llvm-commits at lists.llvm.org> wrote:
> Author: spop
> Date: Thu Jul 21 18:22:10 2016
> New Revision: 276358
>
> URL: http://llvm.org/viewvc/llvm-project?rev=276358&view=rev
> Log:
> GVH-hoist: only clone GEPs (PR28606)
>
> Do not clone stored values unless they are GEPs that are special cased to avoid
> hoisting them without hoisting their associated ld/st.
>
> Differential revision: https://reviews.llvm.org/D22652
>
> Added:
>     llvm/trunk/test/Transforms/GVN/hoist-pr28606.ll
> Modified:
>     llvm/trunk/lib/Transforms/Scalar/GVNHoist.cpp
>     llvm/trunk/test/Transforms/GVN/pr28626.ll
>
> Modified: llvm/trunk/lib/Transforms/Scalar/GVNHoist.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/GVNHoist.cpp?rev=276358&r1=276357&r2=276358&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Scalar/GVNHoist.cpp (original)
> +++ llvm/trunk/lib/Transforms/Scalar/GVNHoist.cpp Thu Jul 21 18:22:10 2016
> @@ -584,21 +584,14 @@ public:
>      if (auto *St = dyn_cast<StoreInst>(Repl)) {
>        Gep = dyn_cast<GetElementPtrInst>(St->getPointerOperand());
>        Val = dyn_cast<Instruction>(St->getValueOperand());
> +      // Check that the stored value is available.
> +      if (Val && !isa<GetElementPtrInst>(Val) &&
> +          !DT->dominates(Val->getParent(), HoistPt))
> +        return false;
>      }
>
> -    if (!Gep)
> -      return false;
> -
> -    // PHIs may only be inserted at the start of a block.
> -    if (Val && isa<PHINode>(Val))
> -      return false;
> -
>      // Check whether we can compute the Gep at HoistPt.
> -    if (!allOperandsAvailable(Gep, HoistPt))
> -      return false;
> -
> -    // Also check that the stored value is available.
> -    if (Val && !allOperandsAvailable(Val, HoistPt))
> +    if (!Gep || !allOperandsAvailable(Gep, HoistPt))
>        return false;
>
>      // Copy the gep before moving the ld/st.
> @@ -618,8 +611,8 @@ public:
>      }
>      Repl->replaceUsesOfWith(Gep, ClonedGep);
>
> -    // Also copy Val.
> -    if (Val) {
> +    // Also copy Val when it is a GEP.
> +    if (Val && isa<GetElementPtrInst>(Val)) {
>        Instruction *ClonedVal = Val->clone();
>        ClonedVal->insertBefore(HoistPt->getTerminator());
>        // Conservatively discard any optimization hints, they may differ on the
>
> Added: llvm/trunk/test/Transforms/GVN/hoist-pr28606.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/GVN/hoist-pr28606.ll?rev=276358&view=auto
> ==============================================================================
> --- llvm/trunk/test/Transforms/GVN/hoist-pr28606.ll (added)
> +++ llvm/trunk/test/Transforms/GVN/hoist-pr28606.ll Thu Jul 21 18:22:10 2016
> @@ -0,0 +1,50 @@
> +; RUN: opt -gvn-hoist -S < %s | FileCheck %s
> +
> +target datalayout = "e-m:x-p:32:32-i64:64-f80:32-n8:16:32-a:0:32-S32"
> +target triple = "i686-pc-windows-msvc18.0.0"
> +
> +%struct.S = type { i8* }
> +
> +declare void @f(<{ %struct.S }>* inalloca)
> +
> +
> +; Check that we don't clone the %x alloca and insert it in the live range of
> +; %argmem, which would break the inalloca contract.
> +;
> +; CHECK-LABEL: @test
> +; CHECK: alloca i8
> +; CHECK: stacksave
> +; CHECK: alloca inalloca
> +; CHECK-NOT: alloca i8
> +
> +; Check that store instructions are hoisted.
> +; CHECK: store i8
> +; CHECK-NOT: store i8
> +; CHECK: stackrestore
> +
> +define void @test(i1 %b) {
> +entry:
> +  %x = alloca i8
> +  %inalloca.save = call i8* @llvm.stacksave()
> +  %argmem = alloca inalloca <{ %struct.S }>, align 4
> +  %0 = getelementptr inbounds <{ %struct.S }>, <{ %struct.S }>* %argmem, i32 0, i32 0
> +  br i1 %b, label %true, label %false
> +
> +true:
> +  %p = getelementptr inbounds %struct.S, %struct.S* %0, i32 0, i32 0
> +  store i8* %x, i8** %p, align 4
> +  br label %exit
> +
> +false:
> +  %p2 = getelementptr inbounds %struct.S, %struct.S* %0, i32 0, i32 0
> +  store i8* %x, i8** %p2, align 4
> +  br label %exit
> +
> +exit:
> +  call void @f(<{ %struct.S }>* inalloca %argmem)
> +  call void @llvm.stackrestore(i8* %inalloca.save)
> +  ret void
> +}
> +
> +declare i8* @llvm.stacksave()
> +declare void @llvm.stackrestore(i8*)
>
> Modified: llvm/trunk/test/Transforms/GVN/pr28626.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/GVN/pr28626.ll?rev=276358&r1=276357&r2=276358&view=diff
> ==============================================================================
> --- llvm/trunk/test/Transforms/GVN/pr28626.ll (original)
> +++ llvm/trunk/test/Transforms/GVN/pr28626.ll Thu Jul 21 18:22:10 2016
> @@ -38,5 +38,5 @@ if.end6:
>  ; CHECK: %[[gep0:.*]] = getelementptr inbounds i1, i1* %[[load]], i64 0
>  ; CHECK: store i1 %[[phi]], i1* %[[gep0]], align 4
>
> -; CHECK: %[[gep1:.*]] = getelementptr inbounds i1, i1* %[[load]], i64 0
> -; CHECK: store i1 %[[phi]], i1* %[[gep1]], align 4
> +; Check that store instructions are hoisted.
> +; CHECK-NOT: store
> \ No newline at end of file
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits