[Mlir-commits] [flang] [mlir] [mlir][CSE] Add pruneDeadOps to CSE pass (PR #193778)

[lonely eagle]

================
@@ -294,15 +297,38 @@ LogicalResult CSEDriver::simplifyOperation(ScopedMapTy &knownValues,
   return failure();
 }
 
+void CSEDriver::pruneDeadOps(Operation *root, ScopedMapTy &knownValues) {
+  // We use `SetVector` to prevent already inserted ops from being added to the
+  // `worklist` repeatedly, avoiding secondary access to erased operations.
+  llvm::SetVector<Operation *> worklist;
----------------
linuxlonelyeagle wrote:

> Why do you think it makes sense?

Consistent with the PR description and the provided test cases, this implementation further optimization for CSE. There's no need to run trivial-dce or canonicalize beforehand. In my view, we only need to visit the ops once during CSE and can handle the DCE work along the way.

 > Do you have a case not covered by applying a trivial DCE before CSE? That is one example for my previous question: is there a case where a DCE opportunity only presents itself after CSE merged some operations?

Technically speaking, these examples are indeed present, and they have existed since the initial implementation. After running CSE, `%c1_dup = arith.constant 1 : i32` becomes a dead op. However, it doesn't have any operands. Even if it did, there's no need to recursively delete the def-ops of those operands, because they are shared by the identical existingOp.

```
func.func @example() -> i32 {
  %c1 = arith.constant 1 : i32
  %c2 = arith.addi %c1, %c1 : i32
  %c1_dup = arith.constant 1 : i32
  %result = arith.addi %c1_dup, %c2 : i32
  return %result : i32
}
```

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