[Mlir-commits] [mlir] [MLIR] Add `mpi.comm` type to MPI dialect (PR #125361)

[Théo Degioanni]

Moxinilian wrote:

Sorry I have very little familiarity with MPI idioms.

1. A constant makes sense when it could be multiple different things. Is it a sort of ID-like constant, where you could have many? If so, you should create an enum attribute `https://mlir.llvm.org/docs/DefiningDialects/Operations/#enum-attributes`, which you can attach to whatever operation needs it. If you need it in SSA (which I'm not sure you do, again, let me know if I misunderstand what you are modelling), you can create a constant op like in arith that takes in this enum attribute instead of say an integer attr.

2. I think you have two options here:
  - Make the syntax always an explicit function-like, for example `() -> (!mpi.retval)` or `(!mpi.retval) -> ()`. I think this is the best way.
  - If you are truly sure your shorter syntax is worth it and unambiguous, you can implement a custom assembly format by setting `hasCustomAssemblyFormat` to 1, and implementing the generated declarations.
 
3. Your new attribute probably added a new argument to the builder functions yes. To check what changed, I recommend looking at the generated C++ by tablegen (in the ops header declaration) to look at the `build` functions that have changed. You should be able to update the `create` calls so they map the arguments in those build functions (the first two arguments of `build` are added by the internals of `create`, what matters is the rest).

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