[clang] [serialization] no transitive decl change (PR #92083)

Ilya Biryukov via cfe-commits cfe-commits at lists.llvm.org
Fri Jun 14 07:03:11 PDT 2024


ilya-biryukov wrote:

I got to the bottom of it. The problem is that our hash function for 64 bit ints is [not very good](https://github.com/llvm/llvm-project/blob/d5297b72aa32ad3a69563a1fcc61294282f0b379/llvm/include/llvm/ADT/DenseMapInfo.h#L140).

It will have a lot of collision when lower 32 bits are the same and the higher 32 bits of a 64 bit value change. Coincidentally, this is quite common in this case because collisions in low bits of `GlobalDeclID` are very much expected.

I tried replacing it with a variant of MurmurHash I found randomly on the internet and performance went back to normal.
@ChuanqiXu9 I think changing the default hash function for ints might have some far-reaching effects for LLVM code using `DenseSet` and would warrant a bigger discussion on what that function should be (I am not an expert in hash functions myself).
I will be happy to start that discussion on Monday, but feel free to jump into it earlier, if you have time.

In the meantime, I would again propose to revert the change until we can fix the hash function.
(Alternatively, we can change the hash function for `GlobalDeclID` specifically to workaround more locally, but we surely want a better hash function for uint64 in general)

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


More information about the cfe-commits mailing list