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

[Martin Erhart]

maerhart wrote:

Thanks a lot for working on this!

> I just went back to the discussion on #101514. It sounded like #104668 would fix your issue. (That PR has landed.) Is that not the case?

Yes, https://github.com/llvm/llvm-project/pull/104668 fixed the issue. The problem is just that this PR reverted that fix.

I checked out https://github.com/llvm/llvm-project/pull/107109, and all our tests pass on top of that branch. It's not very time-critical; we integrate upstream weekly, but delaying it by a few days should be fine.

I don't have an upstream example to reproduce this, but we can return to it if necessary.

As an example, the following fails with a "null operands" error (I think for an op created during type conversion materialization, maybe because the `%1` operand still had the source type when the `add` was converted and later gets replaced with the result of the converted `hwarith.constant` op, but in some way not correctly?)
```mlir
  hw.module @backedges() {
    %0 = hwarith.add %1, %1 : (ui1, ui1) -> ui2
    %1 = hwarith.constant 1 : ui1
    hw.output
  }
```
If the order of the two operations in the body is changed, it succeeds and returns the following:
```mlir
  hw.module @backedges() {
    %true = hw.constant true
    %false = hw.constant false
    %0 = comb.concat %false, %true : i1, i1
    %false_0 = hw.constant false
    %1 = comb.concat %false_0, %true : i1, i1
    %2 = comb.add %0, %1 : i2
    hw.output
  }
```
command: `circt-opt -lower-hwarith-to-hw input.mlir`

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