[llvm] [ADT][NFC] Refactor/optimize DenseMap::copyFrom (PR #108377)

Marc Auberer via llvm-commits llvm-commits at lists.llvm.org
Thu Sep 12 12:54:55 PDT 2024


https://github.com/marcauberer updated https://github.com/llvm/llvm-project/pull/108377

>From bb5fc2960a28688ef7650d7c95b1c7fb2cebc97b Mon Sep 17 00:00:00 2001
From: Marc Auberer <marc.auberer at sap.com>
Date: Thu, 12 Sep 2024 12:10:11 +0000
Subject: [PATCH 1/3] [ADT][NFC] Refactor/optimize DenseMap::copyFrom

---
 llvm/include/llvm/ADT/DenseMap.h | 27 +++++++++++++++------------
 1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/llvm/include/llvm/ADT/DenseMap.h b/llvm/include/llvm/ADT/DenseMap.h
index 083d5c9388f7c8..8951ecfeb4720d 100644
--- a/llvm/include/llvm/ADT/DenseMap.h
+++ b/llvm/include/llvm/ADT/DenseMap.h
@@ -471,19 +471,22 @@ class DenseMapBase : public DebugEpochBase {
     setNumEntries(other.getNumEntries());
     setNumTombstones(other.getNumTombstones());
 
-    if (std::is_trivially_copyable<KeyT>::value &&
-        std::is_trivially_copyable<ValueT>::value)
-      memcpy(reinterpret_cast<void *>(getBuckets()), other.getBuckets(),
+    BucketT *Buckets = getBuckets();
+    const BucketT *OtherBuckets = other.getBuckets();
+    if constexpr (std::is_trivially_copyable_v<KeyT> &&
+                  std::is_trivially_copyable_v<ValueT>) {
+      memcpy(reinterpret_cast<void *>(Buckets), OtherBuckets,
              getNumBuckets() * sizeof(BucketT));
-    else
-      for (size_t i = 0; i < getNumBuckets(); ++i) {
-        ::new (&getBuckets()[i].getFirst())
-            KeyT(other.getBuckets()[i].getFirst());
-        if (!KeyInfoT::isEqual(getBuckets()[i].getFirst(), getEmptyKey()) &&
-            !KeyInfoT::isEqual(getBuckets()[i].getFirst(), getTombstoneKey()))
-          ::new (&getBuckets()[i].getSecond())
-              ValueT(other.getBuckets()[i].getSecond());
+    } else {
+      const KeyT EmptyKey = getEmptyKey();
+      const KeyT TombstoneKey = getTombstoneKey();
+      for (size_t I = 0; I < getNumBuckets(); ++I) {
+        ::new (&Buckets[I].getFirst()) KeyT(OtherBuckets[I].getFirst());
+        if (!KeyInfoT::isEqual(Buckets[I].getFirst(), EmptyKey) &&
+            !KeyInfoT::isEqual(Buckets[I].getFirst(), TombstoneKey))
+          ::new (&Buckets[I].getSecond()) ValueT(OtherBuckets[I].getSecond());
       }
+    }
   }
 
   static unsigned getHashValue(const KeyT &Val) {
@@ -496,7 +499,7 @@ class DenseMapBase : public DebugEpochBase {
   }
 
   static const KeyT getEmptyKey() {
-    static_assert(std::is_base_of<DenseMapBase, DerivedT>::value,
+    static_assert(std::is_base_of_v<DenseMapBase, DerivedT>,
                   "Must pass the derived type to this template!");
     return KeyInfoT::getEmptyKey();
   }

>From 20b69debf07f2061e0aea1bc8c99477eed86606d Mon Sep 17 00:00:00 2001
From: Marc Auberer <marc.auberer at sap.com>
Date: Thu, 12 Sep 2024 14:50:12 +0000
Subject: [PATCH 2/3] Don't evaluate getNumBuckets() in every loop iteration

---
 llvm/include/llvm/ADT/DenseMap.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/llvm/include/llvm/ADT/DenseMap.h b/llvm/include/llvm/ADT/DenseMap.h
index 8951ecfeb4720d..e41f1aac6c6b3e 100644
--- a/llvm/include/llvm/ADT/DenseMap.h
+++ b/llvm/include/llvm/ADT/DenseMap.h
@@ -480,7 +480,8 @@ class DenseMapBase : public DebugEpochBase {
     } else {
       const KeyT EmptyKey = getEmptyKey();
       const KeyT TombstoneKey = getTombstoneKey();
-      for (size_t I = 0; I < getNumBuckets(); ++I) {
+      const size_t NumBuckets = getNumBuckets();
+      for (size_t I = 0; I < NumBuckets; ++I) {
         ::new (&Buckets[I].getFirst()) KeyT(OtherBuckets[I].getFirst());
         if (!KeyInfoT::isEqual(Buckets[I].getFirst(), EmptyKey) &&
             !KeyInfoT::isEqual(Buckets[I].getFirst(), TombstoneKey))

>From e4b27183b1e05cc407cdf6c8d41012d7a45e5764 Mon Sep 17 00:00:00 2001
From: Marc Auberer <marc.auberer at chillibits.com>
Date: Thu, 12 Sep 2024 21:54:44 +0200
Subject: [PATCH 3/3] Hoist NumBuckets outside of if

---
 llvm/include/llvm/ADT/DenseMap.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/llvm/include/llvm/ADT/DenseMap.h b/llvm/include/llvm/ADT/DenseMap.h
index e41f1aac6c6b3e..8c17d2ac1b8d54 100644
--- a/llvm/include/llvm/ADT/DenseMap.h
+++ b/llvm/include/llvm/ADT/DenseMap.h
@@ -473,14 +473,14 @@ class DenseMapBase : public DebugEpochBase {
 
     BucketT *Buckets = getBuckets();
     const BucketT *OtherBuckets = other.getBuckets();
+    const size_t NumBuckets = getNumBuckets();
     if constexpr (std::is_trivially_copyable_v<KeyT> &&
                   std::is_trivially_copyable_v<ValueT>) {
       memcpy(reinterpret_cast<void *>(Buckets), OtherBuckets,
-             getNumBuckets() * sizeof(BucketT));
+             NumBuckets * sizeof(BucketT));
     } else {
       const KeyT EmptyKey = getEmptyKey();
       const KeyT TombstoneKey = getTombstoneKey();
-      const size_t NumBuckets = getNumBuckets();
       for (size_t I = 0; I < NumBuckets; ++I) {
         ::new (&Buckets[I].getFirst()) KeyT(OtherBuckets[I].getFirst());
         if (!KeyInfoT::isEqual(Buckets[I].getFirst(), EmptyKey) &&



More information about the llvm-commits mailing list