[Mlir-commits] [mlir] [vector][mlir] Canonicalize to shape_cast where possible (PR #140583)

[Kunwar Grover]

Groverkss wrote:

> Note, James has already proposed/implemented a number of steps to improve shape_cast:

Lowering ([mlir][vector] Improve shape_cast lowering  #140800)
Folding (https://github.com/llvm/llvm-project/pull/146368 + https://github.com/llvm/llvm-project/pull/135841)
This PR simply takes things further (and was discussed before: https://github.com/llvm/llvm-project/pull/138777). I just wanted to highlight - there has been a number of PRs through which we concluded that this would be beneficial.

I'm sorry, but this response is a bit confusing to me. I'm very aware of the previous prs by James, because James and I are aligned on what we are working towards. I didn't chime in on those prs because I didn't want two people with the same ideas , from the same team and working on similar things to just review things between them and land things, given that we have multiple maintainers for Vector dialect. I do skim by those prs, I just didn't read until the title of this PR that we came to a conclusion that these patterns should be canonicalizations.

In fact, i'm more confused that we talk about context here, given that the same change was reverted in the past and I distinctly remember we never came to conclusion what is the right form here: https://github.com/llvm/llvm-project/pull/72918 . I do not see that we resolved any of the concerns in that discussion, so I don't think concluding this is beneficial is correct, without previous context. There was even an RFC where @banach-space and @dcaballe both were present: https://discourse.llvm.org/t/improving-handling-of-unit-dimensions-in-the-vector-dialect/75216/29 .I don't see anybody followup on any of those discussions, so we cannot throw those conclusions away and proceed with something a lot of reviewers disagreed with.

> Do we need an RFC?

Given that there was already an RFC about this, with no followup on things, yes we need an RFC on this. A lot of vector dialect users were present in that discussion whose concerns need to be addressed. I'm not okay with this proceeding as a PR given that there was an RFC about this where this PR was not the conclusion.

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