[Mlir-commits] [mlir] [mlir][linalg] Implement Winograd Conv2D. (PR #94470)

Oleksandr Alex Zinenko llvmlistbot at llvm.org
Fri Jun 14 11:37:30 PDT 2024


https://github.com/ftynse requested changes to this pull request.

Thank you for your contribution! This is a rather big change so I would strongly suggest splitting it into several parts if you don't want to be blocked waiting on reviewers to find time to get through 2k LoC. For example:
- introduce the data modification operations + tests;
- implement the tiling interface for them + tests;
- implement the transformation and one way of executing it (test pass or transform dialect);
- implement the other way of executing it.
This will let reviewers focus on relevant aspects and not require committing a big block of time.

I left a couple of stylistic comments. The big one is to please document all functions and operations extensively.

If there is a design discussion to be had, please do so on https://discourse.llvm.org/c/mlir/31 rather than in PR comments. In particular, choosing the design for upstream MLIR is not conditioned on the needs of a particular downstream, however large. 

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


More information about the Mlir-commits mailing list