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.

37

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.

39

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"
 
 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:
+/// work-list driven manner.
+/// This variant may stop after a predefined number of iterations, see the
+/// alternative below to provide a specific number of iterations before stopping
+/// in absence of convergence.
+/// Return success if the iterative process converged and no more patterns can
+/// be matched in the result operation regions.
+/// Note: This does not apply patterns to the top-level operation itself.
 ///       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);
+
+/// Rewrite the regions of the specified operation, with a user-provided limit
+/// on iterations to attempt before reaching convergence.
+LogicalResult
+applyPatternsAndFoldGreedily(Operation *op,
+                             const FrozenRewritePatternList &patterns,
+                             unsigned maxIterations);
+
 /// Rewrite the given regions, which must be isolated from above.
 LogicalResult
 applyPatternsAndFoldGreedily(MutableArrayRef<Region> regions,
                              const FrozenRewritePatternList &patterns);
 
+/// Rewrite the given regions, with a user-provided limit on iterations to
+/// attempt before reaching convergence.
+LogicalResult
+applyPatternsAndFoldGreedily(MutableArrayRef<Region> regions,
+                             const FrozenRewritePatternList &patterns,
+                             unsigned maxIterations);
+
 /// 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

 /// 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) {
-  return applyPatternsAndFoldGreedily(op->getRegions(), patterns);
+  return applyPatternsAndFoldGreedily(op, patterns, maxPatternMatchIterations);
+}
+LogicalResult
+mlir::applyPatternsAndFoldGreedily(Operation *op,
+                                   const FrozenRewritePatternList &patterns,
+                                   unsigned maxIterations) {
+  return applyPatternsAndFoldGreedily(op->getRegions(), patterns,
+                                      maxIterations);
 }
 /// Rewrite the given regions, which must be isolated from above.
 LogicalResult
 mlir::applyPatternsAndFoldGreedily(MutableArrayRef<Region> regions,
                                    const FrozenRewritePatternList &patterns) {
+  return applyPatternsAndFoldGreedily(regions, patterns,
+                                      maxPatternMatchIterations);
+}
+LogicalResult
+mlir::applyPatternsAndFoldGreedily(MutableArrayRef<Region> regions,
+                                   const FrozenRewritePatternList &patterns,
+                                   unsigned maxIterations) {
   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, maxIterations);
   LLVM_DEBUG(if (!converged) {
     llvm::dbgs() << "The pattern rewrite doesn't converge after scanning "
-                 << maxPatternMatchIterations << " times";
+                 << maxIterations << " times";
   });
   return success(converged);
 }
 
 //===----------------------------------------------------------------------===//
 // OpPatternRewriteDriver
 //===----------------------------------------------------------------------===//