[PATCH] D79271: [mlir][Linalg] Mostly NFC - Refactor Linalg patterns and transformations.

[Mahesh Ravishankar via Phabricator via llvm-commits]

mravishankar added a comment.

+1 to all these patterns. This is mostly how IREEs code-generation is structured. I can pretty much replace the patterns in IREE to use these instead.



================
Comment at: mlir/include/mlir/Dialect/Linalg/Transforms/Transforms.h:203
+template <typename OpTy>
+struct LinalgTilingPattern : public LinalgBaseTilingPattern {
+  LinalgTilingPattern(MLIRContext *context, LinalgTilingOptions options,
----------------
Is there a similar pattern for TileAndFuse?


================
Comment at: mlir/include/mlir/Dialect/Linalg/Transforms/Transforms.h:267
+template <typename OpTy>
+struct LinalgPromotionPattern : public LinalgBasePromotionPattern {
+  LinalgPromotionPattern(MLIRContext *context,
----------------
+1 to this and most patterns below. This is mostly how IREEs code-generation is structured. I can pretty much replace the patterns in IREE to use these instead.


================
Comment at: mlir/lib/Dialect/Linalg/Transforms/Loops.cpp:562
 template <typename LoopTy, typename ConcreteOpTy>
-Optional<LinalgLoops>
-LinalgOpToLoopsImpl<LoopTy, ConcreteOpTy>::doit(Operation *op,
-                                                PatternRewriter &rewriter) {
+Optional<LinalgLoops> linalgOpToLoopsImpl(Operation *op, OpBuilder &builder) {
   using Impl = GenerateLoopNest<LoopTy, ConcreteOpTy>;
----------------
I am not sure making this a method is that ergonomic. You cant partially specialize functions like you can do structs. With multiple template parameters having a struct and ability to partially specialize is better?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79271/new/

https://reviews.llvm.org/D79271