[Mlir-commits] [mlir] [mlir][Transforms] Fix crash in `-remove-dead-values` on private functions (PR #169269)

[Matthias Springer]

matthias-springer wrote:

I took a look at #158760. It is indeed similar. It's the same problem that I am fixing here, but for branch ops. My fix from this PR here won't help because the op is not a "simple" op.

I see 3 options to fix issues such as #158760 and the one from this PR:
1. Remove unreachable functions/symbols at the beginning.
2. Fix the way operations are erased, similar to what this PR does.
3. When erasing an SSA value that still has uses, insert a `ub.poison` operation.

I am not very happy with option 1 because it conceptually does not fit in well with `-remove-dead-values`. The `-remove-dead-values` pass is a data flow analysis and symbol/function DCE cannot be done with a data flow analysis. We could call the symbol DCE pass from this pass, but it would be nice (design-wise) to avoid calling another pass/functionality that is not fully dataflow-driven.

As for option 2, the problem in #158760 is that the condition value of a `cf.cond_br` is dead. We cannot just remove that op because the basic block would loose its terminator. Maybe we could insert an unconditional `cf.br` op. (Difficult because we match for `BranchOpInterface` and not specific ops.) Or we could insert a `ub.poison` for the condition.

Which brings me to option 3, which I would like to explore a bit more...


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