[Mlir-commits] [mlir] [mlir][Transforms] Dialect conversion: Make materializations optional (PR #104668)

[Matthias Springer]

matthias-springer wrote:

There is a more direct way to fix the issue (#106760 is still desirable because it simplifies the code base). In the code that was deleted by this PR, there is the following:

```c++

    // Update the reverse mapping, or remove the mapping if we couldn't update
    // it. Not being able to update signals that the mapping would have become
    // circular (i.e. %foo -> newValue -> %foo), which may occur as values are
    // propagated through temporary materializations. We simply drop the
    // mapping, and let the post-conversion replacement logic handle updating
    // uses.
    for (Value inverseMapVal : inverseMapIt->second)
      if (!rewriterImpl.mapping.tryMap(inverseMapVal, newValue))
        rewriterImpl.mapping.erase(inverseMapVal);
```

I saw such circular mapping in the resulting IR in IREE and thought is a problem with this PR. But the above code snippet shows that these are actually expected to appear. #106760 changes the implementation so that we never generate them. That's one way to handle the problem. Alternatively, we can do something similar as the old code and handle them post-conversion during `reconcileUnrealizedCasts`. I'm going to give that a try.


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