Skip to content

Commit

Permalink
[ADT] Update hash function of uint64_t for DenseMap (llvm#95734)
Browse files Browse the repository at this point in the history
(Background: See the comment:
llvm#92083 (comment))

It looks like the hash function for 64bits integers are not very good:

```
  static unsigned getHashValue(const unsigned long long& Val) {
    return (unsigned)(Val * 37ULL);
  }
```

Since the result is truncated to 32 bits. It looks like the higher 32
bits won't contribute to the result. So that `0x1'00000001` will have
the the same results to `0x2'00000001`, `0x3'00000001`, ...

Then we may meet a lot collisions in such cases. I feel it should
generally good to include higher 32 bits for hashing functions.

Not sure who's the appropriate reviewer, adding some people by
impressions.
  • Loading branch information
ChuanqiXu9 committed Jun 20, 2024
1 parent 57778ec commit ad79a14
Showing 1 changed file with 5 additions and 2 deletions.
7 changes: 5 additions & 2 deletions llvm/include/llvm/ADT/DenseMapInfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,10 @@ template<> struct DenseMapInfo<unsigned long> {
static inline unsigned long getTombstoneKey() { return ~0UL - 1L; }

static unsigned getHashValue(const unsigned long& Val) {
return (unsigned)(Val * 37UL);
if constexpr (sizeof(Val) == 4)
return DenseMapInfo<unsigned>::getHashValue(Val);
else
return detail::combineHashValue(Val >> 32, Val);
}

static bool isEqual(const unsigned long& LHS, const unsigned long& RHS) {
Expand All @@ -153,7 +156,7 @@ template<> struct DenseMapInfo<unsigned long long> {
static inline unsigned long long getTombstoneKey() { return ~0ULL - 1ULL; }

static unsigned getHashValue(const unsigned long long& Val) {
return (unsigned)(Val * 37ULL);
return detail::combineHashValue(Val >> 32, Val);
}

static bool isEqual(const unsigned long long& LHS,
Expand Down

0 comments on commit ad79a14

Please sign in to comment.