[llvm] [ADT] Use StorageT for temporary storage in DenseMapBase::grow (NFC) (PR #228802)
Kazu Hirata via llvm-commits
llvm-commits at lists.llvm.org
Sat Oct 3 18:26:55 PDT 2026
https://github.com/kazutakahirata created https://github.com/llvm/llvm-project/pull/228802
Without this patch, the non-relocatable path of DenseMapBase::grow
rehashes into a temporary DenseMapBase instance:
DenseMapBase Tmp(NumBuckets, ExactBucketCount{});
This patch switches to:
StorageT Tmp;
while converting initEmpty, initWithExactBucketCount, and moveFrom to
static helpers to operate directly on StorageT. This brings the
following benefits:
- grow() no longer calls the destructor for Tmp because StorageT does
not have one. Without this patch, we call Tmp.~DenseMapBase(), which
in turn calls destroyAll() even though the bucket array has no
elements to destruct. Even with inlining enabled, the host compiler
retains the body of destroyAll() because NumBuckets == 0 is set inside
moveFrom, which is marked LLVM_ATTRIBUTE_NOINLINE.
- We get to remove the following helpers as their only purpose is to
facilitate a temporary instance of DenseMapBase during grow():
- DenseMapBase::ExactBucketCount
- DenseMapBase::DenseMapBase(unsigned, ExactBucketCount)
- DenseMapStorage::kill()
- SmallDenseMapStorage::kill()
In a release build of clang, this patch reduces the size of
non-relocatable DenseMapBase::grow instantiations by 40% on average, in
turn reducing the .text section by 0.11% (75.76 MB to 75.67 MB).
Below is the geometric mean of the perf stat -r10 deltas (after 1 warmup
run) across SLPVectorizer.ii, SemaOpenMP.ii, and X86ISelLowering.ii with
clang -O3 -g -fdebug-info-for-profiling -flto=thin:
Metric Geomean Delta
-----------------------------
instructions:u -0.02%
cpu-cycles:u -0.17%
time elapsed -0.10%
Assisted-by: Antigravity
>From 1bbce0c6e9bc0c26627cac026a1020fcada85989 Mon Sep 17 00:00:00 2001
From: Kazu Hirata <kazu at google.com>
Date: Sat, 3 Oct 2026 09:41:55 -0700
Subject: [PATCH] [ADT] Use StorageT for temporary storage in
DenseMapBase::grow (NFC)
Without this patch, the non-relocatable path of DenseMapBase::grow
rehashes into a temporary DenseMapBase instance:
DenseMapBase Tmp(NumBuckets, ExactBucketCount{});
This patch switches to:
StorageT Tmp;
while converting initEmpty, initWithExactBucketCount, and moveFrom to
static helpers to operate directly on StorageT. This brings the
following benefits:
- grow() no longer calls the destructor for Tmp because StorageT does
not have one. Without this patch, we call Tmp.~DenseMapBase(), which
in turn calls destroyAll() even though the bucket array has no
elements to destruct. Even with inlining enabled, the host compiler
retains the body of destroyAll() because NumBuckets == 0 is set inside
moveFrom, which is marked LLVM_ATTRIBUTE_NOINLINE.
- We get to remove the following helpers as their only purpose is to
facilitate a temporary instance of DenseMapBase during grow():
- DenseMapBase::ExactBucketCount
- DenseMapBase::DenseMapBase(unsigned, ExactBucketCount)
- DenseMapStorage::kill()
- SmallDenseMapStorage::kill()
In a release build of clang, this patch reduces the size of
non-relocatable DenseMapBase::grow instantiations by 40% on average, in
turn reducing the .text section by 0.11% (75.76 MB to 75.67 MB).
Below is the geometric mean of the perf stat -r10 deltas (after 1 warmup
run) across SLPVectorizer.ii, SemaOpenMP.ii, and X86ISelLowering.ii with
clang -O3 -g -fdebug-info-for-profiling -flto=thin:
Metric Geomean Delta
-----------------------------
instructions:u -0.02%
cpu-cycles:u -0.17%
time elapsed -0.10%
Assisted-by: Antigravity
---
llvm/include/llvm/ADT/DenseMap.h | 89 +++++++++++++-------------------
1 file changed, 37 insertions(+), 52 deletions(-)
diff --git a/llvm/include/llvm/ADT/DenseMap.h b/llvm/include/llvm/ADT/DenseMap.h
index dc77237f75d93..282ba52ce31c8 100644
--- a/llvm/include/llvm/ADT/DenseMap.h
+++ b/llvm/include/llvm/ADT/DenseMap.h
@@ -269,10 +269,6 @@ template <typename BucketT> class DenseMapStorage {
return true;
}
- // Put the zombie instance in a known good state after a move.
- // deallocateBuckets() already resets to the empty state.
- void kill() { deallocateBuckets(); }
-
static unsigned roundUpNumBuckets(unsigned MinNumBuckets) {
return std::max(64u, MinNumBuckets);
}
@@ -487,13 +483,6 @@ class SmallDenseMapStorage {
return true;
}
- // Put the zombie instance in a known good state after a move.
- void kill() {
- deallocateBuckets();
- Small = false;
- storage.Large = LargeRep{nullptr, nullptr, 0};
- }
-
static unsigned roundUpNumBuckets(unsigned MinNumBuckets) {
if (MinNumBuckets <= InlineBuckets)
return InlineBuckets;
@@ -762,11 +751,11 @@ class DenseMapBase : public DebugEpochBase {
auto [Reallocate, NewNumBuckets] = Storage.planShrinkAndClear();
destroyAll();
if (!Reallocate) {
- initEmpty();
+ initEmpty(Storage);
return;
}
Storage.deallocateBuckets();
- initWithExactBucketCount(NewNumBuckets);
+ initWithExactBucketCount(Storage, NewNumBuckets);
}
/// Return true if the specified key is in the map, false otherwise.
@@ -1002,7 +991,7 @@ class DenseMapBase : public DebugEpochBase {
/// that this number of elements can be inserted in the map without grow().
explicit DenseMapBase(unsigned NumElementsToReserve) {
initWithExactBucketCount(
- getMinBucketToReserveForEntries(NumElementsToReserve));
+ Storage, getMinBucketToReserveForEntries(NumElementsToReserve));
}
DenseMapBase(const DenseMapBase &other) : DenseMapBase() {
@@ -1038,7 +1027,7 @@ class DenseMapBase : public DebugEpochBase {
DenseMapBase &operator=(DenseMapBase &&other) {
this->destroyAll();
Storage.deallocateBuckets();
- this->initWithExactBucketCount(0);
+ initWithExactBucketCount(Storage, 0);
this->swap(other);
return *this;
}
@@ -1054,19 +1043,22 @@ class DenseMapBase : public DebugEpochBase {
private:
StorageT Storage;
- struct ExactBucketCount {};
-
using Rep = llvm::densemap::detail::StorageRep<BucketT>;
- DenseMapBase(unsigned NumBuckets, ExactBucketCount) {
- initWithExactBucketCount(NumBuckets);
+ static void initEmpty(StorageT &S) {
+ S.setNumEntries(0);
+
+ assert((S.getNumBuckets() & (S.getNumBuckets() - 1)) == 0 &&
+ "# initial buckets must be a power of two!");
+ if (S.getNumBuckets())
+ llvm::densemap::detail::clearUsed(S.getUsed(), S.getNumBuckets());
}
- void initWithExactBucketCount(unsigned NewNumBuckets) {
- if (Storage.allocateBuckets(NewNumBuckets))
- initEmpty();
+ static void initWithExactBucketCount(StorageT &S, unsigned NewNumBuckets) {
+ if (S.allocateBuckets(NewNumBuckets))
+ initEmpty(S);
else
- setNumEntries(0);
+ S.setNumEntries(0);
}
void destroyAll() {
@@ -1087,15 +1079,6 @@ class DenseMapBase : public DebugEpochBase {
});
}
- void initEmpty() {
- setNumEntries(0);
-
- assert((getNumBuckets() & (getNumBuckets() - 1)) == 0 &&
- "# initial buckets must be a power of two!");
- if (getNumBuckets())
- llvm::densemap::detail::clearUsed(getUsed(), getNumBuckets());
- }
-
/// Returns the number of buckets to allocate to ensure that the DenseMap can
/// accommodate \p NumEntries without need to grow().
unsigned getMinBucketToReserveForEntries(unsigned NumEntries) {
@@ -1111,27 +1094,28 @@ class DenseMapBase : public DebugEpochBase {
return llvm::densemap::detail::hasherFor<KeyT, KeyInfoT>();
}
- // Move key/value from Other to *this.
- // Other is left in a valid but empty state.
- LLVM_ATTRIBUTE_NOINLINE void moveFrom(DenseMapBase &Other) {
- assert(getNumEntries() == 0 && "moveFrom requires an empty destination");
- BucketT *OtherB = Other.getBuckets();
- UsedT *OtherU = Other.getUsed();
- const unsigned E = Other.getNumBuckets();
- UsedT *U = getUsed();
- BucketT *B = getBuckets();
- const unsigned Mask = getNumBuckets() - 1;
- llvm::densemap::detail::forEachUsed(OtherU, E, [&](unsigned I) {
+ // Move key/value from Src to Dst.
+ // Src is left in a valid but empty state.
+ static LLVM_ATTRIBUTE_NOINLINE void moveFrom(StorageT &Dst, StorageT &Src) {
+ assert(Dst.getNumEntries() == 0 &&
+ "moveFrom requires an empty destination");
+ BucketT *SrcB = Src.getBuckets();
+ UsedT *SrcU = Src.getUsed();
+ const unsigned E = Src.getNumBuckets();
+ UsedT *U = Dst.getUsed();
+ BucketT *B = Dst.getBuckets();
+ const unsigned Mask = Dst.getNumBuckets() - 1;
+ llvm::densemap::detail::forEachUsed(SrcU, E, [&](unsigned I) {
// Find the first empty slot on this key's probe chain; there is no equal
// key in the destination, so nothing to compare against.
- unsigned BucketNo = KeyInfoT::getHashValue(OtherB[I].getFirst()) & Mask;
+ unsigned BucketNo = KeyInfoT::getHashValue(SrcB[I].getFirst()) & Mask;
while (llvm::densemap::detail::used(U, BucketNo))
BucketNo = (BucketNo + 1) & Mask;
- llvm::densemap::detail::relocateBucket(B + BucketNo, &OtherB[I]);
+ llvm::densemap::detail::relocateBucket(B + BucketNo, &SrcB[I]);
llvm::densemap::detail::setUsed(U, BucketNo);
});
- setNumEntries(Other.getNumEntries());
- Other.Storage.kill();
+ Dst.setNumEntries(Src.getNumEntries());
+ Src.deallocateBuckets();
}
LLVM_ATTRIBUTE_NOINLINE void copyFrom(const DenseMapBase &other) {
@@ -1253,12 +1237,13 @@ class DenseMapBase : public DebugEpochBase {
Storage.grow(MinNumBuckets, hasher());
} else {
unsigned NumBuckets = StorageT::roundUpNumBuckets(MinNumBuckets);
- DenseMapBase Tmp(NumBuckets, ExactBucketCount{});
- Tmp.moveFrom(*this);
- if (Storage.maybeMoveFast(std::move(Tmp.Storage)))
+ StorageT Tmp;
+ initWithExactBucketCount(Tmp, NumBuckets);
+ moveFrom(Tmp, Storage);
+ if (Storage.maybeMoveFast(std::move(Tmp)))
return;
- initWithExactBucketCount(NumBuckets);
- moveFrom(Tmp);
+ initWithExactBucketCount(Storage, NumBuckets);
+ moveFrom(Storage, Tmp);
}
}
More information about the llvm-commits
mailing list