diff --git a/include/llvm/Analysis/MemoryDependenceAnalysis.h b/include/llvm/Analysis/MemoryDependenceAnalysis.h index 5bd6a82d013..b3c12c1d856 100644 --- a/include/llvm/Analysis/MemoryDependenceAnalysis.h +++ b/include/llvm/Analysis/MemoryDependenceAnalysis.h @@ -178,11 +178,6 @@ namespace llvm { /// updating the dependence of instructions that previously depended on it. void removeInstruction(Instruction *InstToRemove); - /// dropInstruction - Remove an instruction from the analysis, making - /// absolutely conservative assumptions when updating the cache. This is - /// useful, for example when an instruction is changed rather than removed. - void dropInstruction(Instruction *InstToDrop); - private: DepResultTy ConvFromResult(MemDepResult R) { if (Instruction *I = R.getInst()) diff --git a/lib/Analysis/MemoryDependenceAnalysis.cpp b/lib/Analysis/MemoryDependenceAnalysis.cpp index 56128e68966..779814ab711 100644 --- a/lib/Analysis/MemoryDependenceAnalysis.cpp +++ b/lib/Analysis/MemoryDependenceAnalysis.cpp @@ -138,7 +138,8 @@ getNonLocalDependency(Instruction *QueryInst, DirtyBlocks.append(pred_begin(QueryBB), pred_end(QueryBB)); NumUncacheNonLocal++; } - + + // Iterate while we still have blocks to update. while (!DirtyBlocks.empty()) { BasicBlock *DirtyBB = DirtyBlocks.back(); @@ -147,10 +148,10 @@ getNonLocalDependency(Instruction *QueryInst, // Get the entry for this block. Note that this relies on DepResultTy // default initializing to Dirty. DepResultTy &DirtyBBEntry = Cache[DirtyBB]; - + // If DirtyBBEntry isn't dirty, it ended up on the worklist multiple times. if (DirtyBBEntry.getInt() != Dirty) continue; - + // Find out if this block has a local dependency for QueryInst. // FIXME: If the dirty entry has an instruction pointer, scan from it! // FIXME: Don't convert back and forth for MemDepResult <-> DepResultTy. @@ -163,12 +164,12 @@ getNonLocalDependency(Instruction *QueryInst, DirtyBBEntry = ConvFromResult(getDependencyFrom(QueryInst, ScanPos, DirtyBB)); - + // If the block has a dependency (i.e. it isn't completely transparent to // the value), remember it! if (DirtyBBEntry.getInt() != NonLocal) { // Keep the ReverseNonLocalDeps map up to date so we can efficiently - // update this when we remove instructions. + // update this when we remove instructions. if (Instruction *Inst = DirtyBBEntry.getPointer()) ReverseNonLocalDeps[Inst].insert(QueryInst); continue; @@ -176,11 +177,10 @@ getNonLocalDependency(Instruction *QueryInst, // If the block *is* completely transparent to the load, we need to check // the predecessors of this block. Add them to our worklist. - for (pred_iterator I = pred_begin(DirtyBB), E = pred_end(DirtyBB); - I != E; ++I) - DirtyBlocks.push_back(*I); + DirtyBlocks.append(pred_begin(DirtyBB), pred_end(DirtyBB)); } + // Copy the result into the output set. for (DenseMap::iterator I = Cache.begin(), E = Cache.end(); I != E; ++I) @@ -324,65 +324,6 @@ MemDepResult MemoryDependenceAnalysis::getDependency(Instruction *QueryInst) { return Res; } - -/// dropInstruction - Remove an instruction from the analysis, making -/// absolutely conservative assumptions when updating the cache. This is -/// useful, for example when an instruction is changed rather than removed. -void MemoryDependenceAnalysis::dropInstruction(Instruction* drop) { - LocalDepMapType::iterator depGraphEntry = LocalDeps.find(drop); - if (depGraphEntry != LocalDeps.end()) - if (Instruction *Inst = depGraphEntry->second.getPointer()) - ReverseLocalDeps[Inst].erase(drop); - - // Drop dependency information for things that depended on this instr - SmallPtrSet& set = ReverseLocalDeps[drop]; - for (SmallPtrSet::iterator I = set.begin(), E = set.end(); - I != E; ++I) - LocalDeps.erase(*I); - - LocalDeps.erase(drop); - ReverseLocalDeps.erase(drop); - - for (DenseMap::iterator DI = - NonLocalDeps[drop].begin(), DE = NonLocalDeps[drop].end(); - DI != DE; ++DI) - if (Instruction *Inst = DI->second.getPointer()) - ReverseNonLocalDeps[Inst].erase(drop); - - if (ReverseNonLocalDeps.count(drop)) { - SmallVector, 8> ReverseDepsToAdd; - - SmallPtrSet& set = - ReverseNonLocalDeps[drop]; - for (SmallPtrSet::iterator I = set.begin(), E = set.end(); - I != E; ++I) - for (DenseMap::iterator DI = - NonLocalDeps[*I].begin(), DE = NonLocalDeps[*I].end(); - DI != DE; ++DI) - if (DI->second.getPointer() == drop) { - // Convert to a dirty entry for the subsequent instruction. - DI->second.setInt(Dirty); - if (drop->isTerminator()) - DI->second.setPointer(0); - else { - Instruction *NextI = next(BasicBlock::iterator(drop)); - DI->second.setPointer(NextI); - ReverseDepsToAdd.push_back(std::make_pair(NextI, *I)); - } - } - - // Add new reverse deps after scanning the set, to avoid invalidating 'Set' - while (!ReverseDepsToAdd.empty()) { - ReverseNonLocalDeps[ReverseDepsToAdd.back().first] - .insert(ReverseDepsToAdd.back().second); - ReverseDepsToAdd.pop_back(); - } - } - - ReverseNonLocalDeps.erase(drop); - NonLocalDeps.erase(drop); -} - /// removeInstruction - Remove an instruction from the dependence analysis, /// updating the dependence of instructions that previously depended on it. /// This method attempts to keep the cache coherent using the reverse map. @@ -439,6 +380,8 @@ void MemoryDependenceAnalysis::removeInstruction(Instruction *RemInst) { // Loop over all of the things that depend on the instruction we're removing. // + SmallVector, 8> ReverseDepsToAdd; + ReverseDepMapType::iterator ReverseDepIt = ReverseLocalDeps.find(RemInst); if (ReverseDepIt != ReverseLocalDeps.end()) { SmallPtrSet &ReverseDeps = ReverseDepIt->second; @@ -451,20 +394,34 @@ void MemoryDependenceAnalysis::removeInstruction(Instruction *RemInst) { if (InstDependingOnRemInst == RemInst) continue; // Insert the new dependencies. + // FIXME: DEPENDENCIES ARE NOT TRANSITIVE! + //cerr << "FOO:\n"; + //RemInst->dump(); + //InstDependingOnRemInst->dump(); LocalDeps[InstDependingOnRemInst] = NewDependency; // If our NewDependency is an instruction, make sure to remember that new // things depend on it. - if (Instruction *Inst = NewDependency.getPointer()) - ReverseLocalDeps[Inst].insert(InstDependingOnRemInst); + if (Instruction *Inst = NewDependency.getPointer()) { + assert(Inst != RemInst); + ReverseDepsToAdd.push_back(std::make_pair(Inst, + InstDependingOnRemInst)); + } + } + + ReverseLocalDeps.erase(ReverseDepIt); + + // Add new reverse deps after scanning the set, to avoid invalidating the + // 'ReverseDeps' reference. + while (!ReverseDepsToAdd.empty()) { + ReverseLocalDeps[ReverseDepsToAdd.back().first] + .insert(ReverseDepsToAdd.back().second); + ReverseDepsToAdd.pop_back(); } - ReverseLocalDeps.erase(RemInst); } ReverseDepIt = ReverseNonLocalDeps.find(RemInst); if (ReverseDepIt != ReverseNonLocalDeps.end()) { - SmallVector, 8> ReverseDepsToAdd; - SmallPtrSet& set = ReverseDepIt->second; for (SmallPtrSet::iterator I = set.begin(), E = set.end(); I != E; ++I) @@ -479,24 +436,23 @@ void MemoryDependenceAnalysis::removeInstruction(Instruction *RemInst) { else { Instruction *NextI = next(BasicBlock::iterator(RemInst)); DI->second.setPointer(NextI); + assert(NextI != RemInst); ReverseDepsToAdd.push_back(std::make_pair(NextI, *I)); } } - + + ReverseNonLocalDeps.erase(ReverseDepIt); + // Add new reverse deps after scanning the set, to avoid invalidating 'Set' while (!ReverseDepsToAdd.empty()) { ReverseNonLocalDeps[ReverseDepsToAdd.back().first] .insert(ReverseDepsToAdd.back().second); ReverseDepsToAdd.pop_back(); } - - ReverseNonLocalDeps.erase(ReverseDepIt); } NonLocalDeps.erase(RemInst); - getAnalysis().deleteValue(RemInst); - DEBUG(verifyRemoved(RemInst)); } diff --git a/lib/Transforms/Scalar/MemCpyOptimizer.cpp b/lib/Transforms/Scalar/MemCpyOptimizer.cpp index 40eaa1d47c3..5d68388a99c 100644 --- a/lib/Transforms/Scalar/MemCpyOptimizer.cpp +++ b/lib/Transforms/Scalar/MemCpyOptimizer.cpp @@ -609,7 +609,7 @@ bool MemCpyOpt::performCallSlotOptzn(MemCpyInst *cpy, CallInst *C) { // Drop any cached information about the call, because we may have changed // its dependence information by changing its parameter. MemoryDependenceAnalysis& MD = getAnalysis(); - MD.dropInstruction(C); + MD.removeInstruction(C); // Remove the memcpy MD.removeInstruction(cpy); @@ -691,11 +691,9 @@ bool MemCpyOpt::processMemCpy(MemCpyInst* M) { // If C and M don't interfere, then this is a valid transformation. If they // did, this would mean that the two sources overlap, which would be bad. if (MD.getDependency(C) == dep) { - MD.dropInstruction(M); + MD.removeInstruction(M); M->eraseFromParent(); - NumMemCpyInstr++; - return true; } @@ -703,7 +701,6 @@ bool MemCpyOpt::processMemCpy(MemCpyInst* M) { // inserted and act like nothing happened. MD.removeInstruction(C); C->eraseFromParent(); - return false; }