[llvm] [ADT,CodeGen] Remove stable_hash_combine_string (PR #100668)
Fangrui Song via llvm-commits
llvm-commits at lists.llvm.org
Fri Jul 26 18:56:27 PDT 2024
MaskRay wrote:
> `stable` is designed to be consistent across different versions or builds for safe entity matching. How consistent is `xxh3_64bits` across versions?
>
> This change seems conflicting with my upcoming changes detailed in [b5d7f5f#diff-56094e62599ca13f28f0857547a76c2387917badcb618eda3ee1547c915888ce](https://github.com/llvm/llvm-project/commit/b5d7f5f918ff250763e916865e0ee5bc5c160474#diff-56094e62599ca13f28f0857547a76c2387917badcb618eda3ee1547c915888ce), based on [discourse.llvm.org/t/rfc-enhanced-machine-outliner-part-2-thinlto-nolto/78753](https://discourse.llvm.org/t/rfc-enhanced-machine-outliner-part-2-thinlto-nolto/78753). However, optimistic transformations using stable hashes should not depend solely on hash stability, although it affects efficiency. I believe there's no upstream dependency yet, pending the RFC's implementation. So, I think updating the hash implementation now might be okay if it remains stable across versions, but continuous updates pose a risk as it will impact the effectiveness of these optimizations.
`xxh3_64bits` is a fixed algorithm and the hash values will not change.
To a wide audience, `StableHashing.h` is like a label name that points to the currently used deterministic hash and the underlying implementation might change.
If the proposed optimization has larger reliance on the stability across versions, using `StableHashing.h` is not suitable.
Actually, currently `StableHashing.h` only has one user. I hope that its uses can be eliminated entirely. Then we add a fast `StableHashing.h`, and add some mechanism to prevent reliance, then re add this header file.
https://github.com/llvm/llvm-project/pull/100668
More information about the llvm-commits
mailing list