Skip to content
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 PMPI interface bugs in mpi_f08 module #4667

Merged
merged 5 commits into from
Jan 5, 2018

Conversation

kawashima-fj
Copy link
Member

@kawashima-fj kawashima-fj commented Dec 26, 2017

This PR brings bug fixes which were cared in MPI_ interface but were not cared in PMPI_ interfaces of the Fortran mpi_f08 module.

As the result, ompi/mpi/fortran/use-mpi-f08/mod/pmpi-f08-interfaces.F90 with sed -e s/PMPI_/MPI_/g -e s/pmpi_/mpi_/g becomes same as ompi/mpi/fortran/use-mpi-f08/mod/mpi-f08-interfaces.F90 except copyright statements.

@jsquyres, you changed PMPI_Aint_add_f08 and PMPI_Aint_diff_f08 to subroutines from functions in your 258d1aa. I supposed it was an unintentional change and I reverted them to functions in my d4fc404. Could you confirm it just in case?

Better fix is generating pmpi-* files from mpi-* files. I remain it as a future work.

I'll create PRs for release branches to handle this PR and #4659.

NEWS: Correct PMPI_Aint_{add|diff} to be functions (not subroutines) in the Fortran mpi_f08 module.

It was removed from only `mpi` as a bug fix in db41d74.

Signed-off-by: KAWASHIMA Takahiro <t-kawashima@jp.fujitsu.com>
They were incorrectly changed to subroutines in only `pmpi`
in 258d1aa.

Strictly speaking, this change involves binary incompatibility.
But nobody used these subroutines and nobody will be affected because
these subroutines were useless (didn't return a calculated value).

Signed-off-by: KAWASHIMA Takahiro <t-kawashima@jp.fujitsu.com>
It was changed to use `C_PTR` in only `mpi` in fc69c0b.

Signed-off-by: KAWASHIMA Takahiro <t-kawashima@jp.fujitsu.com>
It is incorrectly typed as `MPI_Comm` in only `pmpi` in 24f7bd3.

Signed-off-by: KAWASHIMA Takahiro <t-kawashima@jp.fujitsu.com>
This change makes comparison of `mpi-f08-interfaces.F90` and
`pmpi-f08-interfaces.F90` easier.

Signed-off-by: KAWASHIMA Takahiro <t-kawashima@jp.fujitsu.com>
Copy link
Member

@bwbarrett bwbarrett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are the ABI impacts of this change (if any, I suck at Fortran)?

@jsquyres
Copy link
Member

jsquyres commented Jan 3, 2018

@bwbarrett It breaks ABI for PMPI_Aint_{add|diff} (by changing them from subroutines to functions). But the OMPI was wrong (i.e., the MPI spec says they must be functions). Therefore, no user applications could have been using these functions.

This is worth a note in NEWS, though.

@kawashima-fj
Copy link
Member Author

kawashima-fj commented Jan 5, 2018

@bwbarrett @jsquyres My commit message d4fc404 explains it.

Strictly speaking, this change involves binary incompatibility.
But nobody used these subroutines and nobody will be affected because
these subroutines were useless (didn't return a calculated value).

Is it OK to merge this change to release branches (v2.0, v2.1, v3.0, v3.1)?

The NEWS item will be something like this:

  • Correct PMPI_Aint_{add|diff} to be functions (not subroutines) in the Fortran mpi_f08 module.

@kawashima-fj kawashima-fj merged commit 710080b into open-mpi:master Jan 5, 2018
@bwbarrett
Copy link
Member

@kawashima-fj, yes, I think we should merge to the impacted release branches (although v2.0.x is dead, so don't worry about that branch).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants