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.

Sign up for GitHub

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

Jump to bottom

v6.0.x: A bunch of mpi_f08 updates #9837

Draft
wants to merge 3 commits into
base: v5.0.x
Choose a base branch
from
Draft

v6.0.x: A bunch of mpi_f08 updates #9837

wants to merge 3 commits into from

Conversation

jsquyres
Copy link
Member

@jsquyres jsquyres commented Jan 5, 2022

This is the v5.0x version of master PR #8173.

Fix a bunch of mpi_f08 bindings issues relating to [OMPI_]ASYNCHRONOUS and INTENT. See individual commit messages for details.

@ggouaillardet @jsquyres
fortran: ensure not to use [AM_]CPPFLAGS
b882b46 
Add missing bits from open-mpi/ompi@ab398f4

Signed-off-by: Gilles Gouaillardet <gilles@rist.or.jp>
(cherry picked from commit ca03269)
@ggouaillardet @jsquyres
use-mpi-f08: add missing subroutines in the mpi_f08 module
8b741b5 
 - MPI_Status_f082f
 - MPI_Status_f2f08

Signed-off-by: Gilles Gouaillardet <gilles@rist.or.jp>
(cherry picked from commit 83eb116)
@ggouaillardet @jsquyres
use-mpi-f08: fix ASYNCHRONOUS and INTENT
baa05a2 
Add a bunch of ASYNCHRONOUSs and INTENTs that were missing.

All instances of ASYNCHRONOUS were added as OMPI_ASYNCHRONOUS (and a
bunch of existing instances of ASYNCHRONOUS were updated to
OMPI_ASYNCHRONOUS) so that we can properly handle compilers that do
not support ASYNCHRONOUS.

Subroutines that had [OMPI_]ASYNCHRONOUS or INTENTs added were:

- MPI_Aint_add
- MPI_Aint_diff
- MPI_Accumulate
- MPI_Bsend_init
- MPI_Comm_idup
- MPI_Compare_and_swap
- MPI_F_sync_reg
- MPI_Fetch_and_op
- MPI_File_iread*
- MPI_File_iwrite*
- MPI_File_read_all_begin
- MPI_File_read_all_end
- MPI_File_read_at_all_begin
- MPI_File_read_at_all_end
- MPI_File_read_ordered_begin
- MPI_File_read_ordered_end
- MPI_File_write_all_begin
- MPI_File_write_all_end
- MPI_File_write_at_all
- MPI_File_write_at_all_begin
- MPI_File_write_at_all_end
- MPI_File_write_ordered_begin
- MPI_File_write_ordered_end
- MPI_Get
- MPI_Get_accumulate
- MPI_Get_address
- MPI_Ialltoall
- MPI_Ibsend
- MPI_Improbe
- MPI_Imrecv
- MPI_Ineighbor_allgather
- MPI_Ineighbor_allgatherv
- MPI_Ineighbor_alltoall
- MPI_Ineighbor_alltoallv
- MPI_Ineighbor_alltoallw
- MPI_Iprobe
- MPI_Irecv
- MPI_Irsend
- MPI_Isend
- MPI_Issend
- MPI_Mprobe
- MPI_Probe
- MPI_Put
- MPI_Raccumulate
- MPI_Recv_init
- MPI_Rget
- MPI_Rget_accumulate
- MPI_Rput
- MPI_Rsend_init
- MPI_Send_init
- MPI_Ssend_init
- MPI_Win_attach
- MPI_Win_create
- MPI_Win_detach
- MPI_Win_get_info

Subroutines that just had ASYNCHRONOUS updated to OMPI_ASYNCHRONOUS
are not included in the list above.

Signed-off-by: Gilles Gouaillardet <gilles@rist.or.jp>
Signed-off-by: Jeff Squyres <jsquyres@cisco.com>
(cherry picked from commit bd2b80b)
@jsquyres jsquyres requested a review from ggouaillardet January 5, 2022 12:48
@jsquyres jsquyres changed the title A bunch of mpi_f08 updates v5.0.x: A bunch of mpi_f08 updates Jan 5, 2022
@jsquyres jsquyres marked this pull request as draft January 5, 2022 23:11
@jsquyres
Copy link
Member Author

jsquyres commented Jan 5, 2022

