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

Andrzej WarzyƄski llvmlistbot at llvm.org
Thu Oct 19 04:16:47 PDT 2023


banach-space wrote:

Thanks for the explanation @FranklandJack !

> I guess I'm still a little confused about the utility of converting {h,w,c,f} to {f,w,h,c} here when the purpose is to sort of align on the {h,w,c,f} ordering for the filters, perhaps we could add an option or something to define which way you want to go?

It sounds like you are only interested in one specific case, which is totally fine :)

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.

> Sure, I'm happy to add a test, although I'm not sure if this is testing the transformation or the nhwc-fhwc conv2d operation itself? I see we don't have tests for that convolution channel but it existed before this PR.

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. 

> By the way as an aside, the TOSA documentatino refers to the filter as "weights" so I guess this is where I got that from

Ha, makes sense :) Naming is hard!

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


More information about the Mlir-commits mailing list