[Mlir-commits] [mlir] [mlir][linalg] Fix inferConvolutionDimsImpl (depthwise convs) (PR #90057)

[Quinn Dawkins]

================
@@ -1062,6 +1062,28 @@ module attributes { transform.target_tag = "start_here" } {
     return %result : tensor<10x18x15xf64>
   }
 
+  func.func @convolution_depthwise(%input: tensor<1x10x196x48xf32>, %filter: tensor<1x4x48xf32>) -> tensor<1x10x191x48xf32> {
+    %cst = arith.constant 0.0 : f32 
+    %empty = tensor.empty() : tensor<1x10x191x48xf32>
+    %fill = linalg.fill ins(%cst : f32) outs(%empty : tensor<1x10x191x48xf32>) -> tensor<1x10x191x48xf32>
+    // expected-remark @below {{convolution}}
+    // expected-remark @below {{batch dims 0}}
+    // expected-remark @below {{output image dims 1 : i64, 2 : i64}}
+    // expected-remark @below {{output channel dims 3}}
+    // expected-remark @below {{filter loop dims 4 : i64, 5 : i64}}
+    // expected-remark @below {{input channel dims}}
+    // expected-remark @below {{depth dims 3}}
----------------
qedawkins wrote:

This doesn't look like what I would expect because it is classifying dim `3` twice. The naming might need to be improved for depthwise convolutions, but the idea with these classifications is that they tell users exactly how each different dimension appears in the indexing maps (and thus input/filter/output tensors). Right now the impl uses this to imply that all dims classified as `outputChannel` are only featured in the filter and output maps. Dims classified as `depth` appear in all three of the input, filter, and output.

I'm noticing that there is no description depth in this comment though: https://github.com/llvm/llvm-project/blob/76a3be7c766bd55221c3d0d0a74c42f82c5d76ed/mlir/lib/Dialect/Linalg/IR/LinalgInterfaces.cpp#L622-L645

(also the comment is kind of convoluted and could probably be rewritten, maybe with an example).

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