-
Notifications
You must be signed in to change notification settings - Fork 145
Check for both MPI and PMPI versions #2533
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
base: main
Are you sure you want to change the base?
Conversation
| llvm::StringRef callee) { | ||
| // The function could exist in two forms in the module - with PMPI_ or MPI_ | ||
| // prefix. Check for both. | ||
| assert(startsWith(callee, "MPI")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no this is wrong, we should create the func decl if not exists
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This only checks that the user side is passing in MPI_Gather. (Used to be in getRenamedPerCallingConv)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry comment applied to three lines later
|
this is the wrong fix overall getRenamedPerCallingConv("PMPI_...", "MPI_x") should give PMPI_x, which ought resolve? |
No, that's the crux of the issue. A module may contain a mix of PMPI and MPI names, likely due to Enzyme.jl trying to backsolve the name and the symbol either being PMPI or MPI. The full module is here. https://gist.github.com/vchuravy/6f5edc18764db407a019294eba7f39e5 And we are running on |
|
I mean the bigger issue is that we should never have a mix of mpi/pmpi? since I assume julia never gives us a mix. So we should never generate a mix? |
|
the issue imo is that this Enzyme/enzyme/Enzyme/Utils.cpp Line 1803 in 6b1848d
|
|
we have the args, so therefore we ought be able to form the functiontype |
Enzyme.jl is to blame, it tries to invert a pointer from Julia and what name it ends up is a 50/50.
See the module, Julia precisely gives us a mix and that is the issue I am trying to fix. |
|
ah bleh |
@wsmoses on 1.10 I am seeing:
So when
PMPI_Waitgoes looking forMPI_Isendit fails to look at the right one.If this looks kosher to you, I can also go and fix all the other users for getRenamedPerCallingConv