[Mlir-commits] [mlir] [mlir][Transforms] Fix build after #116524 (part 2) (PR #121662)

[Markus Böck]

zero9178 wrote:

> I merged this PR immediately to fix the build.
> 
> But it is unclear to me why this fix was necessary. @zero9178, you commented about `ValueVectorMapInfo` in [#116524 (comment)](https://github.com/llvm/llvm-project/pull/116524#discussion_r1893991557). I found this fix by accident after many hours of debugging. Do you have any idea what could be going on here?

Mostly speculating, but based on the syndromes and the fix I think it is probably the following: Usually `erase` replaces a bucket item with the tombstone value to make sure that later lookups of an item in the map that require probing past the erased item still succeed:
https://github.com/llvm/llvm-project/blob/fd38a95586477f8f60330ef723406d69b33b91f6/llvm/include/llvm/ADT/DenseMap.h#L609-L618

https://github.com/llvm/llvm-project/blob/fd38a95586477f8f60330ef723406d69b33b91f6/llvm/include/llvm/ADT/DenseMap.h#L332-L335

Meaning that if:
* We have two values in the hashmap that with the current bucket size lead to the same hash and therefore create a probing chain of "v1" followed by "v2" and
* "v1" is erased, leading to it being replaced by a tombstone then
* "v2" can no longer be found as the implementation believes it to be an empty slot instead. `erase` and `find` will fail.

`DenseMapInfo<Value>::getEmptyKey()` and `DenseMapInfo<Value>::getTombstoneKey()` return special values of `Value` that are never ever used (`(void*)0x1` and `(void*)0x2`) hence safe to use.

https://github.com/llvm/llvm-project/pull/121662