[Mlir-commits] [llvm] [mlir] [mlir][tblgen] Expose more of MLIRTableGen as library (PR #92709)

[River Riddle]

River707 wrote:

> > (other tools got refactored because of the motivation to expose some APIs publicly / to downstream)
> 
> I'm not sure that holds for mlir-lsp-server :)
> 
> We already have a list of APIs exposed for mlir-tblgen. This does expose more and I think valid question as to are too much exposed (I haven't looked yet) and what are the expectations around these. Especially as it is just generating code which is not done in too general a way.
> 
> But there is also a different question being asked here: if downstream users want to add new features here & experiment, should they just work, do all their dev and come have the conversation later? Esp if experimental it may not be desirable upstream but if fully fork downstream then incentive/cost of making improvements/moving parts up is low/high (resp.).
> 
> I think your guard rail here is that this should not be considered as invitation to make tools that add other semantics to the ops (I don't think upstream or downstream matters much here). Is that correct? The current set up doesn't stop downstream forks of the ODS code, I could see from a maintenance cost it dissuading, but given the speed at which these move, its not a high cost. So it seems to be more of a policy discussion.

I don't think it's a good comparison for tools that load MLIR dialects and agnostic tools. The tools using MLIR dialects (like mlir-lsp-server) were generalized because they had to be (i.e. how do I load my dialect without it being a library?). The tablegen tools IMO have mostly been operating under a good development cycle -> The emitters that generate c++ are in the end tool, whereas the wrappers around the TableGen API are in libraries. I don't yet understand the goal or value of moving the emitters to a library (because I don't understand what value that gives you?), as opposed to just invoking mlir-tblgen in your build. 

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