Optional argument for pattern rewriter max iteration count.

[Augusteijn]

Summary

Some rewriters take more iterations to converge,
add a parameter to overwrite the built-in maximum iteration count.

[rriddle]

Thanks for the patch! Can you please run clang-format?

mlir/include/mlir/Transforms/GreedyPatternRewriteDriver.h

19

Please avoid the use of macros for things like this.

36

Can you just add an additional overload that accepts a max iteration count? The current overload could just call that one with the default. That would remove the need for exposing the default publicly.

38

MLIR uses camelCase for variable names.

[Augusteijn]

mlir/include/mlir/Transforms/GreedyPatternRewriteDriver.h

19

clang-format does not work for me:
YAML:2:34: error: invalid boolean

AlwaysBreakTemplateDeclarations: Yes

[Augusteijn]

1. Updating D91553: Optional argument for pattern rewriter max iteration count. #
2. Enter a brief description of the changes included in this update.
3. The first line is used as subject, next lines as comment. #
4. If you intended to create a new revision, use:
5. $ arc diff --create

Do not expose default max iteration count

[mehdi_amini]

mlir/include/mlir/Transforms/GreedyPatternRewriteDriver.h

19

Seems like you have an old version of clang-format.

[mehdi_amini]

I tweaked slightly the doc and the commit message:
We like to tag commit messages with "NFC" when they don't modify an existing behavior and don't include tests (this change is a bit borderline, it *could* be plumbed and tested with a pass)

mlir/include/mlir/Transforms/GreedyPatternRewriteDriver.h

 //
 //===----------------------------------------------------------------------===//
 
 #ifndef MLIR_TRANSFORMS_GREEDYPATTERNREWRITEDRIVER_H_
 #define MLIR_TRANSFORMS_GREEDYPATTERNREWRITEDRIVER_H_
 
 #include "mlir/Rewrite/FrozenRewritePatternList.h"
 
+#define MAX_PATTERN_MATCH_ITERATIONS 10
+
 namespace mlir {
 
 //===----------------------------------------------------------------------===//
 // applyPatternsGreedily
 //===----------------------------------------------------------------------===//
 
 /// Rewrite the regions of the specified operation, which must be isolated from
 /// above, by repeatedly applying the highest benefit patterns in a greedy
 /// work-list driven manner. Return success if no more patterns can be matched
 /// in the result operation regions.
 /// Note: This does not apply patterns to the top-level operation itself. Note:
 ///       These methods also perform folding and simple dead-code elimination
 ///       before attempting to match any of the provided patterns.
 ///
 LogicalResult
 applyPatternsAndFoldGreedily(Operation *op,
-                             const FrozenRewritePatternList &patterns);
+                             const FrozenRewritePatternList &patterns,
+                 unsigned max_iterations = MAX_PATTERN_MATCH_ITERATIONS);
 /// Rewrite the given regions, which must be isolated from above.
 LogicalResult
 applyPatternsAndFoldGreedily(MutableArrayRef<Region> regions,
-                             const FrozenRewritePatternList &patterns);
+                             const FrozenRewritePatternList &patterns,
+                 unsigned max_iterations = MAX_PATTERN_MATCH_ITERATIONS);
 
 /// Applies the specified patterns on `op` alone while also trying to fold it,
 /// by selecting the highest benefits patterns in a greedy manner. Returns
 /// success if no more patterns can be matched. `erased` is set to true if `op`
 /// was folded away or erased as a result of becoming dead. Note: This does not
 /// apply any patterns recursively to the regions of `op`.
 LogicalResult applyOpPatternsAndFold(Operation *op,
                                      const FrozenRewritePatternList &patterns,
                                      bool *erased = nullptr);
 
 } // end namespace mlir
 
 #endif // MLIR_TRANSFORMS_GREEDYPATTERNREWRITEDRIVER_H_

mlir/lib/Transforms/Utils/GreedyPatternRewriteDriver.cpp

 #include "llvm/Support/Debug.h"
 #include "llvm/Support/raw_ostream.h"
 
 using namespace mlir;
 
 #define DEBUG_TYPE "pattern-matcher"
 
 /// The max number of iterations scanning for pattern match.
-static unsigned maxPatternMatchIterations = 10;
+static unsigned maxPatternMatchIterations = MAX_PATTERN_MATCH_ITERATIONS;
 
 //===----------------------------------------------------------------------===//
 // GreedyPatternRewriteDriver
 //===----------------------------------------------------------------------===//
 
 namespace {
 /// This is a worklist-driven driver for the PatternMatcher, which repeatedly
 /// applies the locally optimal patterns in a roughly "bottom up" way.

 }
 
 /// Rewrite the regions of the specified operation, which must be isolated from
 /// above, by repeatedly applying the highest benefit patterns in a greedy
 /// work-list driven manner. Return success if no more patterns can be matched
 /// in the result operation regions. Note: This does not apply patterns to the
 /// top-level operation itself.
 ///
 LogicalResult
 mlir::applyPatternsAndFoldGreedily(Operation *op,
-                                   const FrozenRewritePatternList &patterns) {
+                                   const FrozenRewritePatternList &patterns,
+           unsigned max_iterations /*= MAXPATTERNMATCHITERATIONS*/) {
   return applyPatternsAndFoldGreedily(op->getRegions(), patterns);
 }
 /// Rewrite the given regions, which must be isolated from above.
 LogicalResult
 mlir::applyPatternsAndFoldGreedily(MutableArrayRef<Region> regions,
-                                   const FrozenRewritePatternList &patterns) {
+                                   const FrozenRewritePatternList &patterns,
+           unsigned max_iterations /*= MAXPATTERNMATCHITERATIONS*/) {
   if (regions.empty())
     return success();
 
   // The top-level operation must be known to be isolated from above to
   // prevent performing canonicalizations on operations defined at or above
   // the region containing 'op'.
   auto regionIsIsolated = [](Region &region) {
     return region.getParentOp()->isKnownIsolatedFromAbove();
   };
   (void)regionIsIsolated;
   assert(llvm::all_of(regions, regionIsIsolated) &&
          "patterns can only be applied to operations IsolatedFromAbove");
 
   // Start the pattern driver.
   GreedyPatternRewriteDriver driver(regions[0].getContext(), patterns);
-  bool converged = driver.simplify(regions, maxPatternMatchIterations);
+  bool converged = driver.simplify(regions, max_iterations);
   LLVM_DEBUG(if (!converged) {
     llvm::dbgs() << "The pattern rewrite doesn't converge after scanning "
-                 << maxPatternMatchIterations << " times";
+                 << max_iterations << " times";
   });
   return success(converged);
 }
 
 //===----------------------------------------------------------------------===//
 // OpPatternRewriteDriver
 //===----------------------------------------------------------------------===//