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

[Simon Camphausen]

simon-camp wrote:

> > > I'll wait on the opinion of @aniragil and/or @mgehre-amd before approving.
> > > On the one hand we've removed support for several non EmitC ops from the emitter in recent months by converting these to the Emitc dialect. On the other hand we don't have EmitC specific ops for branch like instructions the cf dialect could be lowered to yet.
> > 
> > 
> > I agree with @simon-camp here, not sure if we want to add support for further non-EmitC ops to the emitter. I would therefore love to hear @mgehre-amd's and @aniragil's option. If we want to add this, please also also adjust https://github.com/llvm/llvm-project/blob/main/mlir/docs/Dialects/emitc.md accordingly as part of your PR.
> 
> I share @simon-camp's and @marbre's concerns about extending the translator. How about the following alternative then: Lower `cf.switch` to a new `emitc.switch` op (which would also be useful for lowering `scf.switch`). Within the `emitc.switch` cases we'll generate `cf.br` ops which the translator already supports. The `cf.br` ops will carry the `cf.switch` cases' block arguments. Similarly, we can lower `cf.cond_br` to the existing `emitc.if` op, leaving the translator to handle only `cf.br` until we model blocks/labels and goto's propely.
> 
> We can implement this in either order:
> 
> * First move the existing `cf.cond_br` support to a new CFToEmitC lowering pass, then add an `emitc.switch` op and extend the pass to lower `cf.switch` into it.
> * First add an `emitc.switch` op with a new CFToEmitC lowering pass which uses it for `cf.switch`, then move the existing `cf.cond_br` support there.
> 
> WDYT?

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 would lose the terminator by simple lowering these ops to ifs.

The end goal is fine, I just can't estimate how much work this involves.

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