[MemCpyOpt] Look at any dependency -not just source- for memset+memcpy.

This fixes another miscompile introduced by r235232: when there was a
dependency on the memcpy destination other than the memset, we would
ignore it, because we only looked at the source dependency.

It was a mistake to use SrcDepInfo.  Instead, just use DepInfo.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@237066 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/Transforms/Scalar/MemCpyOptimizer.cpp b/lib/Transforms/Scalar/MemCpyOptimizer.cpp
index 8465ffd50647da4a23ad6f776234d1785a04b2fa..c0fe949b51575a39c4df3365616a091720b0481b 100644 (file)
--- a/lib/Transforms/Scalar/MemCpyOptimizer.cpp
+++ b/lib/Transforms/Scalar/MemCpyOptimizer.cpp
@@ -927,14 +927,12 @@ bool MemCpyOpt::processMemCpy(MemCpyInst *M) {
         return true;
       }
 
-  AliasAnalysis::Location SrcLoc = AliasAnalysis::getLocationForSource(M);
-  MemDepResult SrcDepInfo = MD->getPointerDependencyFrom(SrcLoc, true,
-                                                         M, M->getParent());
+  MemDepResult DepInfo = MD->getDependency(M);
 
   // Try to turn a partially redundant memset + memcpy into
   // memcpy + smaller memset.  We don't need the memcpy size for this.
-  if (SrcDepInfo.isClobber())
-    if (MemSetInst *MDep = dyn_cast<MemSetInst>(SrcDepInfo.getInst()))
+  if (DepInfo.isClobber())
+    if (MemSetInst *MDep = dyn_cast<MemSetInst>(DepInfo.getInst()))
       if (processMemSetMemCpyDependence(M, MDep))
         return true;
 
@@ -948,7 +946,6 @@ bool MemCpyOpt::processMemCpy(MemCpyInst *M) {
   //   c) memcpy from freshly alloca'd space or space that has just started its
   //      lifetime copies undefined data, and we can therefore eliminate the
   //      memcpy in favor of the data that was already at the destination.
-  MemDepResult DepInfo = MD->getDependency(M);
   if (DepInfo.isClobber()) {
     if (CallInst *C = dyn_cast<CallInst>(DepInfo.getInst())) {
       if (performCallSlotOptzn(M, M->getDest(), M->getSource(),
@@ -961,6 +958,10 @@ bool MemCpyOpt::processMemCpy(MemCpyInst *M) {
     }
   }
 
+  AliasAnalysis::Location SrcLoc = AliasAnalysis::getLocationForSource(M);
+  MemDepResult SrcDepInfo = MD->getPointerDependencyFrom(SrcLoc, true,
+                                                         M, M->getParent());
+
   if (SrcDepInfo.isClobber()) {
     if (MemCpyInst *MDep = dyn_cast<MemCpyInst>(SrcDepInfo.getInst()))
       return processMemCpyMemCpyDependence(M, MDep, CopySize->getZExtValue());

diff --git a/test/Transforms/MemCpyOpt/memset-memcpy-redundant-memset.ll b/test/Transforms/MemCpyOpt/memset-memcpy-redundant-memset.ll
index 0a21e71b21b54df4341dc124e4993f4230b5808d..d0ec1dde82849fcca4efd0f6f4d83c4132d933c4 100644 (file)
--- a/test/Transforms/MemCpyOpt/memset-memcpy-redundant-memset.ll
+++ b/test/Transforms/MemCpyOpt/memset-memcpy-redundant-memset.ll
@@ -126,6 +126,24 @@ define void @test_different_dst(i8* %dst2, i8* %src, i64 %src_size, i8* %dst, i6
   ret void
 }
 
+; Make sure we also take into account dependencies on the destination.
+
+; CHECK-LABEL: define i8 @test_intermediate_read
+; CHECK-NEXT: %ca = alloca [64 x i8], align 8
+; CHECK-NEXT: %c = getelementptr [64 x i8], [64 x i8]* %ca, i64 0, i64 0
+; CHECK-NEXT: call void @llvm.memset.p0i8.i64(i8* %a, i8 0, i64 64, i32 1, i1 false)
+; CHECK-NEXT: %r = load i8, i8* %a
+; CHECK-NEXT: call void @llvm.memcpy.p0i8.p0i8.i64(i8* %a, i8* %b, i64 24, i32 1, i1 false)
+; CHECK-NEXT: ret i8 %r
+define i8 @test_intermediate_read(i8* %a, i8* %b) #0 {
+  %ca = alloca [64 x i8], align 8
+  %c = getelementptr [64 x i8], [64 x i8]* %ca, i64 0, i64 0
+  call void @llvm.memset.p0i8.i64(i8* %a, i8 0, i64 64, i32 1, i1 false)
+  %r = load i8, i8* %a
+  call void @llvm.memcpy.p0i8.p0i8.i64(i8* %a, i8* %b, i64 24, i32 1, i1 false)
+  ret i8 %r
+}
+
 declare void @llvm.memset.p0i8.i64(i8* nocapture, i8, i64, i32, i1)
 declare void @llvm.memcpy.p0i8.p0i8.i64(i8* nocapture, i8* nocapture readonly, i64, i32, i1)
 declare void @llvm.memset.p0i8.i32(i8* nocapture, i8, i32, i32, i1)