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

[Gil Rapaport]

aniragil wrote:

> I'd add some background, since @EtoAndruwa works on my team.
> 
> The only reason we went for `cf.switch` is that emitc already has `cf.br` and `cf.cond_br`. In general, I'd favor an `emitc.switch` operation which always lowers without `goto` and uses simple `break`, so the `case` labels are not terminators and you can not _jump_ from them, so it'll look more like `emitc.if`, from what I understand in terms of child basic blocks.

Thanks for the context, this indeed clarifies things!
Agreed, better to keep control flow structured where possible. The `emitc.switch` I had in mind would follow the semantics of `scf.index_switch`, which is less than what a C `switch` statement can do, e.g. fall-through, but captures exactly what MLIR's `scf` dialect models (we followed the same logic when we added `emitc.if` and `emitc.for`). So it makes sense to me to define `emitc.switch` as terminating each case with a `break` statement.

> 
> In general, I'd prefer if `cf` dialect will lower to `emitc` dialect first and then be used by emitter, since it'll result in improved code gen and you won't need to account for `cf` when writing passes, since you can require lowering to `emitc` first.

Agreed. If some emitc pass must take unstructured control flow into account it could either analyze the emitc control flow or be executed before cf-to-emitc.

> I wonder how the `scf.yield` will be lowered (though, I guess you can just specify that you can not `yield` result and say that each `case` should end with `emitc.break`, which yields nothing), though, you can likely define a variable just before the `switch` and assign to it, but all in all, `scf.index_swicth` like `emitc.switch` looks more appealing to us for already said `goto` stuff above, so +1 here.

We lower `scf.yield`'s return/iter args as part of `scf.if`/`scf.for` using `emitc.variables` which get defined before the respective `emitc.if`/`emitc.for` op and get assigned to in the then/else/body regions (`emitc.if` and `emitc.for` indeed cannot return any value). A similar lowering should work for `emitc.switch`, where each case would end with a nullary `emitc.yield`.


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