[llvm] r328748 - [MemorySSA] Consider callsite args for hashing and equality.

[George Burgess IV via llvm-commits]

Author: gbiv
Date: Wed Mar 28 17:54:39 2018
New Revision: 328748

URL: http://llvm.org/viewvc/llvm-project?rev=328748&view=rev
Log:
[MemorySSA] Consider callsite args for hashing and equality.

We use a `DenseMap<MemoryLocOrCall, MemlocStackInfo>` to keep track of
prior work when optimizing uses in MemorySSA. Because we weren't
accounting for callsite arguments in either the hash code or equality
tests for `MemoryLocOrCall`s, we optimized uses too aggressively in
some rare cases.

Fix by Daniel Berlin.

Should fix PR36883.

Added:
    llvm/trunk/test/Analysis/MemorySSA/pr36883.ll
Modified:
    llvm/trunk/lib/Analysis/MemorySSA.cpp

Modified: llvm/trunk/lib/Analysis/MemorySSA.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/MemorySSA.cpp?rev=328748&r1=328747&r2=328748&view=diff
==============================================================================
--- llvm/trunk/lib/Analysis/MemorySSA.cpp (original)
+++ llvm/trunk/lib/Analysis/MemorySSA.cpp Wed Mar 28 17:54:39 2018
@@ -153,9 +153,14 @@ public:
     if (IsCall != Other.IsCall)
       return false;
 
-    if (IsCall)
-      return CS.getCalledValue() == Other.CS.getCalledValue();
-    return Loc == Other.Loc;
+    if (!IsCall)
+      return Loc == Other.Loc;
+
+    if (CS.getCalledValue() != Other.CS.getCalledValue())
+      return false;
+
+    assert(CS.arg_size() == Other.CS.arg_size());
+    return std::equal(CS.arg_begin(), CS.arg_end(), Other.CS.arg_begin());
   }
 
 private:
@@ -179,12 +184,18 @@ template <> struct DenseMapInfo<MemoryLo
   }
 
   static unsigned getHashValue(const MemoryLocOrCall &MLOC) {
-    if (MLOC.IsCall)
-      return hash_combine(MLOC.IsCall,
-                          DenseMapInfo<const Value *>::getHashValue(
-                              MLOC.getCS().getCalledValue()));
-    return hash_combine(
-        MLOC.IsCall, DenseMapInfo<MemoryLocation>::getHashValue(MLOC.getLoc()));
+    if (!MLOC.IsCall)
+      return hash_combine(
+          MLOC.IsCall,
+          DenseMapInfo<MemoryLocation>::getHashValue(MLOC.getLoc()));
+
+    hash_code hash =
+        hash_combine(MLOC.IsCall, DenseMapInfo<const Value *>::getHashValue(
+                                      MLOC.getCS().getCalledValue()));
+
+    for (const Value *Arg : MLOC.getCS().args())
+      hash = hash_combine(hash, DenseMapInfo<const Value *>::getHashValue(Arg));
+    return hash;
   }
 
   static bool isEqual(const MemoryLocOrCall &LHS, const MemoryLocOrCall &RHS) {

Added: llvm/trunk/test/Analysis/MemorySSA/pr36883.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Analysis/MemorySSA/pr36883.ll?rev=328748&view=auto
==============================================================================
--- llvm/trunk/test/Analysis/MemorySSA/pr36883.ll (added)
+++ llvm/trunk/test/Analysis/MemorySSA/pr36883.ll Wed Mar 28 17:54:39 2018
@@ -0,0 +1,38 @@
+; RUN: opt -basicaa -memoryssa -analyze < %s 2>&1 -S | FileCheck %s
+; RUN: opt -aa-pipeline=basic-aa -passes='print<memoryssa>,verify<memoryssa>' -S < %s 2>&1 | FileCheck %s
+;
+; We weren't properly considering the args in callsites in equality or hashing.
+
+target triple = "armv7-dcg-linux-gnueabi"
+
+; CHECK-LABEL: define <8 x i16> @vpx_idct32_32_neon
+define <8 x i16> @vpx_idct32_32_neon(i8* %p, <8 x i16> %v) {
+entry:
+; CHECK: MemoryUse(liveOnEntry)
+  %load1 = call <8 x i16> @llvm.arm.neon.vld1.v8i16.p0i8(i8* %p, i32 2) #4 ; load CSE replacement
+
+; CHECK: 1 = MemoryDef(liveOnEntry)
+  call void @llvm.arm.neon.vst1.p0i8.v8i16(i8* %p, <8 x i16> %v, i32 2) #4 ; clobber
+
+  %p_next = getelementptr inbounds i8, i8* %p, i32 16
+; CHECK: MemoryUse(liveOnEntry)
+  %load2 = call <8 x i16> @llvm.arm.neon.vld1.v8i16.p0i8(i8* %p_next, i32 2) #4 ; non-aliasing load needed to trigger bug
+
+; CHECK: MemoryUse(1)
+  %load3 = call <8 x i16> @llvm.arm.neon.vld1.v8i16.p0i8(i8* %p, i32 2) #4 ; load CSE removed
+
+  %add = add <8 x i16> %load1, %load2
+  %ret = add <8 x i16> %add, %load3
+  ret <8 x i16> %ret
+}
+
+; Function Attrs: argmemonly nounwind readonly
+declare <8 x i16> @llvm.arm.neon.vld1.v8i16.p0i8(i8*, i32) #2
+
+; Function Attrs: argmemonly nounwind
+declare void @llvm.arm.neon.vst1.p0i8.v8i16(i8*, <8 x i16>, i32) #1
+
+attributes #1 = { argmemonly nounwind }
+attributes #2 = { argmemonly nounwind readonly }
+attributes #3 = { nounwind readnone }
+attributes #4 = { nounwind }