[Mlir-commits] [mlir] [Linalg] Update Conv Decomposition patterns to work with generic convolution ops as well (PR #174196)

[Han-Chung Wang]

hanhanW wrote:

> > Doesn't vectorization already force users to collect ops and call [vectorize](https://github.com/llvm/llvm-project/blob/dff081c26f11e1679411e7c0b4012e6a740b6cc3/mlir/lib/Dialect/Linalg/Transforms/Vectorization.cpp#L2703-L2707) method? I thought that users are already filtering the ops and apply vectorization. E.g., you match the op and vectorize the op. Users still can disallow the op in their vectorization list. Am I missing something here?
> 
> Hm, but if you pass `linalg.generic` and specialize inside the vectorizer, then you will end-up here:
> 
> https://github.com/llvm/llvm-project/blob/dff081c26f11e1679411e7c0b4012e6a740b6cc3/mlir/lib/Dialect/Linalg/Transforms/Vectorization.cpp#L2739
> 
> rather than here:
> 
> https://github.com/llvm/llvm-project/blob/dff081c26f11e1679411e7c0b4012e6a740b6cc3/mlir/lib/Dialect/Linalg/Transforms/Vectorization.cpp#L2768

I see what you mean. I feel that it is how we order the methods and we can update the method names. E.g., we can start with a few categories and the last one is `vectorizeLinalgOp` which is a generic version. Actually, the reason of the `vectorizeAsLinalgGeneric` naming is that it is a generic version but not tie to `linalg::GenericOp`. See the comment: https://github.com/llvm/llvm-project/blob/dff081c26f11e1679411e7c0b4012e6a740b6cc3/mlir/lib/Dialect/Linalg/Transforms/Vectorization.cpp#L1429-L1454

The convolution one is a special case because it has strides and dilations; it does not work with the generic vectorization well. I honestly don't know if there is a feasible solution to collapse them or not; it is what it is for years. So to me, there are no two paths. It is more like making the method name more accurate.

```
vectorize() {
  if (isConvLikeOp(op)) {
    return vectorizeConvLikeOp();
  }
  if (createNamedContraction && isContractionLikeOp(op))
            return vectorizeAsLinalgContraction(rewriter, state, linalgOp,
                                             results);
  return vectorizeAsLinalgGeneric(); // or vectorizeLinalgOp()
}
```

> You can still solve this with a control function, yes. Btw, here's a slightly different TD example:
> 
> https://github.com/llvm/llvm-project/blob/dff081c26f11e1679411e7c0b4012e6a740b6cc3/mlir/test/Dialect/Linalg/vectorization/convolution-with-patterns.mlir#L68
> 
> 
> Note that in this case it doesn't really matter what Op is matched.

In C++ code, it is more like `walking all the ops and pass all of them to vectorize method`. It is user's choise about how they create a pass.

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