[PATCH] D79518: [mlir][Linalg] Introduce a helper function for staged pattern application

[River Riddle via Phabricator via llvm-commits]

rriddle added inline comments.
Herald added a subscriber: jurahul.


================
Comment at: mlir/include/mlir/Dialect/Linalg/Transforms/Transforms.h:386
+    const OwningRewritePatternList &stage2Patterns,
+    llvm::function_ref<LogicalResult(Operation *)> stage3Lambda = nullptr);
 } // namespace linalg
----------------
nit: Drop the llvm::


================
Comment at: mlir/include/mlir/IR/PatternMatch.h:397
+  OwningRewritePatternList(T &&t) {
+    patterns.emplace_back(std::make_unique<T>(t));
+  }
----------------
You aren't properly forwarding `t` here, you need std::forward<T>(t)


================
Comment at: mlir/lib/Dialect/Linalg/Transforms/Transforms.cpp:205
+    const OwningRewritePatternList &stage2Patterns,
+    llvm::function_ref<LogicalResult(Operation *)> stage3Lambda) {
+  for (const auto &patterns : stage1Patterns) {
----------------
nit: Drop the llvm::


================
Comment at: mlir/lib/Dialect/Linalg/Transforms/Transforms.cpp:208
+    if (!applyPatternsAndFoldGreedily(op, patterns)) {
+      llvm::dbgs() << "Underlying first stage rewrite did not converge";
+      return failure();
----------------
Why aren't these llvm::dbgs calls wrapped by LLVM_DEBUG?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79518