[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:51
+
+//============================================================================//
+// Preconditions that ensure the corresponding transformation suceeds and can be
----------------
Please use the same header blocks as the rest of the infra.


================
Comment at: mlir/include/mlir/Dialect/Linalg/Transforms/Transforms.h:87
+  SmallVector<StringRef, 4> matchDisjunction;
+  llvm::Optional<StringRef> replacement;
+};
----------------
Drop the extraneous llvm::


================
Comment at: mlir/include/mlir/Dialect/Linalg/Transforms/Transforms.h:127
+                               LinalgMarker marker = LinalgMarker(),
+                               int benefit = 1);
+  LogicalResult matchAndRewrite(Operation *op,
----------------
This should really be using PatternBenefit instead of int


================
Comment at: mlir/lib/Dialect/Linalg/Transforms/Transforms.cpp:40
+
+//============================================================================//
+// Transformations exposed as rewrite patterns.
----------------
same here.


================
Comment at: mlir/lib/Dialect/Linalg/Transforms/Transforms.cpp:215
+    return failure();
+  if (!linalgOp)
+    return failure();
----------------
This looks duplicated


================
Comment at: mlir/lib/Dialect/Linalg/Transforms/Vectorization.cpp:43
+    return false;
+  if (r.getBlocks().size() != 1)
+    return false;
----------------
Replace these two with llvm::hasSingleElement


================
Comment at: mlir/lib/Dialect/Linalg/Transforms/Vectorization.cpp:46
+  auto &ops = r.front().getOperations();
+  if (ops.size() != 3)
+    return false;
----------------
llvm::hasNItems(r.front().begin(), r.front().end(), 3);


================
Comment at: mlir/lib/Dialect/Linalg/Transforms/Vectorization.cpp:96
+  };
+  if (!llvm::all_of(genericOp.getInputsAndOutputBuffers(),
+                    isStaticMemRefWithIdentityLayout))
----------------
nit: return success(llvm::all_of(...));


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79271