[Mlir-commits] [mlir] [mlir][arith] Fix issue where setOperand is called inside the fold fu… (PR #204506)

[donald chen]

cxy-1993 wrote:

> Can you provide a complete description for the PR? I don't know what is the supposed issue this is solving: setOperand seems fine to me to do in a folder.

Sorry, I misunderstood the current folding mechanism. I had always assumed that the fold method was only supposed to compute OpFoldResults that can replace the operation's results equivalently, and that it was not allowed to modify the IR. This is also the requirement in our downstream LLVM fork, which is why I submitted this PR.

After carefully reading the documentation on canonicalization with the fold method (https://mlir.llvm.org/docs/Canonicalization/#canonicalizing-with-the-fold-method), I realized that MLIR's design does in fact allow this kind of in-place modification. I will close this PR and update the corresponding code in our downstream accordingly.

That said, I still don't fully understand the rationale behind this design. What is the distinction between a folder and a canonicalization pattern now? Is a folder essentially just a canonicalization pattern that is restricted to modifying only the current operation?

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