[Mlir-commits] [mlir] [mlir][Transforms] `OneToNTypeConversion.cpp`: Fix invalid IR (PR #77922)

[Ingo Müller]

ingomueller-net wrote:

Thanks for the explanation and confirmation!

> There's one more tricky case in the 1-to-n conversion test cases. There are 3 patterns for converting function signatures, one for each of these ops: `func.func`, `func.call`, `func.return`. After applying the `func.func` pattern, the IR fails to verify because the `func.call` ops have not been updated yet. (But it verifies eventually, when the entire IR has been processed.) I don't know yet what to do with that. In a sense, the `func.call` verifier does not adhere to MLIR coding guidelines because it verifies a non-local property.
> 
> Note: We don't have this issue with the regular dialect conversion because there are no "expensive checks" to verify the IR after conversion pattern applications. We only do that for rewrite patterns.

Yeah, I understand the problem. At the time, I think I didn't fully understand it -- I just applied the same pattern as in the regular conversion and saw that it worked...

I am not sure if the problem is really the verifier of `func.call`. I think it doesn't actually verify the matching signatures itself. Instead, it implements ['SymbolUserOpInterface`](https://github.com/llvm/llvm-project/blob/7851670/mlir/include/mlir/IR/SymbolInterfaces.td#L198) and thus [`verifySymbolUses`](https://github.com/llvm/llvm-project/blob/08e4386a2c91/mlir/lib/Dialect/Func/IR/FuncOps.cpp#L62), which is [called](https://github.com/llvm/llvm-project/blob/08e4386a2c91/mlir/lib/IR/SymbolTable.cpp#L483) by the containing symbol table op. In fact, all verification seems to verify local properties this way.

I rather have the feeling that one cannot expect the IR to (fully?) verify while pattern application is still ongoing. Otherwise, symbols and all of their uses would *always* have to be changed by the same pattern, which clearly isn't what's happening, right?

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