-
Notifications
You must be signed in to change notification settings - Fork 908
fortran: Fix many Fortran binding bugs #1538
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
fortran: Fix many Fortran binding bugs #1538
Conversation
Though the `MPI_DUP_FN` subroutine is depricated, it is not yet removed as of MPI-3.1.
Same order for `comm`, `type`, and `win`. No code change.
And insert necessary empty lines and remove unnecessary empty lines. No code change.
This commit adds the following symbols MPI_Alloc_mem_cptr_f MPI_Alloc_mem_cptr_f08 PMPI_Alloc_mem_cptr_f PMPI_Alloc_mem_cptr_f08 These are implemented in the same way as other `_cptr` routines.
#pragma weak MPI_ALLOC_MEM_CPTR = ompi_alloc_mem_f | ||
#pragma weak mpi_alloc_mem_cptr = ompi_alloc_mem_f | ||
#pragma weak mpi_alloc_mem_cptr_ = ompi_alloc_mem_f | ||
#pragma weak mpi_alloc_mem_cptr__ = ompi_alloc_mem_f | ||
|
||
#pragma weak MPI_Alloc_mem_f = ompi_alloc_mem_f | ||
#pragma weak MPI_Alloc_mem_f08 = ompi_alloc_mem_f | ||
|
||
#pragma weak MPI_Alloc_mem_cptr_f = ompi_alloc_mem_f | ||
#pragma weak MPI_Alloc_mem_cptr_f08 = ompi_alloc_mem_f |
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.
Super minor nit: in the above section, it goes:
- pragmas for alloc_mem
- pragmas for alloc_mem_f/_f08
- ...and then all the cptr pragmas
But in this section, it's a different order:
- pragmas for alloc_mem
- pragmas for cptr
- pragmas for alloc_mem_f/f08
- more pragmas for cptr
Can you make the order between these two sections be the same?
(again, I realize this is a super-picky nit -- thank you for your patience!)
@kawashima-fj Thank you for cleaning up all these issues. Can you please PR this stuff to v2.0.0? This PR shows how badly we need to overhaul our Fortran bindings 😦 Specifically: our Fortran bindings are functionally correct, but whenever a dev adds a new MPI interface, it's super easy to leave out one of the X (where X=large) necessary Fortran implementations of that MPI interface. A long time ago, I did the first steps of generating all the Fortran bindings at configure/make time. Slightly less than a long time ago, I gave that code to @ggouaillardet -- @ggouaillardet have you been able to make any progress with it? Is it something you could possibly collaborate with @kawashima-fj to make some progress on, perchance? |
I pushed a commit to fix the order issue. Thanks. @jsquyres @ggouaillardet |
@kawashima-fj I think something was wrong with this PR. Last night, I got a build failure with MTT:
It looks like something is wrong in Can you investigate? |
@kawashima-fj Here's the link to the MTT failure: https://mtt.open-mpi.org/index.php?do_redir=2292 |
I'll take a look. |
@jsquyres I opened a PR to fix this issue. Please review open-mpi/ompi-release#1091. |
@jsquyres Please review.
This PR fixes many Fortran binding issues. Please see each commit message.