[ADT] Add a helper function to create iterators in DenseMap (NFC)

[kazutakahirata]

This patch adds a private helper function, makeInsertIterator, to
encapsulate the logic for creating iterators within functions like
try_emplace and insert.

This refactoring reduces code duplication and improves readability at
the call sites.

[llvmbot]

@llvm/pr-subscribers-llvm-adt

Author: Kazu Hirata (kazutakahirata)

Changes

This patch adds a private helper function, makeInsertIterator, to
encapsulate the logic for creating iterators within functions like
try_emplace and insert.

This refactoring reduces code duplication and improves readability at
the call sites.

---

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

1 Files Affected:

• (modified) llvm/include/llvm/ADT/DenseMap.h (+13-33)

diff --git a/llvm/include/llvm/ADT/DenseMap.h b/llvm/include/llvm/ADT/DenseMap.h
index 156894aa7f0e3..643dee98f0e28 100644
--- a/llvm/include/llvm/ADT/DenseMap.h
+++ b/llvm/include/llvm/ADT/DenseMap.h
@@ -257,22 +257,13 @@ class DenseMapBase : public DebugEpochBase {
   std::pair<iterator, bool> try_emplace(KeyT &&Key, Ts &&...Args) {
     BucketT *TheBucket;
     if (LookupBucketFor(Key, TheBucket))
-      return std::make_pair(makeIterator(TheBucket,
-                                         shouldReverseIterate<KeyT>()
-                                             ? getBuckets()
-                                             : getBucketsEnd(),
-                                         *this, true),
+      return std::make_pair(makeInsertIterator(TheBucket),
                             false); // Already in map.
 
     // Otherwise, insert the new element.
     TheBucket =
         InsertIntoBucket(TheBucket, std::move(Key), std::forward<Ts>(Args)...);
-    return std::make_pair(makeIterator(TheBucket,
-                                       shouldReverseIterate<KeyT>()
-                                           ? getBuckets()
-                                           : getBucketsEnd(),
-                                       *this, true),
-                          true);
+    return std::make_pair(makeInsertIterator(TheBucket), true);
   }
 
   // Inserts key,value pair into the map if the key isn't already in the map.
@@ -282,21 +273,12 @@ class DenseMapBase : public DebugEpochBase {
   std::pair<iterator, bool> try_emplace(const KeyT &Key, Ts &&...Args) {
     BucketT *TheBucket;
     if (LookupBucketFor(Key, TheBucket))
-      return std::make_pair(makeIterator(TheBucket,
-                                         shouldReverseIterate<KeyT>()
-                                             ? getBuckets()
-                                             : getBucketsEnd(),
-                                         *this, true),
+      return std::make_pair(makeInsertIterator(TheBucket),
                             false); // Already in map.
 
     // Otherwise, insert the new element.
     TheBucket = InsertIntoBucket(TheBucket, Key, std::forward<Ts>(Args)...);
-    return std::make_pair(makeIterator(TheBucket,
-                                       shouldReverseIterate<KeyT>()
-                                           ? getBuckets()
-                                           : getBucketsEnd(),
-                                       *this, true),
-                          true);
+    return std::make_pair(makeInsertIterator(TheBucket), true);
   }
 
   /// Alternate version of insert() which allows a different, and possibly
@@ -309,22 +291,13 @@ class DenseMapBase : public DebugEpochBase {
                                       const LookupKeyT &Val) {
     BucketT *TheBucket;
     if (LookupBucketFor(Val, TheBucket))
-      return std::make_pair(makeIterator(TheBucket,
-                                         shouldReverseIterate<KeyT>()
-                                             ? getBuckets()
-                                             : getBucketsEnd(),
-                                         *this, true),
+      return std::make_pair(makeInsertIterator(TheBucket),
                             false); // Already in map.
 
     // Otherwise, insert the new element.
     TheBucket = InsertIntoBucketWithLookup(TheBucket, std::move(KV.first),
                                            std::move(KV.second), Val);
-    return std::make_pair(makeIterator(TheBucket,
-                                       shouldReverseIterate<KeyT>()
-                                           ? getBuckets()
-                                           : getBucketsEnd(),
-                                       *this, true),
-                          true);
+    return std::make_pair(makeInsertIterator(TheBucket), true);
   }
 
   /// insert - Range insertion of pairs.
@@ -545,6 +518,13 @@ class DenseMapBase : public DebugEpochBase {
     return const_iterator(P, E, Epoch, NoAdvance);
   }
 
+  iterator makeInsertIterator(BucketT *TheBucket) {
+    return makeIterator(TheBucket,
+                        shouldReverseIterate<KeyT>() ? getBuckets()
+                                                     : getBucketsEnd(),
+                        *this, true);
+  }
+
   unsigned getNumEntries() const {
     return static_cast<const DerivedT *>(this)->getNumEntries();
   }

[nikic]

FYI this had some peculiar compile-time impact. With GCC as host compiler, it looks like -g builds improve and non--g builds regress: https://llvm-compile-time-tracker.com/compare.php?from=60743358d15f19b89e3ef5daa1b28e7b1bef453e&to=e10b619b12c1b0e14c5868113cee2e4432072037&stat=instructions:u No change for stage2 build.

I guess this perturbed some inlining heuristc. Looks ok overall though.