[llvm] [ADT] Update hash function of uint64_t for DenseMap (PR #95734)

Ilya Biryukov via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 17 03:16:50 PDT 2024


ilya-biryukov wrote:

Are we worried about implications of changing the hash function for something as widely used as 64 bit ints?
I can see a few potential problems:
- code sensitive to performance of hash function might become slower,
- code relying on existing order in hash tables breaks when the order changes (there shouldn't be code like this, but [Hyrum's law](https://www.hyrumslaw.com/)).
- the hash function we use may have collisions on some important cases (using hash_combine as suggested by others will alleviate this concern, though).

I wonder if this change should be postponed until after Clang 19? Or am I too pessimistic in my predictions and things will just work out?

Also a few open questions:
- The C++ standard returns 64 bit hash values (on 64 bit platforms), maybe we should do the same in LLVM? I suspect the code of our hash tables would work just fine with it (although it might add some overhead if we store precomputed hashes somewhere).
- Are there better hash functions we could use for ints/in general? E.g. I've heard about murmurhash, but I'm not sure if we can or should use it LLVM.

It would be nice to get someone expert in state-of-the-art hash functions and hash tables to review this. 

https://github.com/llvm/llvm-project/pull/95734


More information about the llvm-commits mailing list