[MLIR] Add mpi.comm type to MPI dialect
#125361
Conversation
|
Sorry I have very little familiarity with MPI idioms.
|
|
Thanks @mofeing for your exciting work. Sorry for the late reply. I think adding a new type for the Communicator and an operation for MPI_COMM_WORLD/MPI_COMM_SELF is a good approach. A global doesn't look like the right abstraction to me. With a dedicated Communicator type, I suggest closely following MPI and make the communicator mandatory in the IR. We might consider adding builders which do not accept a communicator (and use MPI_COMM_WORLD). As @Moxinilian suggested, you need to add the communicator argument to all places where the modified operations are created. By default, the builders accepts arguments in the order as the inputs are defined in the .td file. |
|
great, thanks both for your comments. i'm quite busy right now so i'll try to apply your comments in the following weeks. |
|
Hi @mofeing, I need exactly the features you are adding here to lower |
|
hey @fschlimb next week im on holidays so i'll be unable to do any change until 1 week later. there are just a few changes left to be ready. mainly making communicator argument mandatory, updating the assembly format and fixing the lowering from mesh. it's short time, but if you have a patch for these changes tomorrow i'll be happy to merge them. |
|
Hi @mofeing, At least one issue remains: The @rolfmorel Any idea? |
|
closing in favor of #133280 |
This is replacing #125361 - communicator is mandatory - new mpi.comm_world - new mp.comm_split - lowering and test --------- Co-authored-by: Sergio Sánchez Ramírez <[email protected]>
This is replacing llvm#125361 - communicator is mandatory - new mpi.comm_world - new mp.comm_split - lowering and test --------- Co-authored-by: Sergio Sánchez Ramírez <[email protected]>
cc @wsmoses @tobiasgrosser @hhkit
There are currently 3 problems for which I need help:
i don't know how to represent a constant in MLIR. not something like a
arith.constant; I'm referring toMPI_COMM_WORLD. what I did instead is create ampi.comm_worldop that returns ampi.commvalue, but not sure if it's the best. furthermore, it would be nice if we make optionalMPI_Commarguments to have a default value which is exactlyMPI_COMM_WORLDor the result ofmpi.comm_world.The
assemblyFormatofMPI_BarrierOp. Since it has one optional input argument and one optional result, I think it should support the following assembly formats:but this imposes some problems on how to parse it. specifically, parsing
->just in the case the two optional arguments are present. also, the 2nd and the 3rd case look confusing because in the 2nd the only type is the result type, while in the 3rd type is the input argument type.what are your opinions on this?
MPI_Commargument that previously didn't, so the op creation functions are different and it's breaking the Mesh to MPI conversion. i guess that we just need to add some morecreatefunctions or update that conversion, but I would appreciate a lil help here since I'm not very famliar with the C++ side of MLIR.