diff options
author | Reid Kleckner <rnk@google.com> | 2017-12-28 05:10:33 +0000 |
---|---|---|
committer | Reid Kleckner <rnk@google.com> | 2017-12-28 05:10:33 +0000 |
commit | 6d31001cd638c7b72362388618369af576822691 (patch) | |
tree | 1f0d336c46b3e3a32b1a7c8dafc428c0427aad67 /llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp | |
parent | 4371e049d4b81177cf3ac0f62726a2d240f60a77 (diff) | |
download | bcm5719-llvm-6d31001cd638c7b72362388618369af576822691.tar.gz bcm5719-llvm-6d31001cd638c7b72362388618369af576822691.zip |
Revert "[memcpyopt] Teach memcpyopt to optimize across basic blocks"
This reverts r321138. It seems there are still underlying issues with
memdep. PR35519 seems to still be present if debug info is enabled. We
end up losing a memcpy. Somehow during store to memset merging, we
insert the memset after the memcpy or fail to update the memdep analysis
to account for the newly inserted memset of a pair.
Reduced test case:
#include <assert.h>
#include <stdio.h>
#include <string>
#include <utility>
#include <vector>
void do_push_back(
std::vector<std::pair<std::string, std::vector<std::string>>>* crls) {
crls->push_back(std::make_pair(std::string(), std::vector<std::string>()));
}
int __attribute__((optnone)) main() {
// Put some data in the vector and then remove it so we take the push_back
// fast path.
std::vector<std::pair<std::string, std::vector<std::string>>> crl_set;
crl_set.push_back({"asdf", {}});
crl_set.pop_back();
printf("first word in vector storage: %p\n", *(void**)crl_set.data());
// Do the push_back which may fail to initialize the data.
do_push_back(&crl_set);
auto* first = &crl_set.back().first;
printf("first word in vector storage (should be zero): %p\n",
*(void**)crl_set.data());
assert(first->empty());
puts("ok");
}
Compile with libc++, enable optimizations, and enable debug info:
$ clang++ -stdlib=libc++ -g -O2 t.cpp -o t.exe -Wl,-rpath=llvm/build/lib
This program will assert with this change.
llvm-svn: 321510
Diffstat (limited to 'llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp')
-rw-r--r-- | llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp | 56 |
1 files changed, 10 insertions, 46 deletions
diff --git a/llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp b/llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp index 6af3fef963d..9c870b42a74 100644 --- a/llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp +++ b/llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp @@ -476,33 +476,22 @@ Instruction *MemCpyOptPass::tryMergingIntoMemset(Instruction *StartInst, Alignment = DL.getABITypeAlignment(EltType); } - // Remember the debug location. - DebugLoc Loc; - if (!Range.TheStores.empty()) - Loc = Range.TheStores[0]->getDebugLoc(); + AMemSet = + Builder.CreateMemSet(StartPtr, ByteVal, Range.End-Range.Start, Alignment); DEBUG(dbgs() << "Replace stores:\n"; for (Instruction *SI : Range.TheStores) - dbgs() << *SI << '\n'); + dbgs() << *SI << '\n'; + dbgs() << "With: " << *AMemSet << '\n'); + + if (!Range.TheStores.empty()) + AMemSet->setDebugLoc(Range.TheStores[0]->getDebugLoc()); // Zap all the stores. for (Instruction *SI : Range.TheStores) { MD->removeInstruction(SI); SI->eraseFromParent(); } - - // Create the memset after removing the stores, so that if there any cached - // non-local dependencies on the removed instructions in - // MemoryDependenceAnalysis, the cache entries are updated to "dirty" - // entries pointing below the memset, so subsequent queries include the - // memset. - AMemSet = - Builder.CreateMemSet(StartPtr, ByteVal, Range.End-Range.Start, Alignment); - if (!Range.TheStores.empty()) - AMemSet->setDebugLoc(Loc); - - DEBUG(dbgs() << "With: " << *AMemSet << '\n'); - ++NumMemSetInfer; } @@ -1042,22 +1031,9 @@ bool MemCpyOptPass::processMemCpyMemCpyDependence(MemCpyInst *M, // // NOTE: This is conservative, it will stop on any read from the source loc, // not just the defining memcpy. - MemoryLocation SourceLoc = MemoryLocation::getForSource(MDep); - MemDepResult SourceDep = MD->getPointerDependencyFrom(SourceLoc, false, - M->getIterator(), M->getParent()); - - if (SourceDep.isNonLocal()) { - SmallVector<NonLocalDepResult, 2> NonLocalDepResults; - MD->getNonLocalPointerDependencyFrom(M, SourceLoc, /*isLoad=*/false, - NonLocalDepResults); - if (NonLocalDepResults.size() == 1) { - SourceDep = NonLocalDepResults[0].getResult(); - assert((!SourceDep.getInst() || - LookupDomTree().dominates(SourceDep.getInst(), M)) && - "when memdep returns exactly one result, it should dominate"); - } - } - + MemDepResult SourceDep = + MD->getPointerDependencyFrom(MemoryLocation::getForSource(MDep), false, + M->getIterator(), M->getParent()); if (!SourceDep.isClobber() || SourceDep.getInst() != MDep) return false; @@ -1259,18 +1235,6 @@ bool MemCpyOptPass::processMemCpy(MemCpyInst *M) { MemDepResult SrcDepInfo = MD->getPointerDependencyFrom( SrcLoc, true, M->getIterator(), M->getParent()); - if (SrcDepInfo.isNonLocal()) { - SmallVector<NonLocalDepResult, 2> NonLocalDepResults; - MD->getNonLocalPointerDependencyFrom(M, SrcLoc, /*isLoad=*/true, - NonLocalDepResults); - if (NonLocalDepResults.size() == 1) { - SrcDepInfo = NonLocalDepResults[0].getResult(); - assert((!SrcDepInfo.getInst() || - LookupDomTree().dominates(SrcDepInfo.getInst(), M)) && - "when memdep returns exactly one result, it should dominate"); - } - } - if (SrcDepInfo.isClobber()) { if (MemCpyInst *MDep = dyn_cast<MemCpyInst>(SrcDepInfo.getInst())) return processMemCpyMemCpyDependence(M, MDep); |