[llvm] ff22501 - [NFC][llvm][StringMap]Extract createTable and getHashTable functions and add the inline attribute to the getMinBucketToReserveForEntries function.

[Shengchen Kan via llvm-commits]

Author: wangyihan
Date: 2022-03-23T10:09:59+08:00
New Revision: ff225019f015b12b31f657b98cfcf0684ab446f8

URL: https://github.com/llvm/llvm-project/commit/ff225019f015b12b31f657b98cfcf0684ab446f8
DIFF: https://github.com/llvm/llvm-project/commit/ff225019f015b12b31f657b98cfcf0684ab446f8.diff

LOG: [NFC][llvm][StringMap]Extract createTable and getHashTable functions and add the inline attribute to the getMinBucketToReserveForEntries function.

1. Extract createTable and getHashTable functions.
2. Add the inline attribute to the getMinBucketToReserveForEntries function.
3. Remove unnecessary local variable HTSize.

Statements in the following order appear in  llvm::StringMapImpl::init and llvm::StringMapImpl::RehashTable, so I extracted this code into a function. getHashTable is for the same reason, it appears in  llvm::StringMapImpl::FindKey,  llvm::StringMapImpl::LookupBucketFor and llvm::StringMapImpl::RehashTable.

```
auto **Table = static_cast<StringMapEntryBase **>(safe_calloc(
      NewNumBuckets + 1, sizeof(StringMapEntryBase **) + sizeof(unsigned)));

  // Allocate one extra bucket, set it to look filled so the iterators stop at
  // end.
  Table[NewNumBuckets] = (StringMapEntryBase *)2;
```

```
unsigned *HashTable = (unsigned *)(TheTable + NumBuckets + 1);
```

Reviewed By: skan, sepavloff

Differential Revision: https://reviews.llvm.org/D121934

Added: 
    

Modified: 
    llvm/lib/Support/StringMap.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Support/StringMap.cpp b/llvm/lib/Support/StringMap.cpp
index 531376ae12a88..9b2f96fca2cd0 100644
--- a/llvm/lib/Support/StringMap.cpp
+++ b/llvm/lib/Support/StringMap.cpp
@@ -18,7 +18,7 @@ using namespace llvm;
 
 /// Returns the number of buckets to allocate to ensure that the DenseMap can
 /// accommodate \p NumEntries without need to grow().
