[Mlir-commits] [mlir] [MLIR] Allowing unsupported conv2d op to fail gracefully from vectorization (PR #130181)

[Han-Chung Wang]

================
@@ -1990,8 +1990,18 @@ static LogicalResult vectorizeLinalgOpPrecondition(
   // TODO: isaConvolutionOpInterface that can also infer from generic
   // features. But we will still need stride/dilation attributes that will be
   // annoying to reverse-engineer...
-  if (isa<ConvolutionOpInterface>(linalgOp.getOperation()))
+  if (isa<ConvolutionOpInterface>(linalgOp.getOperation())) {
+    // Check if it is 2d+ convolution. If it is, return failure because we don't
+    // support it. To use this pass on a 2d+ convolution, it should have already
+    // been decomposed to 1d convolution via
+    // DecomposeConvolutionToLowerDimOpsPass.
+    if (linalgOp.getNumParallelLoops() >= 4) {
+      LDBG("precondition failed: Regular 2d+ convolutions not supported.\n");
+      return failure();
+    }
----------------
hanhanW wrote:

I'm thinking that if we should just create the Conv1DGenerator and check if `valid` is true or not. The constructor does not look too expensive to me. It uses `matchLinalgReduction` and `getCombinerOpKind` methods, and those methods are also used by other precondition checks, [see an example](https://github.com/llvm/llvm-project/blob/3492245ac07ca68f434035c6577f55c790270354/mlir/lib/Dialect/Linalg/Transforms/Vectorization.cpp#L1819-L1838).

https://github.com/llvm/llvm-project/blob/3492245ac07ca68f434035c6577f55c790270354/mlir/lib/Dialect/Linalg/Transforms/Vectorization.cpp#L3128-L3180


If we go with this approach, I think we should update the constructor and move the below logic into the constructor.

https://github.com/llvm/llvm-project/blob/3492245ac07ca68f434035c6577f55c790270354/mlir/lib/Dialect/Linalg/Transforms/Vectorization.cpp#L3899-L3906

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