Replace the custom linked list in LeaderTableEntry with TinyPtrVector.

The purpose of the custom linked list was to optimize for the case
of a single-element list. It turns out that TinyPtrVector handles
the same basic scenario even better, reducing the size of
LeaderTableEntry by 33%, and requiring only log2(N) allocations
as the size of the list grows. The only downside is that we have
to store the Value's and BasicBlock's in separate vectors, which
is slightly awkward in a few cases. Fortunately that ends up being
entirely encapsulated inside helper functions.

Reviewed By: asbirlea

Differential Revision: https://reviews.llvm.org/D125205

Author: resistor

llvm/include/llvm/Transforms/Scalar/GVN.h

@@ -19,6 +19,7 @@
 #include "llvm/ADT/MapVector.h"
 #include "llvm/ADT/SetVector.h"
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/TinyPtrVector.h"
 #include "llvm/IR/Dominators.h"
 #include "llvm/IR/InstrTypes.h"
 #include "llvm/IR/PassManager.h"
@@ -232,12 +233,10 @@ class GVNPass : public PassInfoMixin<GVNPass> {
   /// A mapping from value numbers to lists of Value*'s that
   /// have that value number.  Use findLeader to query it.
   struct LeaderTableEntry {
-    Value *Val;
-    const BasicBlock *BB;
-    LeaderTableEntry *Next;
+    TinyPtrVector<Value *> Val;
+    TinyPtrVector<const BasicBlock *> BB;
   };
   DenseMap<uint32_t, LeaderTableEntry> LeaderTable;
-  BumpPtrAllocator TableAllocator;
 
   // Block-local map of equivalent values to their leader, does not
   // propagate to any successors. Entries added mid-block are applied
@@ -266,44 +265,31 @@ class GVNPass : public PassInfoMixin<GVNPass> {
   /// Push a new Value to the LeaderTable onto the list for its value number.
   void addToLeaderTable(uint32_t N, Value *V, const BasicBlock *BB) {
     LeaderTableEntry &Curr = LeaderTable[N];
-    if (!Curr.Val) {
-      Curr.Val = V;
-      Curr.BB = BB;
-      return;
+    if (Curr.Val.size() == 0) {
+      Curr.Val.push_back(V);
+      Curr.BB.push_back(BB);
+    } else {
+      Curr.Val.insert(Curr.Val.begin()+1, V);
+      Curr.BB.insert(Curr.BB.begin()+1, BB);
     }
-
-    LeaderTableEntry *Node = TableAllocator.Allocate<LeaderTableEntry>();
-    Node->Val = V;
-    Node->BB = BB;
-    Node->Next = Curr.Next;
-    Curr.Next = Node;
   }
 
   /// Scan the list of values corresponding to a given
   /// value number, and remove the given instruction if encountered.
   void removeFromLeaderTable(uint32_t N, Instruction *I, BasicBlock *BB) {
-    LeaderTableEntry *Prev = nullptr;
-    LeaderTableEntry *Curr = &LeaderTable[N];
-
-    while (Curr && (Curr->Val != I || Curr->BB != BB)) {
-      Prev = Curr;
-      Curr = Curr->Next;
-    }
-
-    if (!Curr)
-      return;
-
-    if (Prev) {
-      Prev->Next = Curr->Next;
-    } else {
-      if (!Curr->Next) {
-        Curr->Val = nullptr;
-        Curr->BB = nullptr;
+    LeaderTableEntry &entry = LeaderTable[N];
+    assert(entry.BB.size() == entry.Val.size());
+    auto VI = entry.Val.begin();
+    auto VE = entry.Val.end();
+    auto BI = entry.BB.begin();
+    while (VI != VE) {
+      if (*VI == I && *BI == BB) {
+        VI = entry.Val.erase(VI);
+        BI = entry.BB.erase(BI);
+        VE = entry.Val.end();
       } else {
-        LeaderTableEntry *Next = Curr->Next;
-        Curr->Val = Next->Val;
-        Curr->BB = Next->BB;
-        Curr->Next = Next->Next;
+        ++VI;
+        ++BI;
       }
     }
   }

llvm/lib/Transforms/Scalar/GVN.cpp

@@ -2105,10 +2105,9 @@ GVNPass::ValueTable::assignExpNewValueNum(Expression &Exp) {
 /// defined in \p BB.
 bool GVNPass::ValueTable::areAllValsInBB(uint32_t Num, const BasicBlock *BB,
                                          GVNPass &Gvn) {
-  LeaderTableEntry *Vals = &Gvn.LeaderTable[Num];
-  while (Vals && Vals->BB == BB)
-    Vals = Vals->Next;
-  return !Vals;
+  const LeaderTableEntry &Entry = Gvn.LeaderTable[Num];
+  return all_of(Entry.BB,
+                [BB](const BasicBlock *EntryBB) { return EntryBB == BB; });
 }
 
 /// Wrap phiTranslateImpl to provide caching functionality.
@@ -2130,12 +2129,11 @@ bool GVNPass::ValueTable::areCallValsEqual(uint32_t Num, uint32_t NewNum,
                                            const BasicBlock *PhiBlock,
                                            GVNPass &Gvn) {
   CallInst *Call = nullptr;
-  LeaderTableEntry *Vals = &Gvn.LeaderTable[Num];
-  while (Vals) {
-    Call = dyn_cast<CallInst>(Vals->Val);
+  const LeaderTableEntry &Entry = Gvn.LeaderTable[Num];
+  for (Value *Val : Entry.Val) {
+    Call = dyn_cast<CallInst>(Val);
     if (Call && Call->getParent() == PhiBlock)
       break;
-    Vals = Vals->Next;
   }
 
   if (AA->doesNotAccessMemory(Call))
@@ -2228,23 +2226,18 @@ void GVNPass::ValueTable::eraseTranslateCacheEntry(
 // question.  This is fast because dominator tree queries consist of only
 // a few comparisons of DFS numbers.
 Value *GVNPass::findLeader(const BasicBlock *BB, uint32_t num) {
-  LeaderTableEntry Vals = LeaderTable[num];
-  if (!Vals.Val) return nullptr;
+  const LeaderTableEntry &Entry = LeaderTable[num];
+  if (Entry.Val.empty())
+    return nullptr;
 
   Value *Val = nullptr;
-  if (DT->dominates(Vals.BB, BB)) {
-    Val = Vals.Val;
-    if (isa<Constant>(Val)) return Val;
-  }
-
-  LeaderTableEntry* Next = Vals.Next;
-  while (Next) {
-    if (DT->dominates(Next->BB, BB)) {
-      if (isa<Constant>(Next->Val)) return Next->Val;
-      if (!Val) Val = Next->Val;
+  for (size_t i = 0, e = Entry.Val.size(); i != e; ++i) {
+    if (DT->dominates(Entry.BB[i], BB)) {
+      if (isa<Constant>(Entry.Val[i]))
+        return Entry.Val[i];
+      if (!Val)
+        Val = Entry.Val[i];
     }
-
-    Next = Next->Next;
   }
 
   return Val;
@@ -3033,7 +3026,6 @@ void GVNPass::cleanupGlobalSets() {
   VN.clear();
   LeaderTable.clear();
   BlockRPONumber.clear();
-  TableAllocator.Reset();
   ICF->clear();
   InvalidBlockRPONumbers = true;
 }
@@ -3046,12 +3038,10 @@ void GVNPass::verifyRemoved(const Instruction *Inst) const {
   // Walk through the value number scope to make sure the instruction isn't
   // ferreted away in it.
   for (const auto &I : LeaderTable) {
-    const LeaderTableEntry *Node = &I.second;
-    assert(Node->Val != Inst && "Inst still in value numbering scope!");
-
-    while (Node->Next) {
-      Node = Node->Next;
-      assert(Node->Val != Inst && "Inst still in value numbering scope!");
+    const LeaderTableEntry &Entry = I.second;
+    for (Value *Val : Entry.Val) {
+      (void)Val;
+      assert(Val != Inst && "Inst still in value numbering scope!");
     }
   }
 }