[PATCH] D144108: [mlir][LinAlg][Transform] Add a transform op for conv2d to im2col

Nicolas Vasilache via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Feb 16 02:10:48 PST 2023


nicolasvasilache added inline comments.


================
Comment at: mlir/lib/Dialect/Linalg/TransformOps/LinalgTransformOps.cpp:3049-3052
+  if (failed(applyPatternsAndFoldGreedily(target, std::move(patterns)))) {
+    return emitSilenceableFailure(target->getLoc())
+           << "failed to apply img2col";
+  }
----------------
ftynse wrote:
> Do we actually need the entire greedy rewriter for this, including all the pseudo-canonicalization and DCE stuff it carries? This is applying three patterns that seem mutually exclusive. Can't we just apply patterns one-by-one? With a tiny bit of effort, we can even be more informative as to what didn't match.
strong +1 to this.

Cutting a little deeper to drop the driver like I am doing here https://reviews.llvm.org/D143943 would be very welcome.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144108



More information about the llvm-commits mailing list