Skip to content

Commit

Permalink
[Attributor] Use a pointer value type for the QueryMap
Browse files Browse the repository at this point in the history
This reduces memory consumption and the need to copy complex data
structures repeatedly.

No functional change is intended.

---

Single run of the Attributor module and then CGSCC pass (oldPM)
for SPASS/clause.c (~10k LLVM-IR loc):

Before:
```
calls to allocation functions: 596180 (374484/s)
temporary memory allocations: 84979 (53378/s)
peak heap memory consumption: 52.14MB
peak RSS (including heaptrack overhead): 139.79MB
total memory leaked: 269.04KB
```

After:
```
calls to allocation functions: 489200 (303285/s)
temporary memory allocations: 83406 (51708/s)
peak heap memory consumption: 41.70MB
peak RSS (including heaptrack overhead): 131.76MB
total memory leaked: 269.04KB
```

Difference:
```
calls to allocation functions: -106980 (-5094285/s)
temporary memory allocations: -1573 (-74904/s)
peak heap memory consumption: -10.44MB
peak RSS (including heaptrack overhead): 0B
total memory leaked: 0B

---
  • Loading branch information
jdoerfert committed Apr 21, 2020
1 parent 1f570e0 commit 99662c2
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 22 deletions.
8 changes: 7 additions & 1 deletion llvm/include/llvm/Transforms/IPO/Attributor.h
Expand Up @@ -1235,8 +1235,14 @@ struct Attributor {
/// Set of abstract attributes which were used and which were necessarily
/// required for any potential optimistic state.
SetVector<AbstractAttribute *> RequiredAAs;

/// Clear the sets but keep the allocated storage as it is likely be resued.
void clear() {
OptionalAAs.clear();
RequiredAAs.clear();
}
};
using QueryMapTy = MapVector<const AbstractAttribute *, QueryMapValueTy>;
using QueryMapTy = DenseMap<const AbstractAttribute *, QueryMapValueTy *>;
QueryMapTy QueryMap;
///}

Expand Down
60 changes: 39 additions & 21 deletions llvm/lib/Transforms/IPO/Attributor.cpp
Expand Up @@ -492,6 +492,11 @@ Attributor::~Attributor() {
for (auto &It : AAMap)
It.getSecond()->~Kind2AAMapTy();

// The QueryMapValueTy objects are allocated via a BumpPtrAllocator, we call
// the destructor manually.
for (auto &It : QueryMap)
It.getSecond()->~QueryMapValueTy();

for (auto &It : ArgumentReplacementMap)
DeleteContainerPointers(It.second);
}
Expand Down Expand Up @@ -918,12 +923,17 @@ ChangeStatus Attributor::run() {
// to run updates.
for (unsigned u = 0; u < InvalidAAs.size(); ++u) {
AbstractAttribute *InvalidAA = InvalidAAs[u];
auto &QuerriedAAs = QueryMap[InvalidAA];

// Check the dependences to fast track invalidation.
auto *QuerriedAAs = QueryMap.lookup(InvalidAA);
if (!QuerriedAAs)
continue;

LLVM_DEBUG(dbgs() << "[Attributor] InvalidAA: " << *InvalidAA << " has "
<< QuerriedAAs.RequiredAAs.size() << "/"
<< QuerriedAAs.OptionalAAs.size()
<< QuerriedAAs->RequiredAAs.size() << "/"
<< QuerriedAAs->OptionalAAs.size()
<< " required/optional dependences\n");
for (AbstractAttribute *DepOnInvalidAA : QuerriedAAs.RequiredAAs) {
for (AbstractAttribute *DepOnInvalidAA : QuerriedAAs->RequiredAAs) {
AbstractState &DOIAAState = DepOnInvalidAA->getState();
DOIAAState.indicatePessimisticFixpoint();
++NumAttributesFixedDueToRequiredDependences;
Expand All @@ -934,16 +944,20 @@ ChangeStatus Attributor::run() {
ChangedAAs.push_back(DepOnInvalidAA);
}
if (!RecomputeDependences)
Worklist.insert(QuerriedAAs.OptionalAAs.begin(),
QuerriedAAs.OptionalAAs.end());
Worklist.insert(QuerriedAAs->OptionalAAs.begin(),
QuerriedAAs->OptionalAAs.end());
}

// If dependences (=QueryMap) are recomputed we have to look at all abstract
// attributes again, regardless of what changed in the last iteration.
if (RecomputeDependences) {
LLVM_DEBUG(
dbgs() << "[Attributor] Run all AAs to recompute dependences\n");
QueryMap.clear();
// The query map entries are reused (1) because it is likely a future
// iteration has similar dependences and (2) the QueryMapValueTy is
// allocated via a BumpPtrAllocator and cannot be reused otherwise.
for (auto &It : QueryMap)
It.getSecond()->clear();
ChangedAAs.clear();
Worklist.insert(AllAbstractAttributes.begin(),
AllAbstractAttributes.end());
Expand All @@ -952,11 +966,12 @@ ChangeStatus Attributor::run() {
// Add all abstract attributes that are potentially dependent on one that
// changed to the work list.
for (AbstractAttribute *ChangedAA : ChangedAAs) {
auto &QuerriedAAs = QueryMap[ChangedAA];
Worklist.insert(QuerriedAAs.OptionalAAs.begin(),
QuerriedAAs.OptionalAAs.end());
Worklist.insert(QuerriedAAs.RequiredAAs.begin(),
QuerriedAAs.RequiredAAs.end());
if (auto *QuerriedAAs = QueryMap.lookup(ChangedAA)) {
Worklist.insert(QuerriedAAs->OptionalAAs.begin(),
QuerriedAAs->OptionalAAs.end());
Worklist.insert(QuerriedAAs->RequiredAAs.begin(),
QuerriedAAs->RequiredAAs.end());
}
}

LLVM_DEBUG(dbgs() << "[Attributor] #Iteration: " << IterationCounter
Expand Down Expand Up @@ -1025,11 +1040,12 @@ ChangeStatus Attributor::run() {
NumAttributesTimedOut++;
}

auto &QuerriedAAs = QueryMap[ChangedAA];
ChangedAAs.append(QuerriedAAs.OptionalAAs.begin(),
QuerriedAAs.OptionalAAs.end());
ChangedAAs.append(QuerriedAAs.RequiredAAs.begin(),
QuerriedAAs.RequiredAAs.end());
if (auto *QuerriedAAs = QueryMap.lookup(ChangedAA)) {
ChangedAAs.append(QuerriedAAs->OptionalAAs.begin(),
QuerriedAAs->OptionalAAs.end());
ChangedAAs.append(QuerriedAAs->RequiredAAs.begin(),
QuerriedAAs->RequiredAAs.end());
}
}

LLVM_DEBUG({
Expand Down Expand Up @@ -1670,12 +1686,14 @@ void Attributor::recordDependence(const AbstractAttribute &FromAA,
if (FromAA.getState().isAtFixpoint())
return;

QueryMapValueTy *&DepAAs = QueryMap[&FromAA];
if (!DepAAs)
DepAAs = new (Allocator) QueryMapValueTy();

if (DepClass == DepClassTy::REQUIRED)
QueryMap[&FromAA].RequiredAAs.insert(
const_cast<AbstractAttribute *>(&ToAA));
DepAAs->RequiredAAs.insert(const_cast<AbstractAttribute *>(&ToAA));
else
QueryMap[&FromAA].OptionalAAs.insert(
const_cast<AbstractAttribute *>(&ToAA));
DepAAs->OptionalAAs.insert(const_cast<AbstractAttribute *>(&ToAA));
QueriedNonFixAA = true;
}

Expand Down

0 comments on commit 99662c2

Please sign in to comment.