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

Fangrui Song via llvm-commits llvm-commits at lists.llvm.org
Tue Jun 18 11:56:06 PDT 2024


MaskRay wrote:

> Compile-time for the PR as proposed: http://llvm-compile-time-tracker.com/compare.php?from=3ca17443ef4af21bdb1f3b4fbcfff672cbc6176c&to=9cd0b838265006ff699153bfbb1a1a39ebfb9cdd&stat=instructions:u
> 
> Compile-time using xor mixing (that is, [820edb5](https://github.com/llvm/llvm-project/commit/820edb52ce1abc090f41c5210dcb04edb6203f36) applied on top): http://llvm-compile-time-tracker.com/compare.php?from=3ca17443ef4af21bdb1f3b4fbcfff672cbc6176c&to=820edb52ce1abc090f41c5210dcb04edb6203f36&stat=instructions%3Au
> 
> It doesn't seem like we get any benefit out of the better mixing using combineHashValue() for average-case compilation, so I think you should stick to your first variant using xor.

People use metaheuristics to find good bit mixer functions that achieve good ratings on some metrics (e.g. PractRand)
https://jonkagstrom.com/bit-mixer-construction/ 

`combineHashValue` is a custom bit mixer from 2008 (5fc8ab6d187aefbf1d2cbd36e191e675b14db8f6) that is probably not good. I propose to change it in #95970

This patch can still use `combineHashValue` , but probably change `combineHashValue((uint32_t)x, x>>32)` to `combineHashValue(x>>32, (uint32_t)x)` to avoid a ROR operation.

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


More information about the llvm-commits mailing list