Skip to content

Conversation

@kazutakahirata
Copy link
Contributor

InsertIntoBucket and InsertIntoBucketWithLookup each has exactly one
caller. This patch inlines them into their respective sole callers,
reducing the line count.

While we are at it, this patch renames InsertIntoBucketImpl to
findBucketForInsertion to better reflect its purpose now that
InsertIntoBucket is being removed.

…ir callers (NFC)

InsertIntoBucket and InsertIntoBucketWithLookup each has exactly one
caller.  This patch inlines them into their respective sole callers,
reducing the line count.

While we are at it, this patch renames InsertIntoBucketImpl to
findBucketForInsertion to better reflect its purpose now that
InsertIntoBucket is being removed.
@llvmbot
Copy link
Member

llvmbot commented Aug 27, 2025

@llvm/pr-subscribers-llvm-adt

Author: Kazu Hirata (kazutakahirata)

Changes

InsertIntoBucket and InsertIntoBucketWithLookup each has exactly one
caller. This patch inlines them into their respective sole callers,
reducing the line count.

While we are at it, this patch renames InsertIntoBucketImpl to
findBucketForInsertion to better reflect its purpose now that
InsertIntoBucket is being removed.


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

1 Files Affected:

  • (modified) llvm/include/llvm/ADT/DenseMap.h (+8-25)
diff --git a/llvm/include/llvm/ADT/DenseMap.h b/llvm/include/llvm/ADT/DenseMap.h
index 2dfd1dabd07f1..c44706a597fa6 100644
--- a/llvm/include/llvm/ADT/DenseMap.h
+++ b/llvm/include/llvm/ADT/DenseMap.h
@@ -279,8 +279,9 @@ class DenseMapBase : public DebugEpochBase {
       return {makeInsertIterator(TheBucket), false}; // Already in map.
 
     // Otherwise, insert the new element.
-    TheBucket = InsertIntoBucketWithLookup(TheBucket, std::move(KV.first),
-                                           std::move(KV.second), Val);
+    TheBucket = findBucketForInsertion(Val, TheBucket);
+    TheBucket->getFirst() = std::move(KV.first);
+    ::new (&TheBucket->getSecond()) ValueT(std::move(KV.second));
     return {makeInsertIterator(TheBucket), true};
   }
 
@@ -482,8 +483,9 @@ class DenseMapBase : public DebugEpochBase {
       return {makeInsertIterator(TheBucket), false}; // Already in the map.
 
     // Otherwise, insert the new element.
-    TheBucket = InsertIntoBucket(TheBucket, std::forward<KeyArgT>(Key),
-                                 std::forward<Ts>(Args)...);
+    TheBucket = findBucketForInsertion(Key, TheBucket);
+    TheBucket->getFirst() = std::forward<KeyArgT>(Key);
+    ::new (&TheBucket->getSecond()) ValueT(std::forward<Ts>(Args)...);
     return {makeInsertIterator(TheBucket), true};
   }
 
@@ -561,28 +563,9 @@ class DenseMapBase : public DebugEpochBase {
 
   void shrink_and_clear() { static_cast<DerivedT *>(this)->shrink_and_clear(); }
 
-  template <typename KeyArg, typename... ValueArgs>
-  BucketT *InsertIntoBucket(BucketT *TheBucket, KeyArg &&Key,
-                            ValueArgs &&...Values) {
-    TheBucket = InsertIntoBucketImpl(Key, TheBucket);
-
-    TheBucket->getFirst() = std::forward<KeyArg>(Key);
-    ::new (&TheBucket->getSecond()) ValueT(std::forward<ValueArgs>(Values)...);
-    return TheBucket;
-  }
-
-  template <typename LookupKeyT>
-  BucketT *InsertIntoBucketWithLookup(BucketT *TheBucket, KeyT &&Key,
-                                      ValueT &&Value, LookupKeyT &Lookup) {
-    TheBucket = InsertIntoBucketImpl(Lookup, TheBucket);
-
-    TheBucket->getFirst() = std::move(Key);
-    ::new (&TheBucket->getSecond()) ValueT(std::move(Value));
-    return TheBucket;
-  }
-
   template <typename LookupKeyT>
-  BucketT *InsertIntoBucketImpl(const LookupKeyT &Lookup, BucketT *TheBucket) {
+  BucketT *findBucketForInsertion(const LookupKeyT &Lookup,
+                                  BucketT *TheBucket) {
     incrementEpoch();
 
     // If the load of the hash table is more than 3/4, or if fewer than 1/8 of

@kazutakahirata kazutakahirata merged commit 85ab209 into llvm:main Aug 27, 2025
11 checks passed
@kazutakahirata kazutakahirata deleted the cleanup_20250826_DenseMap_inline_InsertIntoBucket branch August 27, 2025 15:28
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.

3 participants