[llvm] 177c065 - [Attributor] Use a pointer value type for the OpcodeInstMap

[Johannes Doerfert via llvm-commits]

Author: Johannes Doerfert
Date: 2020-04-21T11:20:09-05:00
New Revision: 177c065e5065ef140c1e83ab655c3aad904a3a40

URL: https://github.com/llvm/llvm-project/commit/177c065e5065ef140c1e83ab655c3aad904a3a40
DIFF: https://github.com/llvm/llvm-project/commit/177c065e5065ef140c1e83ab655c3aad904a3a40.diff

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

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: 490390 (320725/s)
temporary memory allocations: 84601 (55330/s)
peak heap memory consumption: 41.70MB
peak RSS (including heaptrack overhead): 131.18MB
total memory leaked: 269.04KB
```

After:
```
calls to allocation functions: 489359 (301144/s)
temporary memory allocations: 82983 (51066/s)
peak heap memory consumption: 36.76MB
peak RSS (including heaptrack overhead): 126.48MB
total memory leaked: 269.04KB
```

Difference:
```
calls to allocation functions: -1031 (-10739/s)
temporary memory allocations: -1618 (-16854/s)
peak heap memory consumption: -4.94MB
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
    llvm/lib/Transforms/IPO/AttributorAttributes.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/Transforms/IPO/Attributor.h b/llvm/include/llvm/Transforms/IPO/Attributor.h
index a82bcfbf11d7..be90b8d1b7fa 100644
--- a/llvm/include/llvm/Transforms/IPO/Attributor.h
+++ b/llvm/include/llvm/Transforms/IPO/Attributor.h
@@ -573,8 +573,11 @@ struct InformationCache {
       It.getSecond()->~FunctionInfo();
   }
 
+  /// A vector type to hold instructions.
+  using InstructionVectorTy = SmallVector<Instruction *, 8>;
+
   /// A map type from opcodes to instructions with this opcode.
-  using OpcodeInstMapTy = DenseMap<unsigned, SmallVector<Instruction *, 32>>;
+  using OpcodeInstMapTy = DenseMap<unsigned, InstructionVectorTy *>;
 
   /// Return the map that relates "interesting" opcodes with all instructions
   /// with that opcode in \p F.
@@ -582,9 +585,6 @@ struct InformationCache {
     return getFunctionInfo(F).OpcodeInstMap;
   }
 
-  /// A vector type to hold instructions.
-  using InstructionVectorTy = SmallVector<Instruction *, 4>;
-
   /// Return the instructions in \p F that may read or write memory.
   InstructionVectorTy &getReadOrWriteInstsForFunction(const Function &F) {
     return getFunctionInfo(F).RWInsts;
@@ -633,6 +633,8 @@ struct InformationCache {
 
 private:
   struct FunctionInfo {
+    ~FunctionInfo();
+
     /// A nested map that remembers all instructions in a function with a
     /// certain instruction opcode (Instruction::getOpcode()).
     OpcodeInstMapTy OpcodeInstMap;

diff  --git a/llvm/lib/Transforms/IPO/Attributor.cpp b/llvm/lib/Transforms/IPO/Attributor.cpp
index 63aa721dc538..0cda3890804c 100644
--- a/llvm/lib/Transforms/IPO/Attributor.cpp
+++ b/llvm/lib/Transforms/IPO/Attributor.cpp
@@ -832,7 +832,12 @@ static bool checkForAllInstructionsImpl(
     const AAIsDead *LivenessAA, const ArrayRef<unsigned> &Opcodes,
     bool CheckBBLivenessOnly = false) {
   for (unsigned Opcode : Opcodes) {
-    for (Instruction *I : OpcodeInstMap[Opcode]) {
+    // Check if we have instructions with this opcode at all first.
+    auto *Insts = OpcodeInstMap.lookup(Opcode);
+    if (!Insts)
+      continue;
+
+    for (Instruction *I : *Insts) {
       // Skip dead instructions.
       if (A && A->isAssumedDead(IRPosition::value(*I), QueryingAA, LivenessAA,
                                 CheckBBLivenessOnly))
@@ -1669,8 +1674,12 @@ void InformationCache::initializeInformationCache(const Function &CF,
       // The alignment of a pointer is interesting for stores.
       IsInterestingOpcode = true;
     }
-    if (IsInterestingOpcode)
-      FI.OpcodeInstMap[I.getOpcode()].push_back(&I);
+    if (IsInterestingOpcode) {
+      auto *&Insts = FI.OpcodeInstMap[I.getOpcode()];
+      if (!Insts)
+        Insts = new (Allocator) InstructionVectorTy();
+      Insts->push_back(&I);
+    }
     if (I.mayReadOrWriteMemory())
       FI.RWInsts.push_back(&I);
   }
@@ -1680,6 +1689,13 @@ void InformationCache::initializeInformationCache(const Function &CF,
     InlineableFunctions.insert(&F);
 }
 
+InformationCache::FunctionInfo::~FunctionInfo() {
+  // The instruction vectors are allocated using a BumpPtrAllocator, we need to
+  // manually destroy them.
+  for (auto &It : OpcodeInstMap)
+    It.getSecond()->~InstructionVectorTy();
+}
+
 void Attributor::recordDependence(const AbstractAttribute &FromAA,
                                   const AbstractAttribute &ToAA,
                                   DepClassTy DepClass) {

diff  --git a/llvm/lib/Transforms/IPO/AttributorAttributes.cpp b/llvm/lib/Transforms/IPO/AttributorAttributes.cpp
index 0556389aaf79..2d63bc830982 100644
--- a/llvm/lib/Transforms/IPO/AttributorAttributes.cpp
+++ b/llvm/lib/Transforms/IPO/AttributorAttributes.cpp
@@ -800,8 +800,9 @@ class AAReturnedValuesImpl : public AAReturnedValues, public AbstractState {
     for (Argument &Arg : F->args()) {
       if (Arg.hasReturnedAttr()) {
         auto &ReturnInstSet = ReturnedValues[&Arg];
-        for (Instruction *RI : OpcodeInstMap[Instruction::Ret])
-          ReturnInstSet.insert(cast<ReturnInst>(RI));
+        if (auto *Insts = OpcodeInstMap.lookup(Instruction::Ret))
+          for (Instruction *RI : *Insts)
+            ReturnInstSet.insert(cast<ReturnInst>(RI));
 
         indicateOptimisticFixpoint();
         return;