[Mlir-commits] [mlir] [MLIR][EmitC] Emit private functions as static (PR #78336)

[Marius Brehler]

marbre wrote:

> > Why isn't private mapping to static for functions?
> 
> I guess the mapping is not one-to-one: `static` from C doesn't necessarily map directly to `private`, although I think `private` maps to `static` perfectly fine in this case.

Yeah, that is what I had in mind. I think adding an `emitc.call` op makes sense anyway as it would allow to emit further keywords such as `inline`, the combination `static inline` as well as further keywords like `__device__`, `__host__` and `__global__` with view to emit CUDA code.

> I agree that it doesn't map directly, OTOH I'm seeing CPP target as an approximation. Re: different implementation: that approach sounds good to me and totally makes sense. I don't need this functionality urgently (I can work with a local patch in the meantime), but it would be nice to get static functions in eventually.

I already restarted my work on an `emitc.func` but this also requires me to implement an `emitc.return` (`func.return` is restricted to have `func.func` as parent) and also a **new** `emitc.call` op (the `emitc.call` op we previously had was renamed to `emitc.call_opaque` back in November with c4fd1fd). 

> Would it also make sense to get this functionality in and then migrate to `emitc.func`? Just trying to avoid this slipping through the cracks 😅

After several discussions there seemed to be kind of conses that one should rather extend the dialect and do what is necessary within conversions instead of extending support in the emitter for non-EmitC functions. Extending `func.func` support isn't a big thing and the patch is quite tiny, but I still would prefer to keep the way we go for other options as well. For example `scf.for` support in the emitter was replaced with an `emitc.for` op implementation and an SCF to EmitC conversion with 2633d94.
Would it be okay if we only accept your patch if I am unable to complete my work within a reasonable time?

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