In D87688#2278471, @abialas wrote:

Thanks Thomas for the review, may I ask you to push this change to llvm master?
Btw, to not bother you again with this, is there a process to get RW access to the repo so I can push future changes on my own?

Why is this only for SToFP op? We should keep it consistent for the different converts. Also you need to add test(s).

In D86876#2251655, @abialas wrote:

Thanks Thomas, good catch. Please take a look at the updated version. If it looks good, may I ask you to push this change to llvm master?

Committed with https://github.com/llvm/llvm-project/commit/8d655042233bcfe572cdffe529b52e2fc285badf

In D86829#2250014, @antiagainst wrote:

My only concern with that is that this is compiler dependent. (for instance visual studio doesn't use this notation). Are you suggesting handling this with a ifdef to deal with different compilers? That's what I was trying to avoid.

Putting into the source code makes it more explicit and serves as documentation too. Is there any downside doing that other than handling different compilers? (BTW, IIRC, ORCJIT at the moment does not properly handle Windows COFF so that these runners cannot be run on Windows. But I could miss something.)

In D86829#2245958, @mehdi_amini wrote:

The file path is quite... unusual ;)

Thanks Aart. Please take another look.

Address code review comments

Thanks Mahesh for the review. I added extra context explaining why we do it that way and we can revisit later.

Add extra comment

Looks good to me.

Looks good to me

Address review comments

Thanks Artur. Looks good to me.

In D85244#2197244, @nicolasvasilache wrote:

The relaxing is too brutal IMO and will likely create multiple issues related to lowering to SCF and LLVM.
To reduce the pain in making this work with transformations I'd suggest to add restrictions on the type.
Basically you're only allowed to:

1. take elements immediately to the left of your element type in the memref and stick these into the vector.
2. cast to vector types that have the same number fo bits and a power of 2 size.

Anything else will be a significant amount of pain until we have a solid DataLayout in MLIR.

So basically you can allow the existing semantic + exactly 1 vector type_cast that changes vector type.

memref<axbxcxdxvector<txuxvxf32>> -> memref<axbx | cxdxvector<txuxvxf32>> -> memref<axbx vector<cxdxtxuxvxf32>> -> memref<axbx vector<whatever>> -> vector<whatever>, where:

1. | is a separator that you can put anywhere on the memref (just like the current vector.type_cast)
2. whatever has the same bitsize and alignment as cxdxtxuxvxf32.

To make 2. reasonably simple I'd say just allow the most minor of whatever and v x f32 that have a power of 2 size if you want to cast the element type.

I'd recommend very slowly and very carefully relaxing the constraints to match your needs without stepping out of bounds for now.

Add extra restrictions.

In D85244#2197244, @nicolasvasilache wrote:

The relaxing is too brutal IMO and will likely create multiple issues related to lowering to SCF and LLVM.
To reduce the pain in making this work with transformations I'd suggest to add restrictions on the type.
Basically you're only allowed to:

1. take elements immediately to the left of your element type in the memref and stick these into the vector.
2. cast to vector types that have the same number fo bits and a power of 2 size.

Anything else will be a significant amount of pain until we have a solid DataLayout in MLIR.

So basically you can allow the existing semantic + exactly 1 vector type_cast that changes vector type.

memref<axbxcxdxvector<txuxvxf32>> -> memref<axbx | cxdxvector<txuxvxf32>> -> memref<axbx vector<cxdxtxuxvxf32>> -> memref<axbx vector<whatever>> -> vector<whatever>, where:

1. | is a separator that you can put anywhere on the memref (just like the current vector.type_cast)
2. whatever has the same bitsize and alignment as cxdxtxuxvxf32.

To make 2. reasonably simple I'd say just allow the most minor of whatever and v x f32 that have a power of 2 size if you want to cast the element type.

I'd recommend very slowly and very carefully relaxing the constraints to match your needs without stepping out of bounds for now.

I'm abandoning this change as it is not needed after https://reviews.llvm.org/D85244.

rebase

In D85244#2194729, @aartbik wrote:

also, with the restrictions out of the way, did you make sure this does not crash the lowering to llvm part (try out a few of your new examples). you may have to return failure() in the lowering logic itself if it breaks anything....

In D85058#2189117, @aartbik wrote:

In D85058#2188725, @mehdi_amini wrote:

Does this op really belong to the vector dialect?

It seems like you're plugging a hole somewhere, but it isn't clear what or why this is desirable. Can you explain this in a thread on Discourse?

I agree that understanding the need better would help. We already have reshape/type_cast/shape_cast ops with somewhat specific behaviors, and I want to make sure we design the right operations (preferable just a few).

Address review comments

Handle scalar case and add a test for it.

Address review comments.

Address review comments

Thanks Nicolas. I addressed the review, please take another look to confirm that I got it right. Based on what you mentioned my understanding is that if any indices are different in the first ranks (from 0 to memrefRank-vectorRank) then the slices are disjoint for the rest of the ranks I check that the distance between the offset is smaller that the vector size (since I check that the vector types must match)

LGTM