[Mlir-commits] [mlir] [mlir][linalg] Add TransposeConv2D Pass (PR #68567)

[Jack Frankland]

FranklandJack wrote:

> However, could you update the summary to make the justification a bit more visible? In particular, the TOSA context makes a lot of sense and IMHO is key (it gives the rationale for this pass). Otherwise it's not clear why would `LinalgTransposeConv2D` work for only one very specific case. You could also add a comment that while the opposite transformation (`hwcf` -> `fhwc`) is not supported yet, it could be added in the future.

Yep, great idea!

> What happened to that test? Ideally there would be one test for 2D convolutions and you'd simply add another configuration to run (that would include your pass). In fact, I would start by parametrizing test-conv-2d-nhwc-hwcf-call.mlir so that it works for both formats. And take it from there. But this could be a task for a separate patch.

Would it be acceptable to do this as part of another patch? The way I see it is that we have lit tests for this transformation, so in that respect it should be "correct". The CPU tests in question seemed to validate the behaviour of convolutions on the CPU runner. I agree it would be beneficial to test the code generated from this tranformation, but since that would amount to testing the behaviour of `linalg.transpose` and `linalg.conv2d_nhwc_hwcf` rather than the output of this transformation it seems like an orthogonal concern to the idea behind this PR? Would be interesting to know what @cfRod thinks as well?



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