Rebase. Reviving this as it's apparently come up again when adding a new register class.

LGTM cheers

In D155034#4648233, @c-rhodes wrote:

I also tried lowering a simple example to LLVM but hit an assert, is this expected to work yet? Example below

Sorry about that. My main goal with this patch was about having the representation working at vector level (and adjust all the impacted tests, which were a lot at the beginning) but not so much about adding support for the new value-based functionality end to end. I just added support for 1-D cases to the LLVM conversion to demonstrate that it works but there are quite a few TODOs, esp. wrt canonicalizations. Hopefully that makes sense!

Thanks for this work Diego! this will useful for ArmSME. We have ops to load/store slices of a tile (2-D scalable vector) and this support should enable us to lower vector.insert / vector.extract to these ops. #66691 adds support to lower vector.print to ArmSME, this could be done generically in VectorToSCF once this lands, after which we could add our target specific lowerings :) Also be useful for places where we can't fully unroll.

LGTM barring Diego's comment, cheers

this has also broke my workflow, I use lit -a and copy/paste the command to shell to see the output or add different flags, it seems like a fundamental feature and I can't understand why this has been removed?

In D159080#4624367, @awarzynski wrote:

I think this op is a bit clunky and could be simplified by returning the element type of the tile rather than an int representing the bitwidth, or perhaps just the 2-D scalable vector type this is extracted from, but that's a slightly move involved refactor.

Which Op are you referring to arm_sme.zero? arm_sme.cast_tile_to_vector?

I think this op is a bit clunky and could be simplified by returning the element type of the tile rather than an int representing the bitwidth, or perhaps just the 2-D scalable vector type this is extracted from, but that's a slightly move involved refactor.

Rebase (vector_to_tile_slice renamed to move_vector_to_tile_slice)

LGTM cheers

Rename vector_to_tile_slice to move_vector_to_tile_slice

In D158586#4613019, @awarzynski wrote:

LGTM, thanks!

I've made a few small suggestions, but it's up to you whether to incorporate them. Great to see all this "just working" :)

Address comments.

LGTM

Update descriptions for op and type constraint.

• Address most comments.
• Add type constraint to VectorToTileSliceOp that verifies 1-D vector type matches element type of 2-D vector type (tile).
• Rename VectorToTileSliceOp::getVectorType() -> VectorToTileSliceOp::getTileType() to prevent confusion with vector operand.

I've left a few minor comments but otherwise LGTM, thanks Ben!

rebased and addressed comments before landing, @dcaballe @awarzynski thanks for review!

In D156689#4556512, @awarzynski wrote:

LGTM, thanks! I've left a few more comments, but feel free to ignore.

this isn't changing anything in the example you provided, the offset is adjusted when materializing the tile slice loop, and that is tested in the -convert-arm-sme-to-scf tests.

Clarified what I had in mind inline in a test. Not a blocker.

In D156689#4550748, @awarzynski wrote:

Thanks Cullen! A few minor comments/suggestions.

address comments

Update comment.

failing top of tree (991855ea8a7ff070ca0e22ccba0ff2e96dcdf65b)

In D156522#4549769, @sameerds wrote:

In D156522#4549644, @c-rhodes wrote:

this broke my build with:

ld.lld-12: error: undefined symbol: llvm::GenericCycle<llvm::GenericSSAContext<llvm::Function> >::contains(llvm::BasicBlock const*) const
>>> referenced by UniformityAnalysis.cpp:72 (/home/culrho01/llvm-project/llvm/lib/Analysis/UniformityAnalysis.cpp:72)
>>>               lib/Analysis/CMakeFiles/LLVMAnalysis.dir/UniformityAnalysis.cpp.o:(llvm::GenericUniformityAnalysisImpl<llvm::GenericSSAContext<llvm::Function> >::usesValueFromCycle(llvm::Instruction const&, llvm::GenericCycle<llvm::GenericSSAContext<llvm::Function> > const&) const)

please could you look into it?

It's not obvious to me how this change could affect affect functions defined in GenericCycle. Can you please provide the commit hash that you are building, and the build parameters? I will need to reproduced this locally to make any progress.

this broke my build with:

ld.lld-12: error: undefined symbol: llvm::GenericCycle<llvm::GenericSSAContext<llvm::Function> >::contains(llvm::BasicBlock const*) const
>>> referenced by UniformityAnalysis.cpp:72 (/home/culrho01/llvm-project/llvm/lib/Analysis/UniformityAnalysis.cpp:72)
>>>               lib/Analysis/CMakeFiles/LLVMAnalysis.dir/UniformityAnalysis.cpp.o:(llvm::GenericUniformityAnalysisImpl<llvm::GenericSSAContext<llvm::Function> >::usesValueFromCycle(llvm::Instruction const&, llvm::GenericCycle<llvm::GenericSSAContext<llvm::Function> > const&) const)

please could you look into it?

In D155839#4547617, @awarzynski wrote:

Incorporate suggestions from Matt and Cullen - thanks!

@c-rhodes, no luck reproducing your failure :( Could you try again?

Remove unnecessary cast_vector_to_tile op at the end of tile_load conversion.

Address comments

LGTM cheers

Address comments

Thanks Andrzej this is a nice refactor, I haven't reviewed every single test as there's a fair amount of churn but generally LGTM.

Thanks for patch the Ben, this will improve tests like the one added in D155839 and mlir/test/Integration/Dialect/Vector/CPU/ArmSME/vector-load-store.mlir that print scalable vectors. I've left a few minor comments but otherwise LGTM, aside from the test failure @kuhar mentioned. Please allow time for others to review.

In D156516#4543072, @lhames wrote:

Otherwise LGTM. Thanks @c-rhodes!