-
Notifications
You must be signed in to change notification settings - Fork 119
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
WIP: Add the initial part of MPI tablegen for adjoint generator #1325
base: main
Are you sure you want to change the base?
Conversation
As discusssed here is a slight adjustment of your PR which you could use to add your mpi code as part of InstructionDerivatives.td instead of copying the style of BlasDerivatives.td It might have some mistakes so make sure to check, but if you get something similar to this style to compile,
|
Based on your second question, this could be a shorter design, which might need a few more adjustments,
|
I would heavily recommend against making a new MPI-specific tablegen infrastructure, as opposed to extending and using the existing call infrastructure with whatever new operations you need. |
@wsmoses I agree on not having a third mpi-tg beside of enzyme-tg and blas-tg, @PragmaTwice Using the existing call class should make it easier to get a first compiling version, so maybe focus on this one. Most rules will likely be to complex and miss features so you won't be able to emit all of the required code to handle mpi yet. However, once you get it to compile we can see which features are missing and add those one by one. |
I will try to construct a |
So if you do want to have a type system as extension for the callpatern, you could use this to extend the list. the above example should therefore translate into:
simplification:
Also you can try to generate the following names and helper for each input argument:
and for all the shadow arguments (might already be done by tablegen, just check)
|
Simplifaction1: Mark all MPI_Gather arguments as InactiveArg,
Beside of the 3 funcitons which you already added, this logic would also be able to handle the forward mode of
So I do think it is worth starting with it :) |
WIP, some comment and pr description will be added soon.