[Mlir-commits] [mlir] [mlir][Transforms] Dialect conversion: Simplify handling of dropped arguments (PR #96207)

[Matthias Springer]

matthias-springer wrote:

I suspected that it's something like that... There is a general problem with the implementation of that pass. It is running a dialect conversion but `populateSCFToControlFlowConversionPatterns` populates non-dialect-conversion patterns.

This is generally unsafe because a dialect conversion materializes some IR changes late. I.e., not when the pattern is running, but at the very end when the dialect conversion is done and it is guaranteed that it will succeed. That means the IR is often invalid after applying a conversion pattern. But non-dialect-conversion patterns may assume that the IR is valid at the beginning of a pattern application. (They basically the result of partial conversion pattern applications.) That's why the two can generally not be mixed. Unfortunately, this was never properly documented.

So I see two options:
* Turn `populateControlFlowToLLVMConversionPatterns` into conversion patterns. Same for `populateAffineToStdConversionPatterns`, `populateArithExpandOpsPatterns`, `populateVectorToLLVMConversionPatterns` (currently populates a mix of rewrite patterns and conversion patterns).
* Or: Split the pass into two steps: first apply all patterns that are regular rewrite patterns as part of a greedy pattern rewrite. Then apply the dialect conversion with the conversion patterns.

The second option may be the simplest fix. What do you think?

In the meantime, I'm going to try to reproduce the error with the pass that you are using, to understand what exactly is happening in this particular case.

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