[llvm] 939a434 - Revert "Replace the custom linked list in LeaderTableEntry with TinyPtrVector."

Owen Anderson via llvm-commits llvm-commits at lists.llvm.org
Thu May 26 09:50:57 PDT 2022


Author: Owen Anderson
Date: 2022-05-26T09:50:36-07:00
New Revision: 939a43461ba3cd540a049a62a27dab2732bedd64

URL: https://github.com/llvm/llvm-project/commit/939a43461ba3cd540a049a62a27dab2732bedd64
DIFF: https://github.com/llvm/llvm-project/commit/939a43461ba3cd540a049a62a27dab2732bedd64.diff

LOG: Revert "Replace the custom linked list in LeaderTableEntry with TinyPtrVector."

This reverts commit 1e9114984490b83d4665f12a11f84c83f50ca8f0.

Pending further discussion.

Added: 
    

Modified: 
    llvm/include/llvm/Transforms/Scalar/GVN.h
    llvm/lib/Transforms/Scalar/GVN.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/Transforms/Scalar/GVN.h b/llvm/include/llvm/Transforms/Scalar/GVN.h
index 5dc5c4b3feb89..16ab1a4901629 100644
--- a/llvm/include/llvm/Transforms/Scalar/GVN.h
+++ b/llvm/include/llvm/Transforms/Scalar/GVN.h
@@ -19,7 +19,6 @@
 #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"
@@ -233,10 +232,12 @@ 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 {
-    TinyPtrVector<Value *> Val;
-    TinyPtrVector<const BasicBlock *> BB;
+    Value *Val;
+    const BasicBlock *BB;
+    LeaderTableEntry *Next;
   };
   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
@@ -265,31 +266,44 @@ 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.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);
+    if (!Curr.Val) {
+      Curr.Val = V;
+      Curr.BB = BB;
+      return;
     }
+
+    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 &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();
+    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;
       } else {
-        ++VI;
-        ++BI;
+        LeaderTableEntry *Next = Curr->Next;
+        Curr->Val = Next->Val;
+        Curr->BB = Next->BB;
+        Curr->Next = Next->Next;
       }
     }
   }

diff  --git a/llvm/lib/Transforms/Scalar/GVN.cpp b/llvm/lib/Transforms/Scalar/GVN.cpp
index 184374d754637..34c1483a37e04 100644
--- a/llvm/lib/Transforms/Scalar/GVN.cpp
+++ b/llvm/lib/Transforms/Scalar/GVN.cpp
@@ -2105,9 +2105,10 @@ GVNPass::ValueTable::assignExpNewValueNum(Expression &Exp) {
 /// defined in \p BB.
 bool GVNPass::ValueTable::areAllValsInBB(uint32_t Num, const BasicBlock *BB,
                                          GVNPass &Gvn) {
-  const LeaderTableEntry &Entry = Gvn.LeaderTable[Num];
-  return all_of(Entry.BB,
-                [BB](const BasicBlock *EntryBB) { return EntryBB == BB; });
+  LeaderTableEntry *Vals = &Gvn.LeaderTable[Num];
+  while (Vals && Vals->BB == BB)
+    Vals = Vals->Next;
+  return !Vals;
 }
 
 /// Wrap phiTranslateImpl to provide caching functionality.
@@ -2129,11 +2130,12 @@ bool GVNPass::ValueTable::areCallValsEqual(uint32_t Num, uint32_t NewNum,
                                            const BasicBlock *PhiBlock,
                                            GVNPass &Gvn) {
   CallInst *Call = nullptr;
-  const LeaderTableEntry &Entry = Gvn.LeaderTable[Num];
-  for (Value *Val : Entry.Val) {
-    Call = dyn_cast<CallInst>(Val);
+  LeaderTableEntry *Vals = &Gvn.LeaderTable[Num];
+  while (Vals) {
+    Call = dyn_cast<CallInst>(Vals->Val);
     if (Call && Call->getParent() == PhiBlock)
       break;
+    Vals = Vals->Next;
   }
 
   if (AA->doesNotAccessMemory(Call))
@@ -2226,18 +2228,23 @@ 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) {
-  const LeaderTableEntry &Entry = LeaderTable[num];
-  if (Entry.Val.empty())
-    return nullptr;
+  LeaderTableEntry Vals = LeaderTable[num];
+  if (!Vals.Val) return nullptr;
 
   Value *Val = nullptr;
-  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];
+  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;
     }
+
+    Next = Next->Next;
   }
 
   return Val;
@@ -3026,6 +3033,7 @@ void GVNPass::cleanupGlobalSets() {
   VN.clear();
   LeaderTable.clear();
   BlockRPONumber.clear();
+  TableAllocator.Reset();
   ICF->clear();
   InvalidBlockRPONumbers = true;
 }
@@ -3038,10 +3046,12 @@ 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 &Entry = I.second;
-    for (Value *Val : Entry.Val) {
-      (void)Val;
-      assert(Val != Inst && "Inst still in value numbering scope!");
+    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!");
     }
   }
 }


        


More information about the llvm-commits mailing list