<div dir="ltr">LGTM</div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Nov 22, 2016 at 7:23 PM, bryant <span dir="ltr"><<a href="mailto:3.14472+reviews.llvm.org@gmail.com" target="_blank">3.14472+reviews.llvm.org@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">bryant updated this revision to Diff 79016.<br>
bryant added a comment.<br>
<br>
Added clobber tests.<br>
<span class=""><br>
<br>
Repository:<br>
  rL LLVM<br>
<br>
<a href="https://reviews.llvm.org/D26659" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D26659</a><br>
<br>
Files:<br>
  lib/Transforms/Utils/<wbr>MemorySSA.cpp<br>
</span>  unittests/Transforms/Utils/<wbr>MemorySSA.cpp<br>
<br>
<br>
Index: unittests/Transforms/Utils/<wbr>MemorySSA.cpp<br>
==============================<wbr>==============================<wbr>=======<br>
--- unittests/Transforms/Utils/<wbr>MemorySSA.cpp<br>
+++ unittests/Transforms/Utils/<wbr>MemorySSA.cpp<br>
@@ -14,6 +14,7 @@<br>
 #include "llvm/IR/Dominators.h"<br>
 #include "llvm/IR/IRBuilder.h"<br>
 #include "llvm/IR/Instructions.h"<br>
+#include "llvm/IR/IntrinsicInst.h"<br>
 #include "llvm/IR/LLVMContext.h"<br>
 #include "gtest/gtest.h"<br>
<br>
@@ -484,3 +485,43 @@<br>
   EXPECT_EQ(Walker-><wbr>getClobberingMemoryAccess(<wbr>NewLoadAccess), LoadClobber);<br>
   EXPECT_EQ(NewLoadAccess-><wbr>getDefiningAccess(), LoadClobber);<br>
 }<br>
+<br>
+TEST_F(MemorySSATest, InstClobbersMemLoc) {<br>
+  F = Function::Create(FunctionType:<wbr>:get(B.getVoidTy(),<br>
+                                         {B.getInt8PtrTy(), B.getInt8PtrTy()},<br>
+                                         false),<br>
+                       GlobalValue::ExternalLinkage, "F", &M);<br>
+  B.SetInsertPoint(BasicBlock::<wbr>Create(C, "", F));<br>
+  Argument *P = &*F->arg_begin();<br>
+  Argument *Q = &*std::next(F->arg_begin());<br>
+<br>
+  Type *Int64 = Type::getInt64Ty(C);<br>
+<br>
+  AllocaInst *A =<br>
+      B.CreateAlloca(Type::<wbr>getInt128Ty(C), ConstantInt::get(Int64, 1), "A");<br>
+<br>
+  auto *M0 = B.CreateMemCpy(A, P, ConstantInt::get(Int64, 16), 1);<br>
+  auto *M1 =<br>
+      cast<MemCpyInst>(B.<wbr>CreateMemCpy(Q, A, ConstantInt::get(Int64, 16), 1));<br>
+  auto *M2 =<br>
+      cast<MemCpyInst>(B.<wbr>CreateMemCpy(Q, A, ConstantInt::get(Int64, 16), 1));<br>
+  auto *M3 =<br>
+      cast<MemCpyInst>(B.<wbr>CreateMemCpy(Q, P, ConstantInt::get(Int64, 16), 1));<br>
+<br>
+  setupAnalyses();<br>
+  MemorySSA &MSSA = *Analyses->MSSA;<br>
+  MemorySSAWalker &Walker = *Analyses->Walker;<br>
+<br>
+  // M1 obviously clobbers M2 -- on the dest loc.<br>
+  EXPECT_EQ(MSSA.<wbr>getMemoryAccess(M1), Walker.<wbr>getClobberingMemoryAccess(M2))<wbr>;<br>
+  // But only M0 clobbers M2 on its source loc. In other words,<br>
+  // instructionClobbersQuery must call getModRefInfo(<wbr>ImmutableCallSite, const<br>
+  // MemoryLocation).<br>
+  EXPECT_EQ(MSSA.<wbr>getMemoryAccess(M0),<br>
+            Walker.<wbr>getClobberingMemoryAccess(<wbr>MSSA.getMemoryAccess(M2),<br>
+                                             MemoryLocation::getForSource(<wbr>M2)));<br>
+  // FIXME: AA should know that memcpy operands never overlap.<br>
+  EXPECT_NE(MSSA.<wbr>getLiveOnEntryDef(),<br>
+            Walker.<wbr>getClobberingMemoryAccess(<wbr>MSSA.getMemoryAccess(M3),<br>
+                                             MemoryLocation::getForSource(<wbr>M3)));<br>
+}<br>
<div class="HOEnZb"><div class="h5">Index: lib/Transforms/Utils/<wbr>MemorySSA.cpp<br>
==============================<wbr>==============================<wbr>=======<br>
--- lib/Transforms/Utils/<wbr>MemorySSA.cpp<br>
+++ lib/Transforms/Utils/<wbr>MemorySSA.cpp<br>
@@ -235,7 +235,7 @@<br>
   }<br>
<br>
   ImmutableCallSite UseCS(UseInst);<br>
-  if (UseCS) {<br>
+  if (UseCS && UseLoc == MemoryLocation()) {<br>
     ModRefInfo I = AA.getModRefInfo(DefInst, UseCS);<br>
     return I != MRI_NoModRef;<br>
   }<br>
<br>
<br>
</div></div></blockquote></div><br></div>