-static unsigned getMinBucketToReserveForEntries(unsigned NumEntries) {
+static inline unsigned getMinBucketToReserveForEntries(unsigned NumEntries) {
   // Ensure that "NumEntries * 4 < NumBuckets * 3"
   if (NumEntries == 0)
     return 0;
@@ -27,6 +27,21 @@ static unsigned getMinBucketToReserveForEntries(unsigned NumEntries) {
   return NextPowerOf2(NumEntries * 4 / 3 + 1);
 }
 
+static inline StringMapEntryBase **createTable(unsigned NewNumBuckets) {
+  auto **Table = static_cast<StringMapEntryBase **>(safe_calloc(
+      NewNumBuckets + 1, sizeof(StringMapEntryBase **) + sizeof(unsigned)));
+
+  // Allocate one extra bucket, set it to look filled so the iterators stop at
+  // end.
+  Table[NewNumBuckets] = (StringMapEntryBase *)2;
+  return Table;
+}
+
+static inline unsigned *getHashTable(StringMapEntryBase **TheTable,
+                                     unsigned NumBuckets) {
+  return reinterpret_cast<unsigned *>(TheTable + NumBuckets + 1);
+}
+
 StringMapImpl::StringMapImpl(unsigned InitSize, unsigned itemSize) {
   ItemSize = itemSize;
 
@@ -54,15 +69,10 @@ void StringMapImpl::init(unsigned InitSize) {
   NumItems = 0;
   NumTombstones = 0;
 
-  TheTable = static_cast<StringMapEntryBase **>(safe_calloc(
-      NewNumBuckets + 1, sizeof(StringMapEntryBase **) + sizeof(unsigned)));
+  TheTable = createTable(NewNumBuckets);
 
   // Set the member only if TheTable was successfully allocated
   NumBuckets = NewNumBuckets;
-
-  // Allocate one extra bucket, set it to look filled so the iterators stop at
-  // end.
-  TheTable[NumBuckets] = (StringMapEntryBase *)2;
 }
 
 /// LookupBucketFor - Look up the bucket that the specified string should end
@@ -71,14 +81,12 @@ void StringMapImpl::init(unsigned InitSize) {
 /// case, the FullHashValue field of the bucket will be set to the hash value
 /// of the string.
 unsigned StringMapImpl::LookupBucketFor(StringRef Name) {
-  unsigned HTSize = NumBuckets;
-  if (HTSize == 0) { // Hash table unallocated so far?
+  // Hash table unallocated so far?
+  if (NumBuckets == 0)
     init(16);
-    HTSize = NumBuckets;
-  }
   unsigned FullHashValue = djbHash(Name, 0);
-  unsigned BucketNo = FullHashValue & (HTSize - 1);
-  unsigned *HashTable = (unsigned *)(TheTable + NumBuckets + 1);
+  unsigned BucketNo = FullHashValue & (NumBuckets - 1);
+  unsigned *HashTable = getHashTable(TheTable, NumBuckets);
 
   unsigned ProbeAmt = 1;
   int FirstTombstone = -1;
@@ -117,7 +125,7 @@ unsigned StringMapImpl::LookupBucketFor(StringRef Name) {
     }
 
     // Okay, we didn't find the item.  Probe to the next bucket.
-    BucketNo = (BucketNo + ProbeAmt) & (HTSize - 1);
+    BucketNo = (BucketNo + ProbeAmt) & (NumBuckets - 1);
 
     // Use quadratic probing, it has fewer clumping artifacts than linear
     // probing and has good cache behavior in the common case.
@@ -129,12 +137,11 @@ unsigned StringMapImpl::LookupBucketFor(StringRef Name) {
 /// in the map, return the bucket number of the key.  Otherwise return -1.
 /// This does not modify the map.
 int StringMapImpl::FindKey(StringRef Key) const {
-  unsigned HTSize = NumBuckets;
-  if (HTSize == 0)
+  if (NumBuckets == 0)
     return -1; // Really empty table?
   unsigned FullHashValue = djbHash(Key, 0);
-  unsigned BucketNo = FullHashValue & (HTSize - 1);
-  unsigned *HashTable = (unsigned *)(TheTable + NumBuckets + 1);
+  unsigned BucketNo = FullHashValue & (NumBuckets - 1);
+  unsigned *HashTable = getHashTable(TheTable, NumBuckets);
 
   unsigned ProbeAmt = 1;
   while (true) {
@@ -161,7 +168,7 @@ int StringMapImpl::FindKey(StringRef Key) const {
     }
 
     // Okay, we didn't find the item.  Probe to the next bucket.
-    BucketNo = (BucketNo + ProbeAmt) & (HTSize - 1);
+    BucketNo = (BucketNo + ProbeAmt) & (NumBuckets - 1);
 
     // Use quadratic probing, it has fewer clumping artifacts than linear
     // probing and has good cache behavior in the common case.
@@ -198,8 +205,6 @@ StringMapEntryBase *StringMapImpl::RemoveKey(StringRef Key) {
 /// the appropriate mod-of-hashtable-size.
 unsigned StringMapImpl::RehashTable(unsigned BucketNo) {
   unsigned NewSize;
-  unsigned *HashTable = (unsigned *)(TheTable + NumBuckets + 1);
-
   // If the hash table is now more than 3/4 full, or if fewer than 1/8 of
   // the buckets are empty (meaning that many are filled with tombstones),
   // grow/rehash the table.
@@ -213,13 +218,9 @@ unsigned StringMapImpl::RehashTable(unsigned BucketNo) {
   }
 
   unsigned NewBucketNo = BucketNo;
-  // Allocate one extra bucket which will always be non-empty.  This allows the
-  // iterators to stop at end.
-  auto NewTableArray = static_cast<StringMapEntryBase **>(safe_calloc(
-      NewSize + 1, sizeof(StringMapEntryBase *) + sizeof(unsigned)));
-
-  unsigned *NewHashArray = (unsigned *)(NewTableArray + NewSize + 1);
-  NewTableArray[NewSize] = (StringMapEntryBase *)2;
+  auto **NewTableArray = createTable(NewSize);
+  unsigned *NewHashArray = getHashTable(NewTableArray, NewSize);
+  unsigned *HashTable = getHashTable(TheTable, NumBuckets);
 
   // Rehash all the items into their new buckets.  Luckily :) we already have
   // the hash values available, so we don't have to rehash any strings.