[MLIR][Linalg] Rename convolution pass

[rengolin]

Rename the pass LinalgNamedOpConversionPass to SimplifyDepthwiseConvPass to avoid conflating it with the new morphisms we are creating between the norms.

[llvmbot]

@llvm/pr-subscribers-mlir

@llvm/pr-subscribers-mlir-linalg

Author: Renato Golin (rengolin)

Changes

Rename the pass LinalgNamedOpConversionPass to SimplifyDepthwiseConvPass to avoid conflating it with the new morphisms we are creating between the norms.

Keep the old pass/function as deprecated for now, and agree on a timing to remove it from the tree at some point in the future.

---

Full diff: https://github.com/llvm/llvm-project/pull/154400.diff

4 Files Affected:

• (modified) mlir/include/mlir/Dialect/Linalg/Passes.td (+7-1)
• (modified) mlir/include/mlir/Dialect/Linalg/Transforms/Transforms.h (+6-2)
• (modified) mlir/lib/Dialect/Linalg/Transforms/CMakeLists.txt (+1-1)
• (renamed) mlir/lib/Dialect/Linalg/Transforms/SimplifyDepthwiseConv.cpp (+24)

diff --git a/mlir/include/mlir/Dialect/Linalg/Passes.td b/mlir/include/mlir/Dialect/Linalg/Passes.td
index 0b2c9c94dc73d..9aa199f140796 100644
--- a/mlir/include/mlir/Dialect/Linalg/Passes.td
+++ b/mlir/include/mlir/Dialect/Linalg/Passes.td
@@ -86,13 +86,19 @@ def LinalgSpecializeGenericOpsPass : Pass<"linalg-specialize-generic-ops">,
   let dependentDialects = ["linalg::LinalgDialect"];
 }
 
