Skip to content

Conversation

@kazutakahirata
Copy link
Contributor

This patch adds computeNumBuckets, a helper function to compute the
number of buckets.

This is part of the effort outlined in #168255. This makes it easier
to move the core logic of grow() to DenseMapBase::grow().

This patch adds computeNumBuckets, a helper function to compute the
number of buckets.

This is part of the effort outlined in llvm#168255.  This makes it easier
to move the core logic of grow() to DenseMapBase::grow().
@llvmbot
Copy link
Member

llvmbot commented Nov 17, 2025

@llvm/pr-subscribers-llvm-adt

Author: Kazu Hirata (kazutakahirata)

Changes

This patch adds computeNumBuckets, a helper function to compute the
number of buckets.

This is part of the effort outlined in #168255. This makes it easier
to move the core logic of grow() to DenseMapBase::grow().


Full diff: https://github.com/llvm/llvm-project/pull/168301.diff

1 Files Affected:

  • (modified) llvm/include/llvm/ADT/DenseMap.h (+15-5)
diff --git a/llvm/include/llvm/ADT/DenseMap.h b/llvm/include/llvm/ADT/DenseMap.h
index 86592f12ce62c..152b6a237357d 100644
--- a/llvm/include/llvm/ADT/DenseMap.h
+++ b/llvm/include/llvm/ADT/DenseMap.h
@@ -556,7 +556,10 @@ class DenseMapBase : public DebugEpochBase {
     return llvm::make_range(getBuckets(), getBucketsEnd());
   }
 
-  void grow(unsigned AtLeast) { derived().grow(AtLeast); }
+  void grow(unsigned AtLeast) {
+    AtLeast = derived().computeNumBuckets(AtLeast);
+    derived().grow(AtLeast);
+  }
 
   template <typename LookupKeyT>
   BucketT *findBucketForInsertion(const LookupKeyT &Lookup,
@@ -840,8 +843,11 @@ class DenseMap : public DenseMapBase<DenseMap<KeyT, ValueT, KeyInfoT, BucketT>,
     NumBuckets = 0;
   }
 
+  static unsigned computeNumBuckets(unsigned NumBuckets) {
+    return std::max(64u, static_cast<unsigned>(NextPowerOf2(NumBuckets - 1)));
+  }
+
   void grow(unsigned AtLeast) {
-    AtLeast = std::max<unsigned>(64, NextPowerOf2(AtLeast - 1));
     DenseMap Tmp(AtLeast, typename BaseT::ExactBucketCount{});
     Tmp.moveFrom(*this);
     swapImpl(Tmp);
@@ -1106,10 +1112,14 @@ class SmallDenseMap
     new (getLargeRep()) LargeRep{nullptr, 0};
   }
 
-  void grow(unsigned AtLeast) {
-    if (AtLeast > InlineBuckets)
-      AtLeast = std::max<unsigned>(64, NextPowerOf2(AtLeast - 1));
+  static unsigned computeNumBuckets(unsigned NumBuckets) {
+    if (NumBuckets > InlineBuckets)
+      NumBuckets =
+          std::max(64u, static_cast<unsigned>(NextPowerOf2(NumBuckets - 1)));
+    return NumBuckets;
+  }
 
+  void grow(unsigned AtLeast) {
     SmallDenseMap Tmp(AtLeast, typename BaseT::ExactBucketCount{});
     Tmp.moveFrom(*this);
 

Copy link
Contributor

@s-barannikov s-barannikov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Few suggestions

@kazutakahirata kazutakahirata changed the title [ADT] Add computeNumBuckets to DenseMap (NFC) [ADT] Add roundUpNumBuckets to DenseMap (NFC) Nov 17, 2025
Copy link
Contributor

@s-barannikov s-barannikov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kazutakahirata kazutakahirata merged commit 4206558 into llvm:main Nov 17, 2025
10 checks passed
@kazutakahirata kazutakahirata deleted the cleanup_20251116b_ADT_DenseMap_compute branch November 17, 2025 04:34
@aengelke
Copy link
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants