[llvm] 1e91149 - Replace the custom linked list in LeaderTableEntry with TinyPtrVector.
Owen Anderson via llvm-commits
llvm-commits at lists.llvm.org
Wed May 25 23:52:50 PDT 2022
Author: Owen Anderson
Date: 2022-05-25T23:52:44-07:00
New Revision: 1e9114984490b83d4665f12a11f84c83f50ca8f0
URL: https://github.com/llvm/llvm-project/commit/1e9114984490b83d4665f12a11f84c83f50ca8f0
DIFF: https://github.com/llvm/llvm-project/commit/1e9114984490b83d4665f12a11f84c83f50ca8f0.diff
LOG: 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
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 16ab1a4901629..5dc5c4b3feb89 100644
--- a/llvm/include/llvm/Transforms/Scalar/GVN.h
+++ b/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;
}
}
}
diff --git a/llvm/lib/Transforms/Scalar/GVN.cpp b/llvm/lib/Transforms/Scalar/GVN.cpp
index 34c1483a37e04..184374d754637 100644
--- a/llvm/lib/Transforms/Scalar/GVN.cpp
+++ b/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!");
}
}
}
More information about the llvm-commits
mailing list