[llvm] [ADT] Remove KeyInfoT forwarders in DenseMap.h (NFC) (PR #165102)
Kazu Hirata via llvm-commits
llvm-commits at lists.llvm.org
Sat Oct 25 07:54:20 PDT 2025
https://github.com/kazutakahirata created https://github.com/llvm/llvm-project/pull/165102
This patch removes getEmptyKey, getTombstoneKey, and getHashValue from
DenseMapBase. These forwarder methods do not really encapsulate
KeyInfoT. Many of their callers already mention KeyInfoT::isEqual for
example.
An existing static_assert is moved to another method. Note that it
must live in a method for type completeness reasons.
>From f6363ac3c8279beb723a54738b4a6107336c0da6 Mon Sep 17 00:00:00 2001
From: Kazu Hirata <kazu at google.com>
Date: Sat, 18 Oct 2025 14:24:28 -0700
Subject: [PATCH] [ADT] Remove KeyInfoT forwarders in DenseMap.h (NFC)
This patch removes getEmptyKey, getTombstoneKey, and getHashValue from
DenseMapBase. These forwarder methods do not really encapsulate
KeyInfoT. Many of their callers already mention KeyInfoT::isEqual for
example.
An existing static_assert is moved to another method. Note that it
must live in a method for type completeness reasons.
---
llvm/include/llvm/ADT/DenseMap.h | 56 +++++++++++++-------------------
1 file changed, 23 insertions(+), 33 deletions(-)
diff --git a/llvm/include/llvm/ADT/DenseMap.h b/llvm/include/llvm/ADT/DenseMap.h
index baa91f3a5f533..f2ebe045c8503 100644
--- a/llvm/include/llvm/ADT/DenseMap.h
+++ b/llvm/include/llvm/ADT/DenseMap.h
@@ -130,13 +130,13 @@ class DenseMapBase : public DebugEpochBase {
return;
}
- const KeyT EmptyKey = getEmptyKey();
+ const KeyT EmptyKey = KeyInfoT::getEmptyKey();
if constexpr (std::is_trivially_destructible_v<ValueT>) {
// Use a simpler loop when values don't need destruction.
for (BucketT &B : buckets())
B.getFirst() = EmptyKey;
} else {
- const KeyT TombstoneKey = getTombstoneKey();
+ const KeyT TombstoneKey = KeyInfoT::getTombstoneKey();
unsigned NumEntries = getNumEntries();
for (BucketT &B : buckets()) {
if (!KeyInfoT::isEqual(B.getFirst(), EmptyKey)) {
@@ -314,7 +314,7 @@ class DenseMapBase : public DebugEpochBase {
return false; // not in map.
TheBucket->getSecond().~ValueT();
- TheBucket->getFirst() = getTombstoneKey();
+ TheBucket->getFirst() = KeyInfoT::getTombstoneKey();
decrementNumEntries();
incrementNumTombstones();
return true;
@@ -322,7 +322,7 @@ class DenseMapBase : public DebugEpochBase {
void erase(iterator I) {
BucketT *TheBucket = &*I;
TheBucket->getSecond().~ValueT();
- TheBucket->getFirst() = getTombstoneKey();
+ TheBucket->getFirst() = KeyInfoT::getTombstoneKey();
decrementNumEntries();
incrementNumTombstones();
}
@@ -362,7 +362,8 @@ class DenseMapBase : public DebugEpochBase {
if (getNumBuckets() == 0) // Nothing to do.
return;
- const KeyT EmptyKey = getEmptyKey(), TombstoneKey = getTombstoneKey();
+ const KeyT EmptyKey = KeyInfoT::getEmptyKey();
+ const KeyT TombstoneKey = KeyInfoT::getTombstoneKey();
for (BucketT &B : buckets()) {
if (!KeyInfoT::isEqual(B.getFirst(), EmptyKey) &&
!KeyInfoT::isEqual(B.getFirst(), TombstoneKey))
@@ -372,12 +373,14 @@ class DenseMapBase : public DebugEpochBase {
}
void initEmpty() {
+ static_assert(std::is_base_of_v<DenseMapBase, DerivedT>,
+ "Must pass the derived type to this template!");
setNumEntries(0);
setNumTombstones(0);
assert((getNumBuckets() & (getNumBuckets() - 1)) == 0 &&
"# initial buckets must be a power of two!");
- const KeyT EmptyKey = getEmptyKey();
+ const KeyT EmptyKey = KeyInfoT::getEmptyKey();
for (BucketT &B : buckets())
::new (&B.getFirst()) KeyT(EmptyKey);
}
@@ -397,8 +400,8 @@ class DenseMapBase : public DebugEpochBase {
initEmpty();
// Insert all the old elements.
- const KeyT EmptyKey = getEmptyKey();
- const KeyT TombstoneKey = getTombstoneKey();
+ const KeyT EmptyKey = KeyInfoT::getEmptyKey();
+ const KeyT TombstoneKey = KeyInfoT::getTombstoneKey();
for (BucketT &B : OldBuckets) {
if (!KeyInfoT::isEqual(B.getFirst(), EmptyKey) &&
!KeyInfoT::isEqual(B.getFirst(), TombstoneKey)) {
@@ -435,8 +438,8 @@ class DenseMapBase : public DebugEpochBase {
memcpy(reinterpret_cast<void *>(Buckets), OtherBuckets,
NumBuckets * sizeof(BucketT));
} else {
- const KeyT EmptyKey = getEmptyKey();
- const KeyT TombstoneKey = getTombstoneKey();
+ const KeyT EmptyKey = KeyInfoT::getEmptyKey();
+ const KeyT TombstoneKey = KeyInfoT::getTombstoneKey();
for (size_t I = 0; I < NumBuckets; ++I) {
::new (&Buckets[I].getFirst()) KeyT(OtherBuckets[I].getFirst());
if (!KeyInfoT::isEqual(Buckets[I].getFirst(), EmptyKey) &&
@@ -446,19 +449,6 @@ class DenseMapBase : public DebugEpochBase {
}
}
- template <typename LookupKeyT>
- static unsigned getHashValue(const LookupKeyT &Val) {
- return KeyInfoT::getHashValue(Val);
- }
-
- static const KeyT getEmptyKey() {
- static_assert(std::is_base_of_v<DenseMapBase, DerivedT>,
- "Must pass the derived type to this template!");
- return KeyInfoT::getEmptyKey();
- }
-
- static const KeyT getTombstoneKey() { return KeyInfoT::getTombstoneKey(); }
-
private:
DerivedT &derived() { return *static_cast<DerivedT *>(this); }
const DerivedT &derived() const {
@@ -566,7 +556,7 @@ class DenseMapBase : public DebugEpochBase {
incrementNumEntries();
// If we are writing over a tombstone, remember this.
- const KeyT EmptyKey = getEmptyKey();
+ const KeyT EmptyKey = KeyInfoT::getEmptyKey();
if (!KeyInfoT::isEqual(TheBucket->getFirst(), EmptyKey))
decrementNumTombstones();
@@ -580,8 +570,8 @@ class DenseMapBase : public DebugEpochBase {
if (NumBuckets == 0)
return nullptr;
- const KeyT EmptyKey = getEmptyKey();
- unsigned BucketNo = getHashValue(Val) & (NumBuckets - 1);
+ const KeyT EmptyKey = KeyInfoT::getEmptyKey();
+ unsigned BucketNo = KeyInfoT::getHashValue(Val) & (NumBuckets - 1);
unsigned ProbeAmt = 1;
while (true) {
const BucketT *Bucket = BucketsPtr + BucketNo;
@@ -618,13 +608,13 @@ class DenseMapBase : public DebugEpochBase {
// FoundTombstone - Keep track of whether we find a tombstone while probing.
BucketT *FoundTombstone = nullptr;
- const KeyT EmptyKey = getEmptyKey();
- const KeyT TombstoneKey = getTombstoneKey();
+ const KeyT EmptyKey = KeyInfoT::getEmptyKey();
+ const KeyT TombstoneKey = KeyInfoT::getTombstoneKey();
assert(!KeyInfoT::isEqual(Val, EmptyKey) &&
!KeyInfoT::isEqual(Val, TombstoneKey) &&
"Empty/Tombstone value shouldn't be inserted into map!");
- unsigned BucketNo = getHashValue(Val) & (NumBuckets - 1);
+ unsigned BucketNo = KeyInfoT::getHashValue(Val) & (NumBuckets - 1);
unsigned ProbeAmt = 1;
while (true) {
BucketT *ThisBucket = BucketsPtr + BucketNo;
@@ -934,8 +924,8 @@ class SmallDenseMap
NumEntries = TmpNumEntries;
std::swap(NumTombstones, RHS.NumTombstones);
- const KeyT EmptyKey = this->getEmptyKey();
- const KeyT TombstoneKey = this->getTombstoneKey();
+ const KeyT EmptyKey = KeyInfoT::getEmptyKey();
+ const KeyT TombstoneKey = KeyInfoT::getTombstoneKey();
if (Small && RHS.Small) {
// If we're swapping inline bucket arrays, we have to cope with some of
// the tricky bits of DenseMap's storage system: the buckets are not
@@ -1135,8 +1125,8 @@ class SmallDenseMap
// Loop over the buckets, moving non-empty, non-tombstones into the
// temporary storage. Have the loop move the TmpEnd forward as it goes.
- const KeyT EmptyKey = this->getEmptyKey();
- const KeyT TombstoneKey = this->getTombstoneKey();
+ const KeyT EmptyKey = KeyInfoT::getEmptyKey();
+ const KeyT TombstoneKey = KeyInfoT::getTombstoneKey();
for (BucketT &B : inlineBuckets()) {
if (!KeyInfoT::isEqual(B.getFirst(), EmptyKey) &&
!KeyInfoT::isEqual(B.getFirst(), TombstoneKey)) {
More information about the llvm-commits
mailing list