[Mlir-commits] [flang] [mlir] [mlir][CSE] Introduce hoist-pure-ops logic to CSE pass (PR #180556)

[lonely eagle]

linuxlonelyeagle wrote:

> CSE has `markAnalysesPreserved<DominanceInfo, PostDominanceInfo>()`, is this still correct? Before we were just deleting redundant expression, now we're moving across blocks which likely invalidate it? We should detect when operations move across blocks and not preserve the analysis when it happens (or update it?).
> 
> Also I'm not sure about test coverage, you had to update many tests, but it's not clear that we cover all the edge cases. Do we have tests covering hoisting blocked at IsIsolatedFromAbove in various conditions? I would suggest adding new minimal tests specifically targeted for this CSE hoisting.

Thank you for taking the time to review this and for your insightful questions. They provide a great opportunity for me to reflect further, and I will give them serious consideration.

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