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

[Nicolas Vasilache via Phabricator via llvm-commits]

nicolasvasilache marked 6 inline comments as done.
nicolasvasilache added inline comments.


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


================
Comment at: mlir/include/mlir/Dialect/Linalg/Transforms/Transforms.h:267
+template <typename OpTy>
+struct LinalgPromotionPattern : public LinalgBasePromotionPattern {
+  LinalgPromotionPattern(MLIRContext *context,
----------------
mravishankar wrote:
> +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.
Cool, I need to refactor a bit more on a use case basis.
I'll send a few changes to promotion and arguments soon.


================
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>;
----------------
mravishankar wrote:
> 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?
There is no type deduction here, it is just called explicitly so it should be equivalent.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79271