address comments

Author: matthias-springer

mlir/include/mlir/IR/PatternMatch.h

@@ -241,16 +241,18 @@ template <typename PatternT>
 class SplitMatchAndRewriteImpl : public PatternT {
   using PatternT::PatternT;
 
+  /// Attempt to match against IR rooted at the specified operation, which is
+  /// the same operation kind as getRootKind().
+  ///
+  /// Note: This function must not modify the IR.
+  virtual LogicalResult match(typename PatternT::OperationT op) const = 0;
+
   /// Rewrite the IR rooted at the specified operation with the result of
   /// this pattern, generating any new operations with the specified
   /// rewriter.
   virtual void rewrite(typename PatternT::OperationT op,
                        PatternRewriter &rewriter) const = 0;
 
-  /// Attempt to match against code rooted at the specified operation,
-  /// which is the same operation code as getRootKind().
-  virtual LogicalResult match(typename PatternT::OperationT op) const = 0;
-
   LogicalResult matchAndRewrite(typename PatternT::OperationT op,
                                 PatternRewriter &rewriter) const final {
     if (succeeded(match(op))) {
@@ -263,9 +265,7 @@ class SplitMatchAndRewriteImpl : public PatternT {
 } // namespace detail
 
 /// RewritePattern is the common base class for all DAG to DAG replacements.
-/// By overloading the "matchAndRewrite" function, the user can perform the
-/// rewrite in the same call as the match.
-///
+
 class RewritePattern : public Pattern {
 public:
   using OperationT = Operation *;
@@ -274,8 +274,11 @@ class RewritePattern : public Pattern {
   virtual ~RewritePattern() = default;
 
   /// Attempt to match against code rooted at the specified operation,
-  /// which is the same operation code as getRootKind(). If successful, this
-  /// function will automatically perform the rewrite.
+  /// which is the same operation code as getRootKind(). If successful, perform
+  /// the rewrite.
+  ///
+  /// Note: Implementations must modify the IR if and only if the function
+  /// returns "success".
   virtual LogicalResult matchAndRewrite(Operation *op,
                                         PatternRewriter &rewriter) const = 0;
 

mlir/include/mlir/Transforms/DialectConversion.h

@@ -536,6 +536,12 @@ template <typename PatternT>
 class ConversionSplitMatchAndRewriteImpl : public PatternT {
   using PatternT::PatternT;
 
+  /// Attempt to match against IR rooted at the specified operation, which is
+  /// the same operation kind as getRootKind().
+  ///
+  /// Note: This function must not modify the IR.
+  virtual LogicalResult match(typename PatternT::OperationT op) const = 0;
+
   /// Rewrite the IR rooted at the specified operation with the result of
   /// this pattern, generating any new operations with the specified
   /// rewriter.
@@ -560,17 +566,6 @@ class ConversionSplitMatchAndRewriteImpl : public PatternT {
     }
   }
 
-  /// Attempt to match against code rooted at the specified operation,
-  /// which is the same operation code as getRootKind().
-  virtual LogicalResult match(typename PatternT::OperationT op) const = 0;
-
-  LogicalResult
-  matchAndRewrite(typename PatternT::OperationT op,
-                  typename PatternT::OpAdaptor adaptor,
-                  ConversionPatternRewriter &rewriter) const final {
-    llvm_unreachable("1:1 matchAndRewrite entry point is never used");
-  }
-
   LogicalResult
   matchAndRewrite(typename PatternT::OperationT op,
                   typename PatternT::OneToNOpAdaptor adaptor,
@@ -581,6 +576,17 @@ class ConversionSplitMatchAndRewriteImpl : public PatternT {
     }
     return failure();
   }
+
+  LogicalResult
+  matchAndRewrite(typename PatternT::OperationT op,
+                  typename PatternT::OpAdaptor adaptor,
+                  ConversionPatternRewriter &rewriter) const final {
+    // Users would normally override this function in conversion patterns to
+    // implement a 1:1 pattern. Patterns that are derived from this class have
+    // separate `match` and `rewrite` functions, so this `matchAndRewrite`
+    // overload is obsolete.
+    llvm_unreachable("this function is unreachable");
+  }
 };
 } // namespace detail