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

Wed Jun 5 08:59:37 PDT 2019

JosephTremoulet marked an inline comment as done.
JosephTremoulet added inline comments.

================
Comment at: llvm/test/Transforms/EarlyCSE/commute.ll:645-651
+; This test is a reproducer for a bug involving inverted min/max selects
+; hashing differently but comparing as equal.  It generates several equivalent
+; pairs with the intent that eventually the hash table will fill enough for
+; the probe paths of an equivalent pair to collide and the hash mismatch
+; to be noticed.  It also includes a negation of each negation to check for
+; the same issue one level deeper.
+define void @many_min_max(i32* %px, i32* %py, i32* %pout) {
----------------
spatel wrote:
> How does this bug manifest? Can we cause an assertion failure without this patch?
> cc'ing some more potential reviewers to see if anyone has ideas for a smaller than ~1000 line regression test to reproduce the bug. ( @lebedev.ri @efriedma @craig.topper @arsenm ).
Yes, this same test reproduces an assertion[1] failure without this patch, but it does so nondeterministically -- I see it about every 20-30 runs.  The original compiland I was hitting this on would hit every ~2000 runs.  If I revert everything except the new assertion I added in isEqual, this test hits the new assertion something like 4 times out of 5 for me.

Happy for ideas how to trim it down / make it more deterministic.

[1] https://github.com/llvm/llvm-project/blob/590b1aee609d30346aee77a699381d21c538dd56/llvm/include/llvm/ADT/DenseMap.h#L404

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62644