[mlir] Improve GreedyPatternRewriteDriver and pass documentation (#77614)

Clarify what kind of IR modifications are allowed. Also improve the
documentation of the greedy rewrite driver entry points.

Addressing comments in #76219.

Author: matthias-springer

mlir/docs/PassManagement.md

@@ -17,22 +17,26 @@ this is a great place to start.
 
 In MLIR, the main unit of abstraction and transformation is an
 [operation](LangRef.md/#operations). As such, the pass manager is designed to
-work on instances of operations at different levels of nesting. The structure of
-the [pass manager](#pass-manager), and the concept of nesting, is detailed
-further below. All passes in MLIR derive from `OperationPass` and adhere to the
-following restrictions; any noncompliance will lead to problematic behavior in
-multithreaded and other advanced scenarios:
-
-*   Must not modify any state referenced or relied upon outside the current
-    operation being operated on. This includes adding or removing operations
-    from the parent block, changing the attributes(depending on the contract
-    of the current operation)/operands/results/successors of the current operation.
-*   Must not modify the state of another operation not nested within the current
-    operation being operated on.
-    *   Other threads may be operating on these operations simultaneously.
-*   Must not inspect the state of sibling operations.
+work on instances of operations at different levels of nesting. In the following
+paragraphs, we refer to the operation that a pass operates on as the "current
+operation".
+
+The structure of the [pass manager](#pass-manager), and the concept of nesting,
+is detailed further below. All passes in MLIR derive from `OperationPass` and
+adhere to the following restrictions; any noncompliance will lead to problematic
+behavior in multithreaded and other advanced scenarios:
+
+*   Must not inspect the state of operations that are siblings of the current
+    operation. Must neither access operations nested under those siblings.
     *   Other threads may be modifying these operations in parallel.
     *   Inspecting the state of ancestor/parent operations is permitted.
+*   Must not modify the state of operations other than the operations that are
+    nested under the current operation. This includes adding, modifying or
+    removing other operations from an ancestor/parent block.
+    *   Other threads may be operating on these operations simultaneously.
+    *   As an exception, the attributes of the current operation may be modified
+        freely. This is the only way that the current operation may be modified.
+        (I.e., modifying operands, etc. is not allowed.)
 *   Must not maintain mutable pass state across invocations of `runOnOperation`.
     A pass may be run on many different operations with no guarantee of
     execution order.

mlir/include/mlir/Transforms/GreedyPatternRewriteDriver.h

@@ -62,7 +62,8 @@ class GreedyRewriteConfig {
 
   /// Only ops within the scope are added to the worklist. If no scope is
   /// specified, the closest enclosing region around the initial list of ops
-  /// is used as a scope.
+  /// (or the specified region, depending on which greedy rewrite entry point
+  /// is used) is used as a scope.
   Region *scope = nullptr;
 
   /// Strict mode can restrict the ops that are added to the worklist during
@@ -86,30 +87,54 @@ class GreedyRewriteConfig {
 //===----------------------------------------------------------------------===//
 
 /// Rewrite ops in the given region, which must be isolated from above, by
-/// repeatedly applying the highest benefit patterns in a greedy work-list
-/// driven manner.
+/// repeatedly applying the highest benefit patterns in a greedy worklist
+/// driven manner until a fixpoint is reached.
 ///
-/// 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.
+/// The greedy rewrite may prematurely stop after a maximum number of
+/// iterations, which can be configured in the configuration parameter.
 ///
-/// Return success if the iterative process converged and no more patterns can
-/// be matched in the result operation regions. `changed` is set to true if the
-/// IR was modified at all.
+/// Also performs folding and simple dead-code elimination before attempting to
+/// match any of the provided patterns.
 ///
-/// 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.
+/// A region scope can be set in the configuration parameter. By default, the
+/// scope is set to the specified region. Only in-scope ops are added to the
+/// worklist and only in-scope ops are allowed to be modified by the patterns.
 ///
-/// You may configure several aspects of this with GreedyRewriteConfig.
+/// Returns "success" if the iterative process converged (i.e., fixpoint was
+/// reached) and no more patterns can be matched within the region. `changed`
+/// is set to "true" if the IR was modified at all.
+///
+/// Note: This method does not apply patterns to the region's parent operation.
 LogicalResult
 applyPatternsAndFoldGreedily(Region &region,
                              const FrozenRewritePatternSet &patterns,
                              GreedyRewriteConfig config = GreedyRewriteConfig(),
                              bool *changed = nullptr);
 
-/// Rewrite ops in all regions of the given op, which must be isolated from
-/// above.
+/// Rewrite ops nested under the given operation, which must be isolated from
+/// above, by repeatedly applying the highest benefit patterns in a greedy
+/// worklist driven manner until a fixpoint is reached.
+///
+/// The greedy rewrite may prematurely stop after a maximum number of
+/// iterations, which can be configured in the configuration parameter.
+///
+/// Also performs folding and simple dead-code elimination before attempting to
+/// match any of the provided patterns.
+///
+/// This overload runs a separate greedy rewrite for each region of the
+/// specified op. A region scope can be set in the configuration parameter. By
+/// default, the scope is set to the region of the current greedy rewrite. Only
+/// in-scope ops are added to the worklist and only in-scope ops and the
+/// specified op itself are allowed to be modified by the patterns.
+///
+/// Note: The specified op may be modified, but it may not be removed by the
+/// patterns.
+///
+/// Returns "success" if the iterative process converged (i.e., fixpoint was
+/// reached) and no more patterns can be matched within the region. `changed`
+/// is set to "true" if the IR was modified at all.
+///
+/// Note: This method does not apply patterns to the given operation itself.
 inline LogicalResult
 applyPatternsAndFoldGreedily(Operation *op,
                              const FrozenRewritePatternSet &patterns,
@@ -129,27 +154,34 @@ applyPatternsAndFoldGreedily(Operation *op,
   return failure(failed);
 }
 
-/// Applies the specified rewrite patterns on `ops` while also trying to fold
-/// these ops.
+/// Rewrite the specified ops by repeatedly applying the highest benefit
+/// patterns in a greedy worklist driven manner until a fixpoint is reached.
+///
+/// The greedy rewrite may prematurely stop after a maximum number of
+/// iterations, which can be configured in the configuration parameter.
+///
+/// Also performs folding and simple dead-code elimination before attempting to
+/// match any of the provided patterns.
 ///
 /// Newly created ops and other pre-existing ops that use results of rewritten
-/// ops or supply operands to such ops are simplified, unless such ops are
+/// ops or supply operands to such ops are also processed, unless such ops are
 /// excluded via `config.strictMode`. Any other ops remain unmodified (i.e.,
 /// regardless of `strictMode`).
 ///
 /// In addition to strictness, a region scope can be specified. Only ops within
 /// the scope are simplified. This is similar to `applyPatternsAndFoldGreedily`,
-/// where only ops within the given regions are simplified. If no scope is
-/// specified, it is assumed to be the first common enclosing region of the
-/// given ops.
+/// where only ops within the given region/op are simplified by default. If no
+/// scope is specified, it is assumed to be the first common enclosing region of
+/// the given ops.
 ///
 /// Note that ops in `ops` could be erased as result of folding, becoming dead,
 /// or via pattern rewrites. If more far reaching simplification is desired,
-/// applyPatternsAndFoldGreedily should be used.
+/// `applyPatternsAndFoldGreedily` should be used.
 ///
-/// Returns success if the iterative process converged and no more patterns can
-/// be matched. `changed` is set to true if the IR was modified at all.
-/// `allOpsErased` is set to true if all ops in `ops` were erased.
+/// Returns "success" if the iterative process converged (i.e., fixpoint was
+/// reached) and no more patterns can be matched. `changed` is set to "true" if
+/// the IR was modified at all. `allOpsErased` is set to "true" if all ops in
+/// `ops` were erased.
 LogicalResult
 applyOpPatternsAndFold(ArrayRef<Operation *> ops,
                        const FrozenRewritePatternSet &patterns,