Skip to content

Commit

Permalink
Use a separate valueenumerator for alias cache and memory behavior cache
Browse files Browse the repository at this point in the history
If we use a shared valueenumerator, imagine the case when one of the AAcache or MBcache
is cleared and we clear the valueenumerator.

This could give rise to collisions (false positives) in the not-yet-cleared cache!
  • Loading branch information
trentxintong committed Dec 23, 2015
1 parent b5e4197 commit 17fe37d
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 13 deletions.
22 changes: 15 additions & 7 deletions include/swift/SILOptimizer/Analysis/AliasAnalysis.h
Expand Up @@ -112,12 +112,19 @@ class AliasAnalysis : public SILAnalysis {
/// The computeMemoryBehavior() method uses this map to cache queries.
llvm::DenseMap<MemBehaviorKeyTy, MemoryBehavior> MemoryBehaviorCache;

/// The AliasAnalysis/MemoryBehavior cache can't directly map a pair of
/// ValueBase pointers to alias/memorybehavior results because we'd like to
/// be able to remove deleted pointers without having to scan the whole map.
/// So, instead of storing pointers we map pointers to indices and store the
/// indices.
ValueEnumerator<ValueBase*> ValueBaseToIndex;
/// The AliasAnalysis cache can't directly map a pair of ValueBase pointers
/// to alias results because we'd like to be able to remove deleted pointers
/// without having to scan the whole map. So, instead of storing pointers we
/// map pointers to indices and store the indices.
ValueEnumerator<ValueBase*> AliasValueBaseToIndex;

/// Same as AliasValueBaseToIndex, map a pointer to the indices for
/// MemoryBehaviorCache.
///
/// NOTE: we do not use the same ValueEnumerator for the alias cache,
/// as when either cache is cleared, we can not clear the ValueEnumerator
/// because doing so could give rise to collisions in the other cache.
ValueEnumerator<ValueBase*> MemoryBehaviorValueBaseToIndex;

AliasResult aliasAddressProjection(SILValue V1, SILValue V2,
SILValue O1, SILValue O2);
Expand All @@ -134,7 +141,8 @@ class AliasAnalysis : public SILAnalysis {
// The pointer I is going away. We can't scan the whole cache and remove
// all of the occurrences of the pointer. Instead we remove the pointer
// from the cache the translates pointers to indices.
ValueBaseToIndex.invalidateValue(I);
AliasValueBaseToIndex.invalidateValue(I);
MemoryBehaviorValueBaseToIndex.invalidateValue(I);
}

virtual bool needsNotifications() override { return true; }
Expand Down
6 changes: 3 additions & 3 deletions lib/SILOptimizer/Analysis/AliasAnalysis.cpp
Expand Up @@ -539,7 +539,7 @@ AliasResult AliasAnalysis::alias(SILValue V1, SILValue V2,
// Flush the cache if the size of the cache is too large.
if (AliasCache.size() > AliasAnalysisMaxCacheSize) {
AliasCache.clear();
ValueBaseToIndex.clear();
AliasValueBaseToIndex.clear();
}

// Calculate the aliasing result and store it in the cache.
Expand Down Expand Up @@ -710,8 +710,8 @@ SILAnalysis *swift::createAliasAnalysis(SILModule *M) {

AliasKeyTy AliasAnalysis::toAliasKey(SILValue V1, SILValue V2,
SILType Type1, SILType Type2) {
size_t idx1 = ValueBaseToIndex.getIndex(V1.getDef());
size_t idx2 = ValueBaseToIndex.getIndex(V2.getDef());
size_t idx1 = AliasValueBaseToIndex.getIndex(V1.getDef());
size_t idx2 = AliasValueBaseToIndex.getIndex(V2.getDef());
unsigned R1 = V1.getResultNumber();
unsigned R2 = V2.getResultNumber();
void *t1 = Type1.getOpaqueValue();
Expand Down
6 changes: 3 additions & 3 deletions lib/SILOptimizer/Analysis/MemoryBehavior.cpp
Expand Up @@ -313,7 +313,7 @@ AliasAnalysis::computeMemoryBehavior(SILInstruction *Inst, SILValue V,
// Flush the cache if the size of the cache is too large.
if (MemoryBehaviorCache.size() > MemoryBehaviorAnalysisMaxCacheSize) {
MemoryBehaviorCache.clear();
ValueBaseToIndex.clear();
MemoryBehaviorValueBaseToIndex.clear();
}

// Calculate the aliasing result and store it in the cache.
Expand All @@ -333,8 +333,8 @@ AliasAnalysis::computeMemoryBehaviorInner(SILInstruction *Inst, SILValue V,

MemBehaviorKeyTy AliasAnalysis::toMemoryBehaviorKey(SILValue V1, SILValue V2,
RetainObserveKind M) {
size_t idx1 = ValueBaseToIndex.getIndex(V1.getDef());
size_t idx2 = ValueBaseToIndex.getIndex(V2.getDef());
size_t idx1 = MemoryBehaviorValueBaseToIndex.getIndex(V1.getDef());
size_t idx2 = MemoryBehaviorValueBaseToIndex.getIndex(V2.getDef());
unsigned R2 = V2.getResultNumber();
return {idx1, idx2, R2, M};
}

2 comments on commit 17fe37d

@trentxintong
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nadavrot can you please review this code ?

@nadavrot
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@trentxintong looks good to me!

Please sign in to comment.