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

[Nikita Popov via llvm-commits]

nikic wrote:

> Are we worried about implications of changing the hash function for something as widely used as 64 bit ints?

I don't think there's particular cause to worry.

> I can see a few potential problems:
> 
>     * code sensitive to performance of hash function might become slower,

We would of course confirm this first -- the patch as-is does not have any impact on compile-time, but I think this is mostly because it modifies the wrong overload. I can re-test this after the patch is updated.

>     * 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/)).

There may be some test fallout to deal with, but I think we should be in a fairly good position thanks to reverse-iteration testing.

> 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).

We currently only use the low bits of the hash, so extending it to 64 bits is not useful at present.

>     * 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.

LLVM is definitely behind the state of the art -- if someone finds the time to port DenseMap and SmallPtrSet to something like swiss tables, that would be great. (And not just for performance, we could also get rid of the need to specify explicit tombstone and empty keys for types.)

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