-def LinalgNamedOpConversionPass: Pass<"linalg-named-op-conversion"> {
+def LinalgNamedOpConversionPass: Pass<"linalg-named-op-conversion">,
+                                 Deprecated<"Use 'simplify-depthwise-conv' instead."> {
   let summary = "Convert from one named linalg op to another.";
   let dependentDialects = ["linalg::LinalgDialect", "tensor::TensorDialect"];
 }
 
 // ------------------ End of "form" conversions
 
+def SimplifyDepthwiseConvPass: Pass<"simplify-depthwise-conv"> {
+  let summary = "Simplify depthwise convolution.";
+  let dependentDialects = ["linalg::LinalgDialect", "tensor::TensorDialect"];
+}
+
 def ConvertElementwiseToLinalgPass : Pass<"convert-elementwise-to-linalg", ""> {
   let summary = "Convert ElementwiseMappable ops to linalg";
   let description = [{
diff --git a/mlir/include/mlir/Dialect/Linalg/Transforms/Transforms.h b/mlir/include/mlir/Dialect/Linalg/Transforms/Transforms.h
index 8d5306dca43e3..d757779a60015 100644
--- a/mlir/include/mlir/Dialect/Linalg/Transforms/Transforms.h
+++ b/mlir/include/mlir/Dialect/Linalg/Transforms/Transforms.h
@@ -24,6 +24,7 @@
 #include "mlir/IR/PatternMatch.h"
 #include "mlir/Interfaces/TilingInterface.h"
 #include "mlir/Transforms/DialectConversion.h"
+#include "llvm/Support/Compiler.h"
 #include "llvm/ADT/SmallBitVector.h"
 #include "llvm/ADT/SmallSet.h"
 
@@ -1962,8 +1963,11 @@ void populateFoldAddIntoDestPatterns(RewritePatternSet &patterns);
 void populateFuseTensorPadWithProducerLinalgOpPatterns(
     RewritePatternSet &patterns);
 
-/// Patterns to convert from one named op to another. These can be seen as
-/// canonicalizations of named ops into another named op.
+/// Patterns to simplify depthwise convolutions.
+void populateSimplifyDepthwiseConvPatterns(RewritePatternSet &patterns);
+
+/// Patterns to convert from one named op to another. So far only used on
+/// depthwise convolutions, so deprecated. Use the pattern avove.
 void populateLinalgNamedOpConversionPatterns(RewritePatternSet &patterns);
 
 /// Patterns to fold unit-extent dimensions in operands/results of linalg ops on
diff --git a/mlir/lib/Dialect/Linalg/Transforms/CMakeLists.txt b/mlir/lib/Dialect/Linalg/Transforms/CMakeLists.txt
index 6ec2e9fd0be7d..fb39e18691e03 100644
--- a/mlir/lib/Dialect/Linalg/Transforms/CMakeLists.txt
+++ b/mlir/lib/Dialect/Linalg/Transforms/CMakeLists.txt
@@ -26,7 +26,7 @@ add_mlir_dialect_library(MLIRLinalgTransforms
   MorphOps.cpp
   TransposeMatmul.cpp
   ShardingInterfaceImpl.cpp
-  NamedOpConversions.cpp
+  SimplifyDepthwiseConv.cpp
   NamedToElementwise.cpp
   BlockPackMatmul.cpp
   PackAndUnpackPatterns.cpp
diff --git a/mlir/lib/Dialect/Linalg/Transforms/NamedOpConversions.cpp b/mlir/lib/Dialect/Linalg/Transforms/SimplifyDepthwiseConv.cpp
similarity index 88%
rename from mlir/lib/Dialect/Linalg/Transforms/NamedOpConversions.cpp
rename to mlir/lib/Dialect/Linalg/Transforms/SimplifyDepthwiseConv.cpp
index a2bd9d92815a0..1d918d7c1663a 100644
--- a/mlir/lib/Dialect/Linalg/Transforms/NamedOpConversions.cpp
+++ b/mlir/lib/Dialect/Linalg/Transforms/SimplifyDepthwiseConv.cpp
@@ -22,6 +22,7 @@
 
 namespace mlir {
 #define GEN_PASS_DEF_LINALGNAMEDOPCONVERSIONPASS
+#define GEN_PASS_DEF_SIMPLIFYDEPTHWISECONVPASS
 #include "mlir/Dialect/Linalg/Passes.h.inc"
 } // namespace mlir
 
@@ -143,6 +144,22 @@ struct SimplifyDepthwiseConvQOp
   }
 };
 
+struct SimplifyDepthwiseConvPass
+    : public impl::SimplifyDepthwiseConvPassBase<
+          SimplifyDepthwiseConvPass> {
+  using impl::SimplifyDepthwiseConvPassBase<
+      SimplifyDepthwiseConvPass>::SimplifyDepthwiseConvPassBase;
+
+  void runOnOperation() override {
+    Operation *op = getOperation();
+    RewritePatternSet patterns(op->getContext());
+    populateSimplifyDepthwiseConvPatterns(patterns);
+    if (failed(applyPatternsGreedily(op, std::move(patterns))))
+      return signalPassFailure();
+  }
+};
+
+// Deprecated, use the one above
 struct LinalgNamedOpConversionPass
     : public impl::LinalgNamedOpConversionPassBase<
           LinalgNamedOpConversionPass> {
@@ -159,6 +176,13 @@ struct LinalgNamedOpConversionPass
 };
 } // namespace
 
+void mlir::linalg::populateSimplifyDepthwiseConvPatterns(
+    RewritePatternSet &patterns) {
+  patterns.add<SimplifyDepthwiseConvOp, SimplifyDepthwiseConvQOp>(
+      patterns.getContext());
+}
+
+// Deprecated, use the one above
 void mlir::linalg::populateLinalgNamedOpConversionPatterns(
     RewritePatternSet &patterns) {
   patterns.add<SimplifyDepthwiseConvOp, SimplifyDepthwiseConvQOp>(

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[MaheshRavishankar]

This seems like a simple pass name change? I don't think we need to do this through a deprecation mechanism. I am fine either way, but dropping the old pass name is fine according to me.

[rengolin]

This seems like a simple pass name change? I don't think we need to do this through a deprecation mechanism. I am fine either way, but dropping the old pass name is fine according to me.

I was unsure who's using. It is indeed simple. Either way, got me trained on emitting deprecation notices. 😆

I'm happy to just rename if everyone agrees.

[banach-space]

I am OOO this week, so replying quickly.

Simple rename would be fine with me. You can add a TODO as well, eg "TODO: Verify whether this is still needed."

In general, less is more - if we can achieve this through "morphism" then I would prefer to maintain only one mechanism.

Thanks!

[rengolin]

In general, less is more - if we can achieve this through "morphism" then I would prefer to maintain only one mechanism.

This particular pattern adds more ops around the convolution, so not a "form conversion". I want in the future to consider DAGs as a form (contract { broadcast map } == broadcast + contract), but that's a long term discussion that need to also include composite ops like softmax. Right now, I don't want to pollute the design space.

Updated the PR to only rename for now.