[PATCH] D61975: [CodeGen] Fix hashing for MO_ExternalSymbol MachineOperands.

[Kristina Brooks via Phabricator via llvm-commits]

kristina added a comment.

In D61975#1519367 <https://reviews.llvm.org/D61975#1519367>, @efriedma wrote:

> > Would it make sense to also add a test explicitly using llvm::hash_combine
>
> Using llvm::hash_combine on what, exactly?
>
> > as well as a negative test where hash code comparison fails for a literal and the same literal in a StringRef every time
>
> You mean something like `ASSERT_NE(SymName1.data(), SymName2.data());`?


I just realized that this was for specialization of hash_value for MO, I'm blind, disregard the first remark, sorry.

Regarding second, I meant a case of performing `hash_combine` manually, and then using `ASSERT_NE`, with first case being a string literal, and second being the same literal but passed in as `StringRef`. Assuming it's possible to have a string that would never yield identical hash result to hashing the pointer to a literal. I'm not sure if that makes sense as a test or not, thinking about it, probably doesn't aside from "just for the sake of it/out of paranoia". Was just a suggestion, if it doesn't make sense to test it, ignore the remark.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D61975