xdslproject / xdsl

A Python Compiler Design Toolkit
Other
255 stars 67 forks source link

xDSL: rename MLContext #249

Open webmiche opened 1 year ago

webmiche commented 1 year ago

The name MLContext was chosen in a time where we were closer to MLIR. We should rename it.

webmiche commented 1 year ago

Does anyone have a preference on how this should be named? I guess a name with two parts makes sense. First a descriptive part "for what", and then something indicating that it is a context.

For the latter, I would say we either use Context or Ctx.

For the former, I could imagine xDSL, IR, Dialect, Compilation (or Comp).

Some example configurations:

xDSLContext, IRContext, IRCtx, DialectContext, CompilationContext, CompCtx

Personally, I think CompContext or CompCtx is quite neat. What do you think? @georgebisbas @math-fehr

georgebisbas commented 1 year ago

xDSLCtx?

math-fehr commented 1 year ago

The reason why this is called MLIRContext in MLIR is that this contains a lot of information, such as the attributes currently used in a program, as well as the multithreading context. However, here, we use the MLContext exclusively to contain the registered dialects, and nothing else. I always thought we would add something else to it, but as for now I have nothing in mind. So maybe naming it RegisteredDialects or something similar would make sense as well? But I find this name ugly, so maybe XDSLCtx would make more sense, even though it does not represent well what it contains.

webmiche commented 1 year ago

AFAIK, the same thing in MLIR is called MLIRContext, so I guess xDSLContext, or xDSLCtx is the most straight-forward idea. In what sense do you think it fits less here? Because it only holds dialect information and nothing else?

math-fehr commented 1 year ago

Yeah, the MLIRContext also contains attributes contents, and many other things. For instance, in our case, we can check equality between operations in different context, but we can't in MLIR. But otherwise, I'm happy to use xDSLContext or xDSLCtx, that would work as well!

webmiche commented 1 year ago

I think xDSLCtx makes a lot of sense, in particular if we end up deciding to move more functionality into this context, which I deem likely. Then, users won't have to rewrite parts of their code to access this additional functionality.

superlopuh commented 1 year ago

Minor nit is that we should probably stick with UpperCamelCase and call it XDSLCtx or XDSLContext

webmiche commented 1 year ago

I see what you mean. Though the fact that the project itself is written xDSL makes the CamelCase look a bit dumb, IMO. Also, we already had this decision with xDSLOptMain, where we decided to use the non-strict CamelCase version.

Not saying that we need to do the same here, just that the two things should be consistent IMO. What do others think? IMO, xDSLCtx looks better than XDSLCtx, even though it isn't strictly CamelCase. Though I feel this difference is quite minimal, so I am fine with both...