[PATCH] !invariant.loads with potentially clobbering calls

[Daniel Berlin]

Looks right to me

On Tue, Mar 24, 2015 at 3:07 PM, Philip Reames <listmail at philipreames.com>
wrote:

> Hi dberlin, hfinkel, atrick, nlewycky,
>
> A load from an invariant location is assumed to not alias any otherwise
> potentially aliasing stores.  Our implementation only applied this rule to
> store instructions themselves whereas they it should apply for any memory
> accessing instruction.   This results in both FRE and PRE becoming more
> effective at eliminating invariant loads.
>
> Note that as a follow on change I will likely move this into AliasAnalysis
> itself.  That's where the TBAA constant flag is handled and the semantics
> are essentially the same.  I'd like to separate the semantic change from
> the refactoring and thus have extended the hack that's already in
> MemoryDependenceAnalysis for this change.
>
> http://reviews.llvm.org/D8591
>
> Files:
>   lib/Analysis/MemoryDependenceAnalysis.cpp
>   test/Transforms/GVN/invariant-load.ll
>
> Index: lib/Analysis/MemoryDependenceAnalysis.cpp
> ===================================================================
> --- lib/Analysis/MemoryDependenceAnalysis.cpp
> +++ lib/Analysis/MemoryDependenceAnalysis.cpp
> @@ -408,6 +408,10 @@
>    // it is racy (undefined) or there is a release followed by an acquire
>    // between the pair of accesses under consideration.
>
> +  // If the load is invariant, we "know" that it doesn't alias *any*
> write. We
> +  // do want to respect mustalias results since defs are useful for value
> +  // forwarding, but any mayalias write can be assumed to be noalias.
> +  // Arguably, this logic should be pushed inside AliasAnalysis itself.
>    if (isLoad && QueryInst) {
>      LoadInst *LI = dyn_cast<LoadInst>(QueryInst);
>      if (LI && LI->getMetadata(LLVMContext::MD_invariant_load) != nullptr)
> @@ -601,6 +605,8 @@
>
>        if (AccessPtr == Inst || AA->isMustAlias(Inst, AccessPtr))
>          return MemDepResult::getDef(Inst);
> +      if (isInvariantLoad)
> +        continue;
>        // Be conservative if the accessed pointer may alias the allocation.
>        if (AA->alias(Inst, AccessPtr) != AliasAnalysis::NoAlias)
>          return MemDepResult::getClobber(Inst);
> @@ -611,6 +617,9 @@
>          continue;
>      }
>
> +    if (isInvariantLoad)
> +       continue;
> +
>      // See if this instruction (e.g. a call or vaarg) mod/ref's the
> pointer.
>      AliasAnalysis::ModRefResult MR = AA->getModRefInfo(Inst, MemLoc);
>      // If necessary, perform additional analysis.
> Index: test/Transforms/GVN/invariant-load.ll
> ===================================================================
> --- test/Transforms/GVN/invariant-load.ll
> +++ test/Transforms/GVN/invariant-load.ll
> @@ -65,5 +65,55 @@
>    ret i32 %res
>  }
>
> +; Checks that we return the mustalias store as a def
> +; so that it contributes to value forwarding.  Note
> +; that we could and should remove the store too.
> +define i32 @test5(i1 %cnd, i32* %p) {
> +; CHECK-LABEL: test5
> +; CHECK-LABEL: entry:
> +; CHECK-NEXT: store i32 5, i32* %p
> +; CHECK-NEXT: ret i32 5
> +entry:
> +  %v1 = load i32, i32* %p, !invariant.load !0
> +  store i32 5, i32* %p ;; must alias store, want to exploit
> +  %v2 = load i32, i32* %p, !invariant.load !0
> +  ret i32 %v2
> +}
> +
> +
> +declare void @foo()
> +
> +; Clobbering (mayalias) stores, even in function calls, can be ignored
> +define i32 @test6(i1 %cnd, i32* %p) {
> +; CHECK-LABEL: test6
> +; CHECK-LABEL: entry:
> +; CHECK-NEXT: @foo
> +; CHECK-NEXT: ret i32 0
> +entry:
> +  %v1 = load i32, i32* %p, !invariant.load !0
> +  call void @foo()
> +  %v2 = load i32, i32* %p, !invariant.load !0
> +  %res = sub i32 %v1, %v2
> +  ret i32 %res
> +}
> +
> +declare noalias i32* @bar(...)
> +
> +; Same as previous, but a function with a noalias result (since they're
> handled
> +; differently in MDA)
> +define i32 @test7(i1 %cnd, i32* %p) {
> +; CHECK-LABEL: test7
> +; CHECK-LABEL: entry:
> +; CHECK-NEXT: @bar
> +; CHECK-NEXT: ret i32 0
> +entry:
> +  %v1 = load i32, i32* %p, !invariant.load !0
> +  call i32* (...)* @bar(i32* %p)
> +  %v2 = load i32, i32* %p, !invariant.load !0
> +  %res = sub i32 %v1, %v2
> +  ret i32 %res
> +}
> +
> +
>  !0 = !{ }
>
> EMAIL PREFERENCES
>   http://reviews.llvm.org/settings/panel/emailpreferences/
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150324/861110ad/attachment.html>