Now that DepType is private, we can start cleaning up some of its uses:

Document the Dirty value more precisely, use it for the uninitialized
DepResultTy value.  Change reverse mappings to be from an instruction*
instead of DepResultTy, and stop tracking other forms.  This makes it more
clear that we only care about the instruction cases.

Eliminate a DepResultTy,bool pair by using Dirty in the local case as well,
shrinking the map and simplifying the code.

This speeds up GVN by ~3% on 403.gcc.



git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@60232 91177308-0d34-0410-b5e6-96231b3b80d8
This commit is contained in:
Chris Lattner 2008-11-29 03:22:12 +00:00
parent 4c72400625
commit 7f52422a3c
2 changed files with 81 additions and 84 deletions

View File

@ -83,9 +83,24 @@ namespace llvm {
/// DepType - This enum is used to indicate what flavor of dependence this /// DepType - This enum is used to indicate what flavor of dependence this
/// is. If the type is Normal, there is an associated instruction pointer. /// is. If the type is Normal, there is an associated instruction pointer.
enum DepType { enum DepType {
/// Dirty - Entries with this marker may come in two forms, depending on
/// whether they are in a LocalDeps map or NonLocalDeps map. In either
/// case, this marker indicates that the cached value has been invalidated
/// by a removeInstruction call.
///
/// If in the LocalDeps map, the Instruction field will indicate the place
/// in the current block to start scanning. If in the non-localdeps map,
/// the instruction will be null.
///
/// In a default-constructed DepResultTy object, the type will be Dirty
/// and the instruction pointer will be null.
///
/// FIXME: Why not add a scanning point for the non-local deps map???
Dirty = 0,
/// Normal - This is a normal instruction dependence. The pointer member /// Normal - This is a normal instruction dependence. The pointer member
/// of the DepResultTy pair holds the instruction. /// of the DepResultTy pair holds the instruction.
Normal = 0, Normal,
/// None - This dependence type indicates that the query does not depend /// None - This dependence type indicates that the query does not depend
/// on any instructions, either because it scanned to the start of the /// on any instructions, either because it scanned to the start of the
@ -96,18 +111,12 @@ namespace llvm {
/// NonLocal - This marker indicates that the query has no dependency in /// NonLocal - This marker indicates that the query has no dependency in
/// the specified block. To find out more, the client should query other /// the specified block. To find out more, the client should query other
/// predecessor blocks. /// predecessor blocks.
NonLocal, NonLocal
/// Dirty - This is an internal marker indicating that that a cache entry
/// is dirty.
Dirty
}; };
typedef PointerIntPair<Instruction*, 2, DepType> DepResultTy; typedef PointerIntPair<Instruction*, 2, DepType> DepResultTy;
// A map from instructions to their dependency, with a boolean // A map from instructions to their dependency.
// flags for whether this mapping is confirmed or not. typedef DenseMap<Instruction*, DepResultTy> LocalDepMapType;
typedef DenseMap<Instruction*,
std::pair<DepResultTy, bool> > LocalDepMapType;
LocalDepMapType LocalDeps; LocalDepMapType LocalDeps;
// A map from instructions to their non-local dependencies. // A map from instructions to their non-local dependencies.
@ -118,7 +127,7 @@ namespace llvm {
// A reverse mapping from dependencies to the dependees. This is // A reverse mapping from dependencies to the dependees. This is
// used when removing instructions to keep the cache coherent. // used when removing instructions to keep the cache coherent.
typedef DenseMap<DepResultTy, typedef DenseMap<Instruction*,
SmallPtrSet<Instruction*, 4> > reverseDepMapType; SmallPtrSet<Instruction*, 4> > reverseDepMapType;
reverseDepMapType reverseDep; reverseDepMapType reverseDep;

View File

@ -26,7 +26,6 @@
#include "llvm/Support/CommandLine.h" #include "llvm/Support/CommandLine.h"
#include "llvm/Support/Debug.h" #include "llvm/Support/Debug.h"
#include "llvm/Target/TargetData.h" #include "llvm/Target/TargetData.h"
using namespace llvm; using namespace llvm;
// Control the calculation of non-local dependencies by only examining the // Control the calculation of non-local dependencies by only examining the
@ -51,7 +50,7 @@ void MemoryDependenceAnalysis::verifyRemoved(Instruction *D) const {
for (LocalDepMapType::const_iterator I = LocalDeps.begin(), for (LocalDepMapType::const_iterator I = LocalDeps.begin(),
E = LocalDeps.end(); I != E; ++I) { E = LocalDeps.end(); I != E; ++I) {
assert(I->first != D && "Inst occurs in data structures"); assert(I->first != D && "Inst occurs in data structures");
assert(I->second.first.getPointer() != D && assert(I->second.getPointer() != D &&
"Inst occurs in data structures"); "Inst occurs in data structures");
} }
@ -89,7 +88,7 @@ void MemoryDependenceAnalysis::getAnalysisUsage(AnalysisUsage &AU) const {
/// of a call site. /// of a call site.
MemDepResult MemoryDependenceAnalysis:: MemDepResult MemoryDependenceAnalysis::
getCallSiteDependency(CallSite C, Instruction *start, BasicBlock *block) { getCallSiteDependency(CallSite C, Instruction *start, BasicBlock *block) {
std::pair<DepResultTy, bool> &cachedResult = LocalDeps[C.getInstruction()]; DepResultTy &cachedResult = LocalDeps[C.getInstruction()];
AliasAnalysis &AA = getAnalysis<AliasAnalysis>(); AliasAnalysis &AA = getAnalysis<AliasAnalysis>();
TargetData &TD = getAnalysis<TargetData>(); TargetData &TD = getAnalysis<TargetData>();
BasicBlock::iterator blockBegin = C.getInstruction()->getParent()->begin(); BasicBlock::iterator blockBegin = C.getInstruction()->getParent()->begin();
@ -135,9 +134,8 @@ getCallSiteDependency(CallSite C, Instruction *start, BasicBlock *block) {
AA.getModRefBehavior(CallSite::get(QI)); AA.getModRefBehavior(CallSite::get(QI));
if (result != AliasAnalysis::DoesNotAccessMemory) { if (result != AliasAnalysis::DoesNotAccessMemory) {
if (!start && !block) { if (!start && !block) {
cachedResult.first = DepResultTy(QI, Normal); cachedResult = DepResultTy(QI, Normal);
cachedResult.second = true; reverseDep[QI].insert(C.getInstruction());
reverseDep[DepResultTy(QI, Normal)].insert(C.getInstruction());
} }
return MemDepResult::get(QI); return MemDepResult::get(QI);
} else { } else {
@ -148,18 +146,15 @@ getCallSiteDependency(CallSite C, Instruction *start, BasicBlock *block) {
if (AA.getModRefInfo(C, pointer, pointerSize) != AliasAnalysis::NoModRef) { if (AA.getModRefInfo(C, pointer, pointerSize) != AliasAnalysis::NoModRef) {
if (!start && !block) { if (!start && !block) {
cachedResult.first = DepResultTy(QI, Normal); cachedResult = DepResultTy(QI, Normal);
cachedResult.second = true; reverseDep[QI].insert(C.getInstruction());
reverseDep[DepResultTy(QI, Normal)].insert(C.getInstruction());
} }
return MemDepResult::get(QI); return MemDepResult::get(QI);
} }
} }
// No dependence found // No dependence found
cachedResult.first = DepResultTy(0, NonLocal); cachedResult = DepResultTy(0, NonLocal);
cachedResult.second = true;
reverseDep[DepResultTy(0, NonLocal)].insert(C.getInstruction());
return MemDepResult::getNonLocal(); return MemDepResult::getNonLocal();
} }
@ -279,7 +274,8 @@ void MemoryDependenceAnalysis::getNonLocalDependency(Instruction* query,
// Update the reverse non-local dependency cache. // Update the reverse non-local dependency cache.
for (DenseMap<BasicBlock*, DepResultTy>::iterator I = cached.begin(), for (DenseMap<BasicBlock*, DepResultTy>::iterator I = cached.begin(),
E = cached.end(); I != E; ++I) { E = cached.end(); I != E; ++I) {
reverseDepNonLocal[I->second].insert(query); if (Instruction *Inst = I->second.getPointer())
reverseDepNonLocal[Inst].insert(query);
resp[I->first] = ConvToResult(I->second); resp[I->first] = ConvToResult(I->second);
} }
@ -296,7 +292,8 @@ void MemoryDependenceAnalysis::getNonLocalDependency(Instruction* query,
for (DenseMap<BasicBlock*, DepResultTy>::iterator I = cached.begin(), for (DenseMap<BasicBlock*, DepResultTy>::iterator I = cached.begin(),
E = cached.end(); I != E; ++I) { E = cached.end(); I != E; ++I) {
// FIXME: Merge with the code above! // FIXME: Merge with the code above!
reverseDepNonLocal[I->second].insert(query); if (Instruction *Inst = I->second.getPointer())
reverseDepNonLocal[Inst].insert(query);
resp[I->first] = ConvToResult(I->second); resp[I->first] = ConvToResult(I->second);
} }
} }
@ -304,6 +301,8 @@ void MemoryDependenceAnalysis::getNonLocalDependency(Instruction* query,
/// getDependency - Return the instruction on which a memory operation /// getDependency - Return the instruction on which a memory operation
/// depends. The local parameter indicates if the query should only /// depends. The local parameter indicates if the query should only
/// evaluate dependencies within the same basic block. /// evaluate dependencies within the same basic block.
/// FIXME: ELIMINATE START/BLOCK and make the caching happen in a higher level
/// METHOD.
MemDepResult MemoryDependenceAnalysis::getDependency(Instruction *query, MemDepResult MemoryDependenceAnalysis::getDependency(Instruction *query,
Instruction *start, Instruction *start,
BasicBlock *block) { BasicBlock *block) {
@ -311,22 +310,20 @@ MemDepResult MemoryDependenceAnalysis::getDependency(Instruction *query,
BasicBlock::iterator QI = query; BasicBlock::iterator QI = query;
// Check for a cached result // Check for a cached result
std::pair<DepResultTy, bool>& cachedResult = LocalDeps[query]; // FIXME: why do this when Block or Start is specified??
// If we have a _confirmed_ cached entry, return it DepResultTy &cachedResult = LocalDeps[query];
if (!block && !start) {
if (cachedResult.second)
return ConvToResult(cachedResult.first);
else if (cachedResult.first.getInt() == Normal &&
cachedResult.first.getPointer())
// If we have an unconfirmed cached entry, we can start our search from
// it.
QI = cachedResult.first.getPointer();
}
if (start) if (start)
QI = start; QI = start;
else if (!start && block) else if (block)
QI = block->end(); QI = block->end();
else if (cachedResult.getInt() != Dirty) {
// If we have a _confirmed_ cached entry, return it.
return ConvToResult(cachedResult);
} else if (Instruction *Inst = cachedResult.getPointer()) {
// If we have an unconfirmed cached entry, we can start our search from it.
QI = Inst;
}
AliasAnalysis& AA = getAnalysis<AliasAnalysis>(); AliasAnalysis& AA = getAnalysis<AliasAnalysis>();
TargetData& TD = getAnalysis<TargetData>(); TargetData& TD = getAnalysis<TargetData>();
@ -372,9 +369,8 @@ MemDepResult MemoryDependenceAnalysis::getDependency(Instruction *query,
// All volatile loads/stores depend on each other // All volatile loads/stores depend on each other
if (queryIsVolatile && S->isVolatile()) { if (queryIsVolatile && S->isVolatile()) {
if (!start && !block) { if (!start && !block) {
cachedResult.first = DepResultTy(S, Normal); cachedResult = DepResultTy(S, Normal);
cachedResult.second = true; reverseDep[S].insert(query);
reverseDep[DepResultTy(S, Normal)].insert(query);
} }
return MemDepResult::get(S); return MemDepResult::get(S);
@ -386,9 +382,8 @@ MemDepResult MemoryDependenceAnalysis::getDependency(Instruction *query,
// All volatile loads/stores depend on each other // All volatile loads/stores depend on each other
if (queryIsVolatile && L->isVolatile()) { if (queryIsVolatile && L->isVolatile()) {
if (!start && !block) { if (!start && !block) {
cachedResult.first = DepResultTy(L, Normal); cachedResult = DepResultTy(L, Normal);
cachedResult.second = true; reverseDep[L].insert(query);
reverseDep[DepResultTy(L, Normal)].insert(query);
} }
return MemDepResult::get(L); return MemDepResult::get(L);
@ -422,9 +417,8 @@ MemDepResult MemoryDependenceAnalysis::getDependency(Instruction *query,
continue; continue;
if (!start && !block) { if (!start && !block) {
cachedResult.first = DepResultTy(QI, Normal); cachedResult = DepResultTy(QI, Normal);
cachedResult.second = true; reverseDep[QI].insert(query);
reverseDep[DepResultTy(QI, Normal)].insert(query);
} }
return MemDepResult::get(QI); return MemDepResult::get(QI);
} else { } else {
@ -444,9 +438,8 @@ MemDepResult MemoryDependenceAnalysis::getDependency(Instruction *query,
continue; continue;
if (!start && !block) { if (!start && !block) {
cachedResult.first = DepResultTy(QI, Normal); cachedResult = DepResultTy(QI, Normal);
cachedResult.second = true; reverseDep[QI].insert(query);
reverseDep[DepResultTy(QI, Normal)].insert(query);
} }
return MemDepResult::get(QI); return MemDepResult::get(QI);
@ -455,11 +448,8 @@ MemDepResult MemoryDependenceAnalysis::getDependency(Instruction *query,
} }
// If we found nothing, return the non-local flag // If we found nothing, return the non-local flag
if (!start && !block) { if (!start && !block)
cachedResult.first = DepResultTy(0, NonLocal); cachedResult = DepResultTy(0, NonLocal);
cachedResult.second = true;
reverseDep[DepResultTy(0, NonLocal)].insert(query);
}
return MemDepResult::getNonLocal(); return MemDepResult::getNonLocal();
} }
@ -470,36 +460,38 @@ MemDepResult MemoryDependenceAnalysis::getDependency(Instruction *query,
void MemoryDependenceAnalysis::dropInstruction(Instruction* drop) { void MemoryDependenceAnalysis::dropInstruction(Instruction* drop) {
LocalDepMapType::iterator depGraphEntry = LocalDeps.find(drop); LocalDepMapType::iterator depGraphEntry = LocalDeps.find(drop);
if (depGraphEntry != LocalDeps.end()) if (depGraphEntry != LocalDeps.end())
reverseDep[depGraphEntry->second.first].erase(drop); if (Instruction *Inst = depGraphEntry->second.getPointer())
reverseDep[Inst].erase(drop);
// Drop dependency information for things that depended on this instr // Drop dependency information for things that depended on this instr
SmallPtrSet<Instruction*, 4>& set = reverseDep[DepResultTy(drop, Normal)]; SmallPtrSet<Instruction*, 4>& set = reverseDep[drop];
for (SmallPtrSet<Instruction*, 4>::iterator I = set.begin(), E = set.end(); for (SmallPtrSet<Instruction*, 4>::iterator I = set.begin(), E = set.end();
I != E; ++I) I != E; ++I)
LocalDeps.erase(*I); LocalDeps.erase(*I);
LocalDeps.erase(drop); LocalDeps.erase(drop);
reverseDep.erase(DepResultTy(drop, Normal)); reverseDep.erase(drop);
for (DenseMap<BasicBlock*, DepResultTy>::iterator DI = for (DenseMap<BasicBlock*, DepResultTy>::iterator DI =
depGraphNonLocal[drop].begin(), DE = depGraphNonLocal[drop].end(); depGraphNonLocal[drop].begin(), DE = depGraphNonLocal[drop].end();
DI != DE; ++DI) DI != DE; ++DI)
if (DI->second.getInt() != None) if (Instruction *Inst = DI->second.getPointer())
reverseDepNonLocal[DI->second].erase(drop); reverseDepNonLocal[Inst].erase(drop);
if (reverseDepNonLocal.count(DepResultTy(drop, Normal))) { if (reverseDepNonLocal.count(drop)) {
SmallPtrSet<Instruction*, 4>& set = SmallPtrSet<Instruction*, 4>& set =
reverseDepNonLocal[DepResultTy(drop, Normal)]; reverseDepNonLocal[drop];
for (SmallPtrSet<Instruction*, 4>::iterator I = set.begin(), E = set.end(); for (SmallPtrSet<Instruction*, 4>::iterator I = set.begin(), E = set.end();
I != E; ++I) I != E; ++I)
for (DenseMap<BasicBlock*, DepResultTy>::iterator DI = for (DenseMap<BasicBlock*, DepResultTy>::iterator DI =
depGraphNonLocal[*I].begin(), DE = depGraphNonLocal[*I].end(); depGraphNonLocal[*I].begin(), DE = depGraphNonLocal[*I].end();
DI != DE; ++DI) DI != DE; ++DI)
if (DI->second == DepResultTy(drop, Normal)) if (DI->second == DepResultTy(drop, Normal))
// FIXME: Why not remember the old insertion point??
DI->second = DepResultTy(0, Dirty); DI->second = DepResultTy(0, Dirty);
} }
reverseDepNonLocal.erase(DepResultTy(drop, Normal)); reverseDepNonLocal.erase(drop);
depGraphNonLocal.erase(drop); depGraphNonLocal.erase(drop);
} }
@ -512,8 +504,8 @@ void MemoryDependenceAnalysis::removeInstruction(Instruction *RemInst) {
for (DenseMap<BasicBlock*, DepResultTy>::iterator DI = for (DenseMap<BasicBlock*, DepResultTy>::iterator DI =
depGraphNonLocal[RemInst].begin(), DE = depGraphNonLocal[RemInst].end(); depGraphNonLocal[RemInst].begin(), DE = depGraphNonLocal[RemInst].end();
DI != DE; ++DI) DI != DE; ++DI)
if (DI->second.getInt() != None) if (Instruction *Inst = DI->second.getPointer())
reverseDepNonLocal[DI->second].erase(RemInst); reverseDepNonLocal[Inst].erase(RemInst);
// Shortly after this, we will look for things that depend on RemInst. In // Shortly after this, we will look for things that depend on RemInst. In
// order to update these, we'll need a new dependency to base them on. We // order to update these, we'll need a new dependency to base them on. We
@ -521,33 +513,31 @@ void MemoryDependenceAnalysis::removeInstruction(Instruction *RemInst) {
// to make a more accurate approximation where possible. Compute that better // to make a more accurate approximation where possible. Compute that better
// approximation if we can. // approximation if we can.
DepResultTy NewDependency; DepResultTy NewDependency;
bool NewDependencyConfirmed = false;
// If we have a cached local dependence query for this instruction, remove it. // If we have a cached local dependence query for this instruction, remove it.
// //
LocalDepMapType::iterator LocalDepEntry = LocalDeps.find(RemInst); LocalDepMapType::iterator LocalDepEntry = LocalDeps.find(RemInst);
if (LocalDepEntry != LocalDeps.end()) { if (LocalDepEntry != LocalDeps.end()) {
DepResultTy LocalDep = LocalDepEntry->second.first; DepResultTy LocalDep = LocalDepEntry->second;
bool IsConfirmed = LocalDepEntry->second.second;
// Remove this local dependency info. // Remove this local dependency info.
LocalDeps.erase(LocalDepEntry); LocalDeps.erase(LocalDepEntry);
// Remove us from DepInst's reverse set now that the local dep info is gone. // Remove us from DepInst's reverse set now that the local dep info is gone.
reverseDep[LocalDep].erase(RemInst); if (Instruction *Inst = LocalDep.getPointer())
reverseDep[Inst].erase(RemInst);
// If we have unconfirmed info, don't trust it. // If we have unconfirmed info, don't trust it.
if (IsConfirmed) { if (LocalDep.getInt() != Dirty) {
// If we have a confirmed non-local flag, use it. // If we have a confirmed non-local flag, use it.
if (LocalDep.getInt() == NonLocal || LocalDep.getInt() == None) { if (LocalDep.getInt() == NonLocal || LocalDep.getInt() == None) {
// The only time this dependency is confirmed is if it is non-local. // The only time this dependency is confirmed is if it is non-local.
NewDependency = LocalDep; NewDependency = LocalDep;
NewDependencyConfirmed = true;
} else { } else {
// If we have dep info for RemInst, set them to it. // If we have dep info for RemInst, set them to it.
Instruction *NDI = next(BasicBlock::iterator(LocalDep.getPointer())); Instruction *NDI = next(BasicBlock::iterator(LocalDep.getPointer()));
if (NDI != RemInst) // Don't use RemInst for the new dependency! if (NDI != RemInst) // Don't use RemInst for the new dependency!
NewDependency = DepResultTy(NDI, Normal); NewDependency = DepResultTy(NDI, Dirty);
} }
} }
} }
@ -556,13 +546,12 @@ void MemoryDependenceAnalysis::removeInstruction(Instruction *RemInst) {
// use the immediate successor of RemInst. We use the successor because // use the immediate successor of RemInst. We use the successor because
// getDependence starts by checking the immediate predecessor of what is in // getDependence starts by checking the immediate predecessor of what is in
// the cache. // the cache.
if (NewDependency == DepResultTy(0, Normal)) if (NewDependency == DepResultTy(0, Dirty))
NewDependency = DepResultTy(next(BasicBlock::iterator(RemInst)), Normal); NewDependency = DepResultTy(next(BasicBlock::iterator(RemInst)), Dirty);
// Loop over all of the things that depend on the instruction we're removing. // Loop over all of the things that depend on the instruction we're removing.
// //
reverseDepMapType::iterator ReverseDepIt = reverseDepMapType::iterator ReverseDepIt = reverseDep.find(RemInst);
reverseDep.find(DepResultTy(RemInst, Normal));
if (ReverseDepIt != reverseDep.end()) { if (ReverseDepIt != reverseDep.end()) {
SmallPtrSet<Instruction*, 4> &ReverseDeps = ReverseDepIt->second; SmallPtrSet<Instruction*, 4> &ReverseDeps = ReverseDepIt->second;
for (SmallPtrSet<Instruction*, 4>::iterator I = ReverseDeps.begin(), for (SmallPtrSet<Instruction*, 4>::iterator I = ReverseDeps.begin(),
@ -574,19 +563,17 @@ void MemoryDependenceAnalysis::removeInstruction(Instruction *RemInst) {
if (InstDependingOnRemInst == RemInst) continue; if (InstDependingOnRemInst == RemInst) continue;
// Insert the new dependencies. // Insert the new dependencies.
LocalDeps[InstDependingOnRemInst] = LocalDeps[InstDependingOnRemInst] = NewDependency;
std::make_pair(NewDependency, NewDependencyConfirmed);
// If our NewDependency is an instruction, make sure to remember that new // If our NewDependency is an instruction, make sure to remember that new
// things depend on it. // things depend on it.
// FIXME: Just insert all deps! if (Instruction *Inst = NewDependency.getPointer())
if (NewDependency.getInt() != NonLocal && NewDependency.getInt() != None) reverseDep[Inst].insert(InstDependingOnRemInst);
reverseDep[NewDependency].insert(InstDependingOnRemInst);
} }
reverseDep.erase(DepResultTy(RemInst, Normal)); reverseDep.erase(RemInst);
} }
ReverseDepIt = reverseDepNonLocal.find(DepResultTy(RemInst, Normal)); ReverseDepIt = reverseDepNonLocal.find(RemInst);
if (ReverseDepIt != reverseDepNonLocal.end()) { if (ReverseDepIt != reverseDepNonLocal.end()) {
SmallPtrSet<Instruction*, 4>& set = ReverseDepIt->second; SmallPtrSet<Instruction*, 4>& set = ReverseDepIt->second;
for (SmallPtrSet<Instruction*, 4>::iterator I = set.begin(), E = set.end(); for (SmallPtrSet<Instruction*, 4>::iterator I = set.begin(), E = set.end();
@ -595,6 +582,7 @@ void MemoryDependenceAnalysis::removeInstruction(Instruction *RemInst) {
depGraphNonLocal[*I].begin(), DE = depGraphNonLocal[*I].end(); depGraphNonLocal[*I].begin(), DE = depGraphNonLocal[*I].end();
DI != DE; ++DI) DI != DE; ++DI)
if (DI->second == DepResultTy(RemInst, Normal)) if (DI->second == DepResultTy(RemInst, Normal))
// FIXME: Why not remember the old insertion point??
DI->second = DepResultTy(0, Dirty); DI->second = DepResultTy(0, Dirty);
reverseDepNonLocal.erase(ReverseDepIt); reverseDepNonLocal.erase(ReverseDepIt);
} }