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

[Hocky Yudhiono]

================
@@ -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;
+    }
----------------
hockyy wrote:

> I feel that the RegionPatternRewriteDriver should operate on the region it is provided with, that is its scope.

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.

If we do `setScope` here using the internal API for scope_a, scope_b will get CSE'd

```
module {
  func.func @scope_a(%x: i32, %y: i32) -> i32 {
    %a = arith.subi %x, %y : i32
    %b = arith.subi %x, %y : i32
    %c = arith.addi %a, %b : i32
    return %c : i32
  }
  func.func @scope_b(%x: i32, %y: i32) -> i32 {
    %a = arith.subi %x, %y : i32
    %b = arith.subi %x, %y : i32
    %c = arith.addi %a, %b : i32
    return %c : i32
  }
}
```

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