From 733c54da1e5432d9d64f88ea960121fa7a16076a Mon Sep 17 00:00:00 2001
From: Dan Gohman <gohman@apple.com>
Date: Wed, 10 Nov 2010 21:45:11 +0000
Subject: [PATCH] Fully invalidate cached results when a prior query's size or
 type is insufficient for, or incompatible with, the current query.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@118721 91177308-0d34-0410-b5e6-96231b3b80d8
---
 lib/Analysis/MemoryDependenceAnalysis.cpp     | 33 +++++--
 .../gvn-nonlocal-type-mismatch.ll             | 91 +++++++++++++++++++
 2 files changed, 114 insertions(+), 10 deletions(-)
 create mode 100644 test/Analysis/TypeBasedAliasAnalysis/gvn-nonlocal-type-mismatch.ll

diff --git a/lib/Analysis/MemoryDependenceAnalysis.cpp b/lib/Analysis/MemoryDependenceAnalysis.cpp
index b4c6d09e4c2..97c679ae149 100644
--- a/lib/Analysis/MemoryDependenceAnalysis.cpp
+++ b/lib/Analysis/MemoryDependenceAnalysis.cpp
@@ -756,24 +756,37 @@ getNonLocalPointerDepFromBB(const PHITransAddr &Pointer,
     NonLocalPointerDeps.insert(std::make_pair(CacheKey, InitialNLPI));
   NonLocalPointerInfo *CacheInfo = &Pair.first->second;
 
+  // If we already have a cache entry for this CacheKey, we may need to do some
+  // work to reconcile the cache entry and the current query.
   if (!Pair.second) {
-    // If this query's Size is inconsistent with the cached one, take the
-    // maximum size and restart the query.
-    if (CacheInfo->Size != Loc.Size) {
-      CacheInfo->Size = std::max(CacheInfo->Size, Loc.Size);
+    if (CacheInfo->Size < Loc.Size) {
+      // The query's Size is greater than the cached one. Throw out the
+      // cached data and procede with the query at the greater size.
+      CacheInfo->Pair = BBSkipFirstBlockPair();
+      CacheInfo->Size = Loc.Size;
+      CacheInfo->NonLocalDeps.clear();
+    } else if (CacheInfo->Size > Loc.Size) {
+      // This query's Size is less than the cached one. Conservatively restart
+      // the query using the greater size.
       return getNonLocalPointerDepFromBB(Pointer,
                                          Loc.getWithNewSize(CacheInfo->Size),
                                          isLoad, StartBB, Result, Visited,
                                          SkipFirstBlock);
     }
 
-    // If this query's TBAATag is inconsistent with the cached one, discard the
-    // tag and restart the query.
+    // If the query's TBAATag is inconsistent with the cached one,
+    // conservatively throw out the cached data and restart the query with
+    // no tag if needed.
     if (CacheInfo->TBAATag != Loc.TBAATag) {
-      CacheInfo->TBAATag = 0;
-      return getNonLocalPointerDepFromBB(Pointer, Loc.getWithoutTBAATag(),
-                                         isLoad, StartBB, Result, Visited,
-                                         SkipFirstBlock);
+      if (CacheInfo->TBAATag) {
+        CacheInfo->Pair = BBSkipFirstBlockPair();
+        CacheInfo->TBAATag = 0;
+        CacheInfo->NonLocalDeps.clear();
+      }
+      if (Loc.TBAATag)
+        return getNonLocalPointerDepFromBB(Pointer, Loc.getWithoutTBAATag(),
+                                           isLoad, StartBB, Result, Visited,
+                                           SkipFirstBlock);
     }
   }
 
diff --git a/test/Analysis/TypeBasedAliasAnalysis/gvn-nonlocal-type-mismatch.ll b/test/Analysis/TypeBasedAliasAnalysis/gvn-nonlocal-type-mismatch.ll
new file mode 100644
index 00000000000..d6051939024
--- /dev/null
+++ b/test/Analysis/TypeBasedAliasAnalysis/gvn-nonlocal-type-mismatch.ll
@@ -0,0 +1,91 @@
+; RUN: opt -enable-tbaa -tbaa -basicaa -gvn -S < %s | FileCheck %s
+
+target datalayout = "e-p:64:64:64"
+
+; GVN should ignore the store to p1 to see that the load from p is
+; fully redundant.
+
+; CHECK: @yes
+; CHECK: if.then:
+; CHECK-NEXT: store i32 0, i32* %q
+; CHECK-NEXT: ret void
+
+define void @yes(i1 %c, i32* %p, i32* %p1, i32* %q) nounwind {
+entry:
+  store i32 0, i32* %p, !tbaa !1
+  store i32 1, i32* %p1, !tbaa !2
+  br i1 %c, label %if.else, label %if.then
+
+if.then:
+  %t = load i32* %p, !tbaa !1
+  store i32 %t, i32* %q
+  ret void
+
+if.else:
+  ret void
+}
+
+; GVN should ignore the store to p1 to see that the first load from p is
+; fully redundant. However, the second load uses a different type. Theoretically
+; the other type could be unified with the first type, however for now, GVN
+; should just be conservative.
+
+; CHECK: @watch_out_for_type_change
+; CHECK: if.then:
+; CHECK:   %t = load i32* %p
+; CHECK:   store i32 %t, i32* %q
+; CHECK:   ret void
+; CHECK: if.else:
+; CHECK:   %u = load i32* %p
+; CHECK:   store i32 %u, i32* %q
+
+define void @watch_out_for_type_change(i1 %c, i32* %p, i32* %p1, i32* %q) nounwind {
+entry:
+  store i32 0, i32* %p, !tbaa !1
+  store i32 1, i32* %p1, !tbaa !2
+  br i1 %c, label %if.else, label %if.then
+
+if.then:
+  %t = load i32* %p, !tbaa !4
+  store i32 %t, i32* %q
+  ret void
+
+if.else:
+  %u = load i32* %p, !tbaa !3
+  store i32 %u, i32* %q
+  ret void
+}
+
+; As before, but the types are swapped. This time GVN does managed to
+; eliminate one of the loads before noticing the type mismatch.
+
+; CHECK: @watch_out_for_another_type_change
+; CHECK: if.then:
+; CHECK:   %t = load i32* %p
+; CHECK:   store i32 %t, i32* %q
+; CHECK:   ret void
+; CHECK: if.else:
+; CHECK:   store i32 0, i32* %q
+
+define void @watch_out_for_another_type_change(i1 %c, i32* %p, i32* %p1, i32* %q) nounwind {
+entry:
+  store i32 0, i32* %p, !tbaa !1
+  store i32 1, i32* %p1, !tbaa !2
+  br i1 %c, label %if.else, label %if.then
+
+if.then:
+  %t = load i32* %p, !tbaa !3
+  store i32 %t, i32* %q
+  ret void
+
+if.else:
+  %u = load i32* %p, !tbaa !4
+  store i32 %u, i32* %q
+  ret void
+}
+
+!0 = metadata !{}
+!1 = metadata !{metadata !"red", metadata !0}
+!2 = metadata !{metadata !"blu", metadata !0}
+!3 = metadata !{metadata !"outer space"}
+!4 = metadata !{metadata !"brick red", metadata !1}