[Mlir-commits] [mlir] [mlir][emitc][cf] add 'cf.switch' support in CppEmitter (PR #101478)

[Gil Rapaport]

aniragil wrote:

> I don't think this that's going to work as is. You can't branch from a region to a basic block in a parent region (I think). Additionally `cf.cond_br` and `cf.switch` are terminators whereas `emitc.if` is not. So we would lose the terminator by simply lowering these ops to ifs.

Hmm, good points. So our options is either to

(a) First replace translator support for `cf` by lowering `cf` into emitc, e.g. by adding a pair of `emitc.label`, `emitc.goto` ops, then add an `emitc.switch` op and support `cf.switch`.

(b) First support `cf.switch` in the translator, then add an `emitc.switch` op by lowering `scf.index_switch` into it, then move all `cf` support from the translator to a lowering pass.

As @marbre mentioned, option (a) is better as it only involves "going forward" with emitc's design, tought it would take a bit more time. Also, option (b) risks changing the emitted code later when we move `cf` lowering to a pass (e.g. if we end up lowering `cf.switch` to cascading block & `cond_br` ops for whatever reason). So if we can come up in a reasonable time with a design for lowering branches and blocks to emitc I'd go for this option.
That being said, option (b) doesn't seem like a big step back as it's a clean extension of the existing mechanism and I'm OK with changing the emitted code later (though I doubt it would be needed), so I'm OK with going with option (b) if @simon-camp and @marbre expect lowering branches and blocks to take long or be complex.

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