It looks like the corresponding master PR (#8173) broke something in the mpi_f08 module. Hold off on merging this to v5.0.x until we can fix it.

@hppritcha
Copy link
Member

@jsquyres I think we need to have an OMPI_FORTRAN_IGNORE_TKR_TYPE specialized for f08 that generates an assumed rank argument rather than assumed size. The standard specifies assumed rank - i.e. foo(..)

@jsquyres
Copy link
Member Author

@jsquyres I think we need to have an OMPI_FORTRAN_IGNORE_TKR_TYPE specialized for f08 that generates an assumed rank argument rather than assumed size. The standard specifies assumed rank - i.e. foo(..)

Translation: it appears that ASYNCHRONOUS is not valid for TYPE(*), DIMENSION(*). But it is valid for TYPE(*), DIMENSION(..).

@awlauria awlauria mentioned this pull request Jan 25, 2022
Merged
@gpaulsen
Copy link
Member

@jsquyres What's the status of this for v5.0.x?

@jsquyres
Copy link
Member Author

jsquyres commented Feb 1, 2022

Still waiting on the corresponding master PR #8173 and getting a fix for that.

@gpaulsen
Copy link
Member

@jsquyres PR #8173 looks like it's in. Does that unblock THIS PR? Should we remove the WIP-DNM?

@gpaulsen
Copy link
Member

@hppritcha @jsquyres Did this ever get fixed on master? It looks like XL compiler still doesn't like something on master:

** test_mpi_bsend   === End of Compilation 1 ===
  CCLD     bsend_free
  CCLD     buffer
COMMAND LINE  1520-022 (W) The FREE option has been replaced by the option FREE(F90).
COMMAND LINE  1520-022 (W) The FREE option has been replaced by the option FREE(F90).
** testsome   === End of Compilation 1 ===
** test_mpi_mprobe   === End of Compilation 1 ===
** waitsome   === End of Compilation 1 ===
** waitsome   === End of Compilation 1 ===
** testall   === End of Compilation 1 ==="allocmem_usempif08.f90", line 81.21: 1516-273 (S) Dummy
argument buf of procedure mpi_isend_f08 was specified with the ASYNCHRONOUS attribute. buf is not an
assumed-shape array or a pointer array.  The corresponding actual argument must not be a pointer
array.
"allocmem_usempif08.f90", line 82.21: 1516-273 (S) Dummy argument buf of procedure mpi_isend_f08 was
specified with the ASYNCHRONOUS attribute. buf is not an assumed-shape array or a pointer array. 
The corresponding actual argument must not be a pointer array.
"allocmem_usempif08.f90", line 83.21: 1516-273 (S) Dummy argument buf of procedure mpi_irecv_f08 was
specified with the ASYNCHRONOUS attribute. buf is not an assumed-shape array or a pointer array. 
The corresponding actual argument must not be a pointer array.
"allocmem_usempif08.f90", line 84.21: 1516-273 (S) Dummy argument buf of procedure mpi_irecv_f08 was
specified with the ASYNCHRONOUS attribute. buf is not an assumed-shape array or a pointer array. 
The corresponding actual argument must not be a pointer array.
** allocmem_test   === End of Compilation 1 ===
1501-510  Compilation successful for file improbe_usempi.f90.
1501-511  Compilation failed for file allocmem_usempif08.f90.
make[1]: *** [Makefile:1462: allocmem_usempif08.o] Error 1

@hppritcha
Copy link
Member

@gpaulsen no this hasn't been fixed yet on master. It's on my todo list. I'll move it up in priority and get to it this week.

@awlauria
Copy link
Contributor

Is there any update on this @hppritcha ?

@hppritcha
Copy link
Member

this isn't going to make it into the 5.0.x release stream. removing that label.

@hppritcha
Copy link
Member

oh sorry mixed this up with my PR #10302

@hppritcha
Copy link
Member

i don't know what the status of this PR is.

@gpaulsen
Copy link
Member

@hppritcha , It's been a long time... I can't remember the details of why I moved the label to v6.0.x. If that's really the case, and the content is on main, can we just close this PR?

@hppritcha
Copy link
Member

good question. Unless @jsquyres objects we should move it to v6.0.x. We need something like #10302 to truly be in compliance with the standard wrt mpi_f08 bindings. This is turning in to a long range project.

@jsquyres
Copy link
Member Author

Agreed. I've had zero cycles to work on this, and that probably won't change any time soon. Push to 6.0.x.

@jsquyres jsquyres changed the title v5.0.x: A bunch of mpi_f08 updates v6.0.x: A bunch of mpi_f08 updates Jan 12, 2023
@gpaulsen gpaulsen changed the title v6.0.x: A bunch of mpi_f08 updates v5.0.x: A bunch of mpi_f08 updates Jun 13, 2023
@github-actions github-actions bot added this to the v5.0.0 milestone Jun 13, 2023
@gpaulsen gpaulsen changed the title v5.0.x: A bunch of mpi_f08 updates v6.0.x: A bunch of mpi_f08 updates Jun 13, 2023
@jsquyres jsquyres modified the milestones: v5.0.0, v5.0.1 Oct 30, 2023
@janjust janjust modified the milestones: v5.0.1, v5.0.2 Jan 8, 2024
@jsquyres jsquyres modified the milestones: v5.0.2, v5.0.3 Feb 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ggouaillardet ggouaillardet Awaiting requested review from ggouaillardet

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
😳 Backward compat break fortran bindings NEWS Target: v6.0.x ⚠️ WIP-DNM!
Projects
None yet
Milestone
v5.0.3
Development

Successfully merging this pull request may close these issues.
6 participants
@jsquyres @hppritcha @gpaulsen @awlauria @janjust @ggouaillardet