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

[Andrzej Warzyński]

banach-space wrote:

Thanks for replying to my comments!

> I can cite few examples that violates the notion that LowerVector*.cpp exclusively contains vector -> vector rewrites such as e.g. : LowerVectorContract.

Can you link a specific example from that file that doesn't lower to other Vector Ops?

Bare in mind that there are many examples in-tree that e.g. don't follow our coding guidelines. However, that doesn't mean that it's OK to violate them, it merely means that there's code that should be re-formatted ;-)

> Im not following the logical inference of the absense of VectorToArith to a keep this as a folder. 

Without a use-case, this felt like moving code around (and creating a new file to maintain and to compile). But now that **there is** a use-case, we can park that discussion.

> We do actually have a usecase downstream. Sometimes, we see patterns like:
>
> %a = vector.step : vector<128xindex>
> %b = vector.extract_strided_slice %thread_id, %a : vector<4xindex>
> If you know it's a vector.step, you can fold the step + slice into:
> 
> %a = vector.step : vector<4xindex>
> %b = arith.constant ... : vector<4xindex>
> %c = arith.addi %b, %a
> %d = arith.addi %c, %thread_id
> So you don't have to materialize a big constant like vector<128xindex> into private memory. Having vector.step allows you to know how the arith.constant materializes.

Cool, why not upstream that? That's a good optimisation and nice justification for this change.

> This part I somewhat agree.
> 
> So there were three options :

> A2 is better choice. 

+1

As for testing, I'd add this pattern to `TestVectorScanLowering` (and rename it as `TestVectorToArithLowering`). That would be the least intrusive and the most straightforward path for now.

Having a dedicated TD op for such a small pattern feels like an overkill and we should probably find other patterns to group this one with. I don't really have a good suggestion just now, so I'd leave it as a TODO. These things become clearer once we start combining stuff.

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