[Mlir-commits] [mlir] [MLIR][SCF] Sink scf.if from scf.while before region into after region. (PR #165216)

[Mehdi Amini]

================

----------------
joker-eph wrote:

> Do you really think adding result values to scf.while is fine? From my perspective, it introduces dead values, and they cannot be eliminated by CSE. I can’t accept the idea that introducing undeletable dead values during compilation is risk-free.

These aren't "dead-values" really... They come necessary from the structure of the `scf.while`, if anything this is a deficiency of `scf.while` design.
I don't quite see what CSE has to do with these?
What concrete problem do you expect from this actually?

> This already makes me question whether the design of scf.while is reasonable—why does it use two regions instead of two basic blocks?

I suspect only to be able to keep "single block regions". We could have had a CFG with two terminators: scf.continue and scf.break to differentiate the looping case from the exit case.

By the way, the `scf.loop` I'm introducing here would not have the same issue as `scf.while`: https://github.com/llvm/llvm-project/pull/166688/files#diff-49eb351905b55963bf6871078619f7511e32362d729b2e69f7a59b4b4b204056


If you're interest is to pattern match the specific pattern produced by `lift-cf-to-scf`, then it may be a better fit in this pass than as a canonicalization because of this implicit coupling.

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