[PATCH] D62644: [EarlyCSE] Ensure equal keys have the same hash value

[Joseph Tremoulet via Phabricator via llvm-commits]

JosephTremoulet added a comment.

In D62644#1543328 <https://reviews.llvm.org/D62644#1543328>, @uabelho wrote:

> In D62644#1543279 <https://reviews.llvm.org/D62644#1543279>, @fhahn wrote:
>
> > It looks like this patch may have caused https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=15236. It would be great if you could have a look.
>
>
> That assertion is also seen here:
>
>   https://bugs.llvm.org/show_bug.cgi?id=42280
>   
>
> However, as I wrote in that PR, with older opt versions it still crashes now and then, only that it often takes more attempts.
>  And then we hit a different assertion:
>
>   opt: ../include/llvm/ADT/DenseMap.h:404: void llvm::DenseMapBase<llvm::DenseMap<(anonymous namespace)::SimpleValue, llvm::ScopedHashTableVal<(anonymous namespace)::SimpleValue, llvm::Value *> *, llvm::DenseMapInfo<(anonymous namespace)::SimpleValue>, llvm::detail::DenseMapPair<(anonymous namespace)::SimpleValue, llvm::ScopedHashTableVal<(anonymous namespace)::SimpleValue, llvm::Value *> *> >, (anonymous namespace)::SimpleValue, llvm::ScopedHashTableVal<(anonymous namespace)::SimpleValue, llvm::Value *> *, llvm::DenseMapInfo<(anonymous namespace)::SimpleValue>, llvm::detail::DenseMapPair<(anonymous namespace)::SimpleValue, llvm::ScopedHashTableVal<(anonymous namespace)::SimpleValue, llvm::Value *> *> >::moveFromOldBuckets(BucketT *, BucketT *) [DerivedT = llvm::DenseMap<(anonymous namespace)::SimpleValue, llvm::ScopedHashTableVal<(anonymous namespace)::SimpleValue, llvm::Value *> *, llvm::DenseMapInfo<(anonymous namespace)::SimpleValue>, llvm::detail::DenseMapPair<(anonymous namespace)::SimpleValue, llvm::ScopedHashTableVal<(anonymous namespace)::SimpleValue, llvm::Value *> *> >, KeyT = (anonymous namespace)::SimpleValue, ValueT = llvm::ScopedHashTableVal<(anonymous namespace)::SimpleValue, llvm::Value *> *, KeyInfoT = llvm::DenseMapInfo<(anonymous namespace)::SimpleValue>, BucketT = llvm::detail::DenseMapPair<(anonymous namespace)::SimpleValue, llvm::ScopedHashTableVal<(anonymous namespace)::SimpleValue, llvm::Value *> *>]: Assertion `!FoundVal && "Key already in new map?"' failed.
>


Taking a look at both.  This change had a bugfix and added more checking.  The new checking is meant to do exactly what @uabelho describes -- hash/isEqual mismatch would previously have led, nondeterministically and rarely, to the "Key already in new map?" assertion failure, and would now lead, nondeterministically but less rarely, to the new assertion ("getHashValueImpl(LHS) == getHashValueImpl(RHS)"), and with -earlycse-debug-hash would now lead deterministically to the new assertion (for functions in which earlycse processes two expressions that compare equal but hash differently).


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62644