[Mlir-commits] [flang] [mlir] [MLIR] Refactor DCE helper to expose worklist entry-point, and use this helper in CSE (PR #195636)

[Hocky Yudhiono]

https://github.com/hockyy commented:

I found that the simplify flow is a bit hard to follow. If `simplifyRegion` always call the `simplifyBlock`, and `simplifyBlock` will recurse to `simplifyRegion` when discovering an operation with a region, can we make it clearer?

What if we create a frame like this:
```c++
  /// Per-`simplifyRegion` state. `notifyParent` is the one-step forward for
  /// trivially-dead ops that are not in `region` (usually the enclosing
  /// `ActiveRegionFrame::recordTriviallyDead`, or the public `simplify` hook
  /// at the root). The parent frame pointer is not stored here—only this
  /// indirection.
  struct ActiveRegionFrame {
    // should we also remove the `Region` parameter from `SimplifyRegion`(?)
    Region ®ion;
    SmallVector<Operation *> triviallyDeadOps;
    SmallVector<Operation *> deadOpsAfterCSE;
    OperationCallback notifyParent;
    // consider even putting the scopedMap itself here(?)

    void recordTriviallyDead(Operation *op);
    void recordCSEDuplicate(Operation *op);
  };

void CSEDriver::ActiveRegionFrame::recordTriviallyDead(Operation *op) {
  if (op->getParentRegion() == &region) {
    if (isOpTriviallyDead(op))
      triviallyDeadOps.push_back(op);
    return;
  }
  if (notifyParent)
    notifyParent(op);
}

void CSEDriver::ActiveRegionFrame::recordCSEDuplicate(Operation *op) {
  deadOpsAfterCSE.push_back(op);
}

```
We make the simplifyBlock to a reference based `regionFrame`:
```c++
  /// Simplify operations in \p bb.
  bool simplifyBlock(ScopedMapTy &knownValues, Block *bb, bool hasSSADominance,
                     ActiveRegionFrame &regionFrame);

  /// Simplify operations in \p region. \p parentFrame links nested regions.
  bool simplifyRegion(ScopedMapTy &knownValues, Region &region,
                      ActiveRegionFrame *parentFrame);
```
wdyt?

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