[Mlir-commits] [mlir] [MLIR] emitc: Add emitc translation unit op (PR #123298)

[Gil Rapaport]

aniragil wrote:

> > Thinking about the semantics of the proposed op, such a file-scope `emitc` op may be useful beyond the functionality introduced by this patch, e.g. for
> > (a) Specifying the C dialect the `emitc` code belongs to, e.g. in order for the translator to know how to emit vector types when targeting OpenCL, GCC vector extensions etc.
> 
> I think this needs to be modeled in the dialect. With this you wouldn't pass a flag to the emitter but of course the emitter would need to know how to handle those ops with dialect information.

Exactly. It could also guide the lowering process, e.g. by scalarizing vectors for C, implementing tuples as `std::tuple` for C++ or as structs for C etc.

> 
> > (b) For offload/launch semantics such as in `gpu.launch_func`/`gpu.module`, which could prove useful for OpenCL, OpenMP etc.
> 
> Not sure I see how this is related to a file-scope as proposed by this patch.

So in order to support `gpu.launch_func`, `gpu.module` is a symbol. We could add this later, but we might want to plan ahead. For instance, if we allow any string as the proposed `id` it might later be unusable as a valid symbol name. We could then add a second attribute for the symbol name or pose restrictions on the `id` attribute, with both options breaking backward compatibility of existing MLIR files.

Another aspect is the necessity of a dedicated `emitc` op. If its sole purpose is to provide a named scope, would the builtin module suffice (as IINM in the LLVM dialect)?

> 
> > (c) Validation: Utilizing standard op validation to verify that all its internal ops are `emitc` and potentially that these ops and types are valid in its designated C-dialect (e.g. vector types, address spaces, templates).
> 
> Adding a validation pass is something I started some time ago but didn't had time to finish. So yes, +1 from me. However, does it influence changed introduced by this patch?

So I was thinking whether the op should have a ::verify() method. Unless the translator itself runs such a verifying pass (which I'm not sure translators are supposed to do) this pass would be optional, letting users call the translator on invalid files. If we implement ::verify() the op would have to be introduced after all lowering to `emitc` has been done. If we do this later on, we might be breaking downstream compilers which expected it to be usable earlier.

> 
> > Would be good to discuss this op's designated uses to make sure we don't introduce backward compatibility later. WDYT?
> 
> Makes sense, yes.

Any other potential uses for this op?

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