[llvm] 99662c2 - [Attributor] Use a pointer value type for the QueryMap

[Johannes Doerfert via llvm-commits]

Author: Johannes Doerfert
Date: 2020-04-21T11:20:04-05:00
New Revision: 99662c22cde06c9d778dc23d950598ac47e35c8c

URL: https://github.com/llvm/llvm-project/commit/99662c22cde06c9d778dc23d950598ac47e35c8c
DIFF: https://github.com/llvm/llvm-project/commit/99662c22cde06c9d778dc23d950598ac47e35c8c.diff

LOG: [Attributor] Use a pointer value type for the QueryMap

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

---

Added: 
    

Modified: 
    llvm/include/llvm/Transforms/IPO/Attributor.h
    llvm/lib/Transforms/IPO/Attributor.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/Transforms/IPO/Attributor.h b/llvm/include/llvm/Transforms/IPO/Attributor.h
index 6cf4115fa984..a82bcfbf11d7 100644
--- a/llvm/include/llvm/Transforms/IPO/Attributor.h
+++ b/llvm/include/llvm/Transforms/IPO/Attributor.h
@@ -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;
   ///}
 

diff  --git a/llvm/lib/Transforms/IPO/Attributor.cpp b/llvm/lib/Transforms/IPO/Attributor.cpp
index 1b7fceb19718..63aa721dc538 100644
--- a/llvm/lib/Transforms/IPO/Attributor.cpp
+++ b/llvm/lib/Transforms/IPO/Attributor.cpp
@@ -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);
 }
@@ -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;
@@ -934,8 +944,8 @@ 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
@@ -943,7 +953,11 @@ ChangeStatus Attributor::run() {
     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());
@@ -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
@@ -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({
@@ -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;
 }