[Mlir-commits] [mlir] [MLIR][Vector] Add Lowering for vector.step (PR #113655)

[Andrzej Warzyński]

banach-space wrote:

Thanks for working on this! I have a few high level comments.

> This is not ideal if we want to do transformation on it and defer the materizaliztion of the constants much later.

Do we? Is there a use case for this? I just want to make that we are not moving code for the sake of ... moving code around 😅 

Re layering, transformations in "mlir/lib/Dialect/Vector/Transforms/LowerVector{.*}.cpp" _lower_ from higher-level Vector Ops to lower-level Vector ops. Rewriting `vector.step` as `arith.constant` feels  more like dialect conversion to me. But then we don't have `VectorToArith`. So, to me, there are actual benefits to keeping this as a folder. 

Re changes in tests, we should avoid adding more patterns and TD Ops to the vectorizer tests that already use `transform.structured.vectorize_children_and_apply_patterns` (this TD Op should encapsulate everything that's needed).

What happens if you don't add `transform.apply_patterns.vector.step_to_arith` to the tests?

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