[PATCH] D102943: Hashing: use a 64-bit storage type on all platforms.

[Alexandre Rames via Phabricator via cfe-commits]

arames added a comment.

In D102943#2775131 <https://reviews.llvm.org/D102943#2775131>, @dexonsmith wrote:

> In D102943#2775115 <https://reviews.llvm.org/D102943#2775115>, @pcc wrote:
>
>> Isn't the bug here that module hashing is using `hash_code`? So shouldn't the correct fix be to use a specific hashing algorithm for module hashes?
>
> Yeah, I tend to agree. I thought modules already used MD5, but maybe just for the AST signature.

For reference, the issue I am trying to fix stems from the hash computed as part of CompilerInvocation::getModuleHash() <https://github.com/llvm/llvm-project/blob/main/clang/lib/Frontend/CompilerInvocation.cpp#L4464>.

Reading the doc around `llvm::hash_code`, my first reaction was indeed that modules may have to use a different hash.
I thought the current proposal was something to consider, because:

1. Practically, everything is done on `uint64_t`, so I would expect switching from `size_t` to `uint64_t` to have little to no impact on non-64bit targets.
2. The current modules code kind of already assumes `hash_code` is stable over different executions. Plain simple use-case with implicit modules would be severely deficient if the hash was not stable across executions. To be fair, the assumption is different from the one I am trying to fix of 32bit vs 64bit.

Alternatively, we could introduce something along the lines of `llvm::stable_hash_code` (other name suggestions welcome).
We can probably restructure the code with very few changes. (Really, it looks like we only need a configurable storage type, and allow skipping the execution seed part.)

Would you think this is a better approach ? Or do you have something else to suggest ?

Cheers


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102943/new/

https://reviews.llvm.org/D102943