Looks good, thanks!

Next wave of comments addressed

Comments of Nicolas incorporated

OK->success, error messages changed a bit

Comments of nicolas addressed

In D88533#2302544, @ftynse wrote:

In general, I wouldn't couple vectorization flags to tiling flags (well, maybe have tiling as a precondition) because we may want to tile several times. But this is already an improvement over the current state.

rebased

NoTile -> noTile, TileSize -> tileSize

Refactored "Magic numbers" away by introducing NoTile and TileSize constants in ConvOpVectorization
rewrite pattern.

Comments of Alex addressed

@aartbik changed in this revision: https://reviews.llvm.org/D87678

synced

integration tests split into different commit

Moved conversion to core (kept only pass).
Removed getZero helper function.
Simplified code with vector and std intrinsics op creation.

All comments except for the migration to Linalg/Vectorization.cpp resolved.

Moved pass to test/lib.
Replaced start and end offset with Mask.
Introduced preconditions for input and kernel sizes.
Reflected changes in tests.

Comments of nicolas resolved

Implementation changed as discussed here:
https://llvm.discourse.group/t/wrong-tile-size-for-convolution-ops/1699

I haven't sync with him regarding implementation details but I think he has an overview of what my goal is and how do I want to achieve it as he is my co-host.

In D86638#2242434, @mravishankar wrote:

Could we start a discourse post on this. Its more easy to document issues and discuss solutions there rather than on the patch.

Also, I am curious what other bugs you found in the convolution operation. Please add those to the discourse post if possible.

Landed as 8dace28f92056a04e02d913684d3f448ee1f295a

@rriddle I think I've resolved all of your comments.. Do you have some more? Thanks.

Conversion tests added

In D86638#2239561, @mravishankar wrote:

@limo1996 This is really nice find! I have been struggling with figuring out why convolutions are giving me errors in some cases, and I suspected something like, but wasnt able to nail this down.

There might be a better fix though. I'll take a look at this as well. For now I am going to just "Request changes" but I dont really have any changes to request. Just a placeholder for me to take a look again.

fixed add_subdirectory

added missing add_subdirectory in CMakeLists

more comments of rriddle addressed

comments of rriddle addressed

Aart's comments addressed

CHECK-NEXT -> CHECK

In D86197#2225972, @nicolasvasilache wrote:

I was under the impression that you were implementing the more general case but indeed not.
Then please just make sure the writes behave properly too, thanks!

Resigning as a reviewer to unblock as I am OOO until early Sept.

added test for transfer_write

In D86197#2225689, @nicolasvasilache wrote:

There are a few different places where patterns apply where we make the test that the permutation map is identity and that the rest is not yet supported.
Please update such places too and add tests to extend support more generally.

Thanks for extending this functionality!

In D86197#2225657, @ftynse wrote:

Can we have a test for this?

added test

update

rebased