[Mlir-commits] [mlir] [mlir] Add option to run CSE between greedy rewriter iterations (PR #193081)

[Mehdi Amini]

================
@@ -899,6 +901,15 @@ LogicalResult RegionPatternRewriteDriver::simplify(bool *changed) && {
           }
         },
         {&region}, iteration);
+
+    // Optionally run full CSE between iterations. If CSE changes the IR we
+    // iterate again so that patterns can fire on the deduplicated operations.
+    if (config.isCSEBetweenIterationsEnabled()) {
+      DominanceInfo domInfo(region.getParentOp());
+      bool cseChanged = false;
+      eliminateCommonSubExpressions(rewriter, domInfo, region, &cseChanged);
+      continueRewrites |= cseChanged;
+    }
----------------
joker-eph wrote:

> It seems like it's not. https://github.com/llvm/llvm-project/blob/main/mlir/lib/Transforms/Utils/GreedyPatternRewriteDriver.cpp#L639 The addToWorkList here manually filters it.

You're looking at the parent class. I'm looking at the behavior for this class:

```
/// This driver simplfies all ops in a region.
class RegionPatternRewriteDriver : public GreedyPatternRewriteDriver {
public:
  explicit RegionPatternRewriteDriver(MLIRContext *ctx,
                                      const FrozenRewritePatternSet &patterns,
                                      const GreedyRewriteConfig &config,
                                      Region &regions);

  /// Simplify ops inside `region` and simplify the region itself. Return
  /// success if the transformation converged.
  LogicalResult simplify(bool *changed) &&;

private:
  /// The region that is simplified.
  Region ®ion;
};
```

It takes a region and `simplify` is documented as "Simplify ops inside `region` and simplify the region itself."

Also I mentioned above:

> This isn't clear to me: the code above performs simplifyRegion on the region itself. 

Which is consistent with the update.

https://github.com/llvm/llvm-project/pull/193081