[PATCH] MemoryDependenceAnalysis always depends on DominatorTree

[Daniel Berlin]

Err, this pretty clearly returns the wrong answer as well :)
If DT is unavailable, it's going to say there are no dependencies.
If we need to determine reachability to give correct answers, and we
don't have DT available, the only safe answer is "no idea""

You can't achieve this by playing with these loops.
Realistically, you should short circuit both these two loops if the
number of entries to be added is non-zero, and DT is not available

IE in the first loop:

if (!DT && (std::distance(Cache->begin(), Cache->end()) >0))
  <create fake unknown clobber and add it to the list, then return>



On Mon, Apr 15, 2013 at 6:06 PM, Matt Arsenault
<Matthew.Arsenault at amd.com> wrote:
>   Conservatively assume that blocks are UNreachable instead of reachable.
>
>   Add test case from nlewycky which in the last patch version infinite looped when DT wasn't available.
>
> Hi nlewycky,
>
> http://llvm-reviews.chandlerc.com/D583
>
> CHANGE SINCE LAST DIFF
>   http://llvm-reviews.chandlerc.com/D583?vs=1472&id=1641#toc
>
> Files:
>   lib/Analysis/MemoryDependenceAnalysis.cpp
>   test/Analysis/MemoryDependenceAnalysis/lit.local.cfg
>   test/Analysis/MemoryDependenceAnalysis/memdep_requires_dominator_tree.ll
>   test/Transforms/GVN/unreachable_block_infinite_loop.ll
>
> Index: lib/Analysis/MemoryDependenceAnalysis.cpp
> ===================================================================
> --- lib/Analysis/MemoryDependenceAnalysis.cpp
> +++ lib/Analysis/MemoryDependenceAnalysis.cpp
> @@ -913,7 +913,6 @@
>                              SmallVectorImpl<NonLocalDepResult> &Result,
>                              DenseMap<BasicBlock*, Value*> &Visited,
>                              bool SkipFirstBlock) {
> -
>    // Look up the cached info for Pointer.
>    ValueIsLoadPair CacheKey(Pointer.getAddr(), isLoad);
>
> @@ -1001,7 +1000,8 @@
>      for (NonLocalDepInfo::iterator I = Cache->begin(), E = Cache->end();
>           I != E; ++I) {
>        Visited.insert(std::make_pair(I->getBB(), Addr));
> -      if (!I->getResult().isNonLocal() && DT->isReachableFromEntry(I->getBB()))
> +      if (!I->getResult().isNonLocal() &&
> +          (DT ? DT->isReachableFromEntry(I->getBB()) : false))
>          Result.push_back(NonLocalDepResult(I->getBB(), I->getResult(), Addr));
>      }
>      ++NumCacheCompleteNonLocalPtr;
> @@ -1047,7 +1047,8 @@
>                                                   NumSortedEntries);
>
>        // If we got a Def or Clobber, add this to the list of results.
> -      if (!Dep.isNonLocal() && DT->isReachableFromEntry(BB)) {
> +      if (!Dep.isNonLocal() &&
> +          (DT ? DT->isReachableFromEntry(BB) : false)) {
>          Result.push_back(NonLocalDepResult(BB, Dep, Pointer.getAddr()));
>          continue;
>        }
> Index: test/Analysis/MemoryDependenceAnalysis/lit.local.cfg
> ===================================================================
> --- /dev/null
> +++ test/Analysis/MemoryDependenceAnalysis/lit.local.cfg
> @@ -0,0 +1 @@
> +config.suffixes = ['.ll']
> Index: test/Analysis/MemoryDependenceAnalysis/memdep_requires_dominator_tree.ll
> ===================================================================
> --- /dev/null
> +++ test/Analysis/MemoryDependenceAnalysis/memdep_requires_dominator_tree.ll
> @@ -0,0 +1,19 @@
> +; RUN: opt -memdep -gvn < %s
> +
> +define void @__memdep_requires_dominator_tree(i32* nocapture %bufUInt, i32* nocapture %pattern) nounwind {
> +entry:
> +  br label %for.body
> +
> +for.exit:                                         ; preds = %for.body
> +  ret void
> +
> +for.body:                                         ; preds = %for.body, %entry
> +  %i.01 = phi i32 [ 0, %entry ], [ %tmp8.7, %for.body ]
> +  %arrayidx = getelementptr i32* %bufUInt, i32 %i.01
> +  %arrayidx5 = getelementptr i32* %pattern, i32 %i.01
> +  %tmp6 = load i32* %arrayidx5, align 4
> +  store i32 %tmp6, i32* %arrayidx, align 4
> +  %tmp8.7 = add i32 %i.01, 8
> +  %cmp.7 = icmp ult i32 %tmp8.7, 1024
> +  br i1 %cmp.7, label %for.body, label %for.exit
> +}
> Index: test/Transforms/GVN/unreachable_block_infinite_loop.ll
> ===================================================================
> --- /dev/null
> +++ test/Transforms/GVN/unreachable_block_infinite_loop.ll
> @@ -0,0 +1,14 @@
> +; RUN: opt -memdep -gvn -disable-output
> +
> +target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64"
> +target triple = "x86_64-apple-darwin10.0"
> +
> +define i32 @test2() nounwind ssp {
> +entry:
> +    ret i32 0
> +
> +unreachable_block:
> +    %a = add i32 %a, 1
> +    ret i32 %a
> +}
> +
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>