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

[River Riddle via Phabricator via llvm-commits]

rriddle added inline comments.


================
Comment at: mlir/include/mlir/Dialect/Linalg/Transforms/Transforms.h:14
+#include "mlir/Dialect/Linalg/IR/LinalgOps.h"
+#include "mlir/Dialect/Linalg/Passes.h"
+#include "mlir/Dialect/Linalg/Utils/Utils.h"
----------------
I don't think all of these are necessary, can you trim this list?


================
Comment at: mlir/include/mlir/Dialect/Linalg/Transforms/Transforms.h:140
+
+//----------------------------------------------------------------------------//
+// Transformations exposed as rewrite patterns.
----------------
```
//===---...---===//
```


================
Comment at: mlir/include/mlir/Dialect/Linalg/Transforms/Transforms.h:345
+    } else {
+      if (failed(linalgOpToParallelLoops<OpTy>(rewriter, op)))
+        return failure();
----------------
nit: merge this with the else.


================
Comment at: mlir/lib/Dialect/Linalg/Transforms/Transforms.cpp:73
+    // 3. Has no marker but was expecting a marker.
+    return rewriter.notifyMatchFailure(op, [&](::mlir::Diagnostic &diag) {
+      diag << " does not have any marker from list: ";
----------------
Remove the `::mlir::`. There are more instances of this throughout the revision. 


================
Comment at: mlir/lib/Dialect/Linalg/Transforms/Transforms.cpp:118
+  auto tileRes =
+      (true) ? tileLinalgOp(rewriter, linalgOp, tileSizes, interchangeVector)
+             : tileLinalgOpToParallelLoops(rewriter, linalgOp, tileSizes,
----------------
This looks weird. This always resolves to tileLinalgOp.


================
Comment at: mlir/lib/Dialect/Linalg/Transforms/Transforms.cpp:149
+
+  // TODO(ntv): figure out how this interplays with named ops. In particular
+  // this should break the named op property.
----------------
nit: Remove user name from TODOs.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79271