[llvm-commits] [llvm] r60232 - in /llvm/trunk: include/llvm/Analysis/MemoryDependenceAnalysis.h lib/Analysis/MemoryDependenceAnalysis.cpp
Chris Lattner
sabre at nondot.org
Fri Nov 28 19:22:12 PST 2008
Author: lattner
Date: Fri Nov 28 21:22:12 2008
New Revision: 60232
URL: http://llvm.org/viewvc/llvm-project?rev=60232&view=rev
Log:
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.
Modified:
llvm/trunk/include/llvm/Analysis/MemoryDependenceAnalysis.h
llvm/trunk/lib/Analysis/MemoryDependenceAnalysis.cpp
Modified: llvm/trunk/include/llvm/Analysis/MemoryDependenceAnalysis.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/MemoryDependenceAnalysis.h?rev=60232&r1=60231&r2=60232&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Analysis/MemoryDependenceAnalysis.h (original)
+++ llvm/trunk/include/llvm/Analysis/MemoryDependenceAnalysis.h Fri Nov 28 21:22:12 2008
@@ -83,9 +83,24 @@
/// DepType - This enum is used to indicate what flavor of dependence this
/// is. If the type is Normal, there is an associated instruction pointer.
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
/// of the DepResultTy pair holds the instruction.
- Normal = 0,
+ Normal,
/// None - This dependence type indicates that the query does not depend
/// on any instructions, either because it scanned to the start of the
@@ -96,18 +111,12 @@
/// NonLocal - This marker indicates that the query has no dependency in
/// the specified block. To find out more, the client should query other
/// predecessor blocks.
- NonLocal,
-
- /// Dirty - This is an internal marker indicating that that a cache entry
- /// is dirty.
- Dirty
+ NonLocal
};
typedef PointerIntPair<Instruction*, 2, DepType> DepResultTy;
- // A map from instructions to their dependency, with a boolean
- // flags for whether this mapping is confirmed or not.
- typedef DenseMap<Instruction*,
- std::pair<DepResultTy, bool> > LocalDepMapType;
+ // A map from instructions to their dependency.
+ typedef DenseMap<Instruction*, DepResultTy> LocalDepMapType;
LocalDepMapType LocalDeps;
// A map from instructions to their non-local dependencies.
@@ -118,7 +127,7 @@
// A reverse mapping from dependencies to the dependees. This is
// used when removing instructions to keep the cache coherent.
- typedef DenseMap<DepResultTy,
+ typedef DenseMap<Instruction*,
SmallPtrSet<Instruction*, 4> > reverseDepMapType;
reverseDepMapType reverseDep;
Modified: llvm/trunk/lib/Analysis/MemoryDependenceAnalysis.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/MemoryDependenceAnalysis.cpp?rev=60232&r1=60231&r2=60232&view=diff
==============================================================================
--- llvm/trunk/lib/Analysis/MemoryDependenceAnalysis.cpp (original)
+++ llvm/trunk/lib/Analysis/MemoryDependenceAnalysis.cpp Fri Nov 28 21:22:12 2008
@@ -26,7 +26,6 @@
#include "llvm/Support/CommandLine.h"
#include "llvm/Support/Debug.h"
#include "llvm/Target/TargetData.h"
-
using namespace llvm;
// Control the calculation of non-local dependencies by only examining the
@@ -51,7 +50,7 @@
for (LocalDepMapType::const_iterator I = LocalDeps.begin(),
E = LocalDeps.end(); I != E; ++I) {
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");
}
@@ -89,9 +88,9 @@
/// of a call site.
MemDepResult MemoryDependenceAnalysis::
getCallSiteDependency(CallSite C, Instruction *start, BasicBlock *block) {
- std::pair<DepResultTy, bool> &cachedResult = LocalDeps[C.getInstruction()];
- AliasAnalysis& AA = getAnalysis<AliasAnalysis>();
- TargetData& TD = getAnalysis<TargetData>();
+ DepResultTy &cachedResult = LocalDeps[C.getInstruction()];
+ AliasAnalysis &AA = getAnalysis<AliasAnalysis>();
+ TargetData &TD = getAnalysis<TargetData>();
BasicBlock::iterator blockBegin = C.getInstruction()->getParent()->begin();
BasicBlock::iterator QI = C.getInstruction();
@@ -135,9 +134,8 @@
AA.getModRefBehavior(CallSite::get(QI));
if (result != AliasAnalysis::DoesNotAccessMemory) {
if (!start && !block) {
- cachedResult.first = DepResultTy(QI, Normal);
- cachedResult.second = true;
- reverseDep[DepResultTy(QI, Normal)].insert(C.getInstruction());
+ cachedResult = DepResultTy(QI, Normal);
+ reverseDep[QI].insert(C.getInstruction());
}
return MemDepResult::get(QI);
} else {
@@ -148,18 +146,15 @@
if (AA.getModRefInfo(C, pointer, pointerSize) != AliasAnalysis::NoModRef) {
if (!start && !block) {
- cachedResult.first = DepResultTy(QI, Normal);
- cachedResult.second = true;
- reverseDep[DepResultTy(QI, Normal)].insert(C.getInstruction());
+ cachedResult = DepResultTy(QI, Normal);
+ reverseDep[QI].insert(C.getInstruction());
}
return MemDepResult::get(QI);
}
}
// No dependence found
- cachedResult.first = DepResultTy(0, NonLocal);
- cachedResult.second = true;
- reverseDep[DepResultTy(0, NonLocal)].insert(C.getInstruction());
+ cachedResult = DepResultTy(0, NonLocal);
return MemDepResult::getNonLocal();
}
@@ -279,7 +274,8 @@
// Update the reverse non-local dependency cache.
for (DenseMap<BasicBlock*, DepResultTy>::iterator I = cached.begin(),
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);
}
@@ -296,7 +292,8 @@
for (DenseMap<BasicBlock*, DepResultTy>::iterator I = cached.begin(),
E = cached.end(); I != E; ++I) {
// 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);
}
}
@@ -304,6 +301,8 @@
/// getDependency - Return the instruction on which a memory operation
/// depends. The local parameter indicates if the query should only
/// 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,
Instruction *start,
BasicBlock *block) {
@@ -311,22 +310,20 @@
BasicBlock::iterator QI = query;
// Check for a cached result
- std::pair<DepResultTy, bool>& cachedResult = LocalDeps[query];
- // If we have a _confirmed_ cached entry, return it
- 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();
- }
+ // FIXME: why do this when Block or Start is specified??
+ DepResultTy &cachedResult = LocalDeps[query];
if (start)
QI = start;
- else if (!start && block)
+ else if (block)
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>();
TargetData& TD = getAnalysis<TargetData>();
@@ -372,9 +369,8 @@
// All volatile loads/stores depend on each other
if (queryIsVolatile && S->isVolatile()) {
if (!start && !block) {
- cachedResult.first = DepResultTy(S, Normal);
- cachedResult.second = true;
- reverseDep[DepResultTy(S, Normal)].insert(query);
+ cachedResult = DepResultTy(S, Normal);
+ reverseDep[S].insert(query);
}
return MemDepResult::get(S);
@@ -386,9 +382,8 @@
// All volatile loads/stores depend on each other
if (queryIsVolatile && L->isVolatile()) {
if (!start && !block) {
- cachedResult.first = DepResultTy(L, Normal);
- cachedResult.second = true;
- reverseDep[DepResultTy(L, Normal)].insert(query);
+ cachedResult = DepResultTy(L, Normal);
+ reverseDep[L].insert(query);
}
return MemDepResult::get(L);
@@ -422,9 +417,8 @@
continue;
if (!start && !block) {
- cachedResult.first = DepResultTy(QI, Normal);
- cachedResult.second = true;
- reverseDep[DepResultTy(QI, Normal)].insert(query);
+ cachedResult = DepResultTy(QI, Normal);
+ reverseDep[QI].insert(query);
}
return MemDepResult::get(QI);
} else {
@@ -444,9 +438,8 @@
continue;
if (!start && !block) {
- cachedResult.first = DepResultTy(QI, Normal);
- cachedResult.second = true;
- reverseDep[DepResultTy(QI, Normal)].insert(query);
+ cachedResult = DepResultTy(QI, Normal);
+ reverseDep[QI].insert(query);
}
return MemDepResult::get(QI);
@@ -455,11 +448,8 @@
}
// If we found nothing, return the non-local flag
- if (!start && !block) {
- cachedResult.first = DepResultTy(0, NonLocal);
- cachedResult.second = true;
- reverseDep[DepResultTy(0, NonLocal)].insert(query);
- }
+ if (!start && !block)
+ cachedResult = DepResultTy(0, NonLocal);
return MemDepResult::getNonLocal();
}
@@ -470,36 +460,38 @@
void MemoryDependenceAnalysis::dropInstruction(Instruction* drop) {
LocalDepMapType::iterator depGraphEntry = LocalDeps.find(drop);
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
- 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();
I != E; ++I)
LocalDeps.erase(*I);
LocalDeps.erase(drop);
- reverseDep.erase(DepResultTy(drop, Normal));
+ reverseDep.erase(drop);
for (DenseMap<BasicBlock*, DepResultTy>::iterator DI =
depGraphNonLocal[drop].begin(), DE = depGraphNonLocal[drop].end();
DI != DE; ++DI)
- if (DI->second.getInt() != None)
- reverseDepNonLocal[DI->second].erase(drop);
+ if (Instruction *Inst = DI->second.getPointer())
+ reverseDepNonLocal[Inst].erase(drop);
- if (reverseDepNonLocal.count(DepResultTy(drop, Normal))) {
+ if (reverseDepNonLocal.count(drop)) {
SmallPtrSet<Instruction*, 4>& set =
- reverseDepNonLocal[DepResultTy(drop, Normal)];
+ reverseDepNonLocal[drop];
for (SmallPtrSet<Instruction*, 4>::iterator I = set.begin(), E = set.end();
I != E; ++I)
for (DenseMap<BasicBlock*, DepResultTy>::iterator DI =
depGraphNonLocal[*I].begin(), DE = depGraphNonLocal[*I].end();
DI != DE; ++DI)
if (DI->second == DepResultTy(drop, Normal))
+ // FIXME: Why not remember the old insertion point??
DI->second = DepResultTy(0, Dirty);
}
- reverseDepNonLocal.erase(DepResultTy(drop, Normal));
+ reverseDepNonLocal.erase(drop);
depGraphNonLocal.erase(drop);
}
@@ -512,8 +504,8 @@
for (DenseMap<BasicBlock*, DepResultTy>::iterator DI =
depGraphNonLocal[RemInst].begin(), DE = depGraphNonLocal[RemInst].end();
DI != DE; ++DI)
- if (DI->second.getInt() != None)
- reverseDepNonLocal[DI->second].erase(RemInst);
+ if (Instruction *Inst = DI->second.getPointer())
+ reverseDepNonLocal[Inst].erase(RemInst);
// 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
@@ -521,33 +513,31 @@
// to make a more accurate approximation where possible. Compute that better
// approximation if we can.
DepResultTy NewDependency;
- bool NewDependencyConfirmed = false;
// If we have a cached local dependence query for this instruction, remove it.
//
LocalDepMapType::iterator LocalDepEntry = LocalDeps.find(RemInst);
if (LocalDepEntry != LocalDeps.end()) {
- DepResultTy LocalDep = LocalDepEntry->second.first;
- bool IsConfirmed = LocalDepEntry->second.second;
+ DepResultTy LocalDep = LocalDepEntry->second;
// Remove this local dependency info.
LocalDeps.erase(LocalDepEntry);
// 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 (IsConfirmed) {
+ if (LocalDep.getInt() != Dirty) {
// If we have a confirmed non-local flag, use it.
if (LocalDep.getInt() == NonLocal || LocalDep.getInt() == None) {
// The only time this dependency is confirmed is if it is non-local.
NewDependency = LocalDep;
- NewDependencyConfirmed = true;
} else {
// If we have dep info for RemInst, set them to it.
Instruction *NDI = next(BasicBlock::iterator(LocalDep.getPointer()));
if (NDI != RemInst) // Don't use RemInst for the new dependency!
- NewDependency = DepResultTy(NDI, Normal);
+ NewDependency = DepResultTy(NDI, Dirty);
}
}
}
@@ -556,13 +546,12 @@
// use the immediate successor of RemInst. We use the successor because
// getDependence starts by checking the immediate predecessor of what is in
// the cache.
- if (NewDependency == DepResultTy(0, Normal))
- NewDependency = DepResultTy(next(BasicBlock::iterator(RemInst)), Normal);
+ if (NewDependency == DepResultTy(0, Dirty))
+ NewDependency = DepResultTy(next(BasicBlock::iterator(RemInst)), Dirty);
// Loop over all of the things that depend on the instruction we're removing.
//
- reverseDepMapType::iterator ReverseDepIt =
- reverseDep.find(DepResultTy(RemInst, Normal));
+ reverseDepMapType::iterator ReverseDepIt = reverseDep.find(RemInst);
if (ReverseDepIt != reverseDep.end()) {
SmallPtrSet<Instruction*, 4> &ReverseDeps = ReverseDepIt->second;
for (SmallPtrSet<Instruction*, 4>::iterator I = ReverseDeps.begin(),
@@ -574,19 +563,17 @@
if (InstDependingOnRemInst == RemInst) continue;
// Insert the new dependencies.
- LocalDeps[InstDependingOnRemInst] =
- std::make_pair(NewDependency, NewDependencyConfirmed);
+ LocalDeps[InstDependingOnRemInst] = NewDependency;
// If our NewDependency is an instruction, make sure to remember that new
// things depend on it.
- // FIXME: Just insert all deps!
- if (NewDependency.getInt() != NonLocal && NewDependency.getInt() != None)
- reverseDep[NewDependency].insert(InstDependingOnRemInst);
+ if (Instruction *Inst = NewDependency.getPointer())
+ reverseDep[Inst].insert(InstDependingOnRemInst);
}
- reverseDep.erase(DepResultTy(RemInst, Normal));
+ reverseDep.erase(RemInst);
}
- ReverseDepIt = reverseDepNonLocal.find(DepResultTy(RemInst, Normal));
+ ReverseDepIt = reverseDepNonLocal.find(RemInst);
if (ReverseDepIt != reverseDepNonLocal.end()) {
SmallPtrSet<Instruction*, 4>& set = ReverseDepIt->second;
for (SmallPtrSet<Instruction*, 4>::iterator I = set.begin(), E = set.end();
@@ -595,6 +582,7 @@
depGraphNonLocal[*I].begin(), DE = depGraphNonLocal[*I].end();
DI != DE; ++DI)
if (DI->second == DepResultTy(RemInst, Normal))
+ // FIXME: Why not remember the old insertion point??
DI->second = DepResultTy(0, Dirty);
reverseDepNonLocal.erase(ReverseDepIt);
}
More information about the llvm-commits
mailing list