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

[Marius Brehler]

marbre wrote:

> Adding an additional attribute later would not break compatibility. And we don't know yet if we actually going to get this, right?

Yes, that sounds reasonable to me.

> > 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.
> 
> I'm opposed to recursive verification in `::verify` as this is very costly and `::verify` gets called a lot. Others have done this heavy validation via separate validation passes (e.g. TOSA).

Yes, with `Adding a validation pass` in my comment above I was referring to a validation pass like the one TOSA has to check for compliance with a TOSA spec. A downstream user would still need to run this pass (or add it to a pipeline) but that should be acceptable.

> > Any other potential uses for this op?
> 
> I think we should be pragmatic here. A source file is a source file. If we need different more elaborate constructs, there is always the option to add new ops, or change existing onces. MLIR/emitc have not been backwards compatible, so we shouldn't make it too hard for us.

:+1: 
 
> In summary, I see consensus to rename this to `emitc.file`.
> 
> Is there any objection to afterwards get this merged? I heard many ideas about possible extensions, but I don't see how the current design (an operation with an identifier) would make any of them impossible.

No objections. I would say, lets go ahead with `emitc.file`.

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