From 6eb095d7ae616b6aaa1330ea4a47dc79ae4c1b23 Mon Sep 17 00:00:00 2001 From: Ahmed Bougacha Date: Mon, 11 May 2015 23:09:46 +0000 Subject: [PATCH] [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 --- lib/Transforms/Scalar/MemCpyOptimizer.cpp | 13 +++++++------ .../memset-memcpy-redundant-memset.ll | 18 ++++++++++++++++++ 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/lib/Transforms/Scalar/MemCpyOptimizer.cpp b/lib/Transforms/Scalar/MemCpyOptimizer.cpp index 8465ffd5064..c0fe949b515 100644 --- 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(SrcDepInfo.getInst())) + if (DepInfo.isClobber()) + if (MemSetInst *MDep = dyn_cast(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(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(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 0a21e71b21b..d0ec1dde828 100644 --- 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) -- 2.34.1