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 integer kind=8 problem #13159

Merged
merged 1 commit into from
Mar 24, 2025

Conversation

hppritcha
Copy link
Member

user reported problem on the ompi-users mail list.

https://www.mail-archive.com/[email protected]//msg35382.html

user reported problem on the ompi-users mail list.

https://www.mail-archive.com/[email protected]//msg35382.html

Signed-off-by: Howard Pritchard <[email protected]>
Copy link
Contributor

@devreal devreal left a comment

Choose a reason for hiding this comment

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

LGTM (although my understand of the problem is limited)

@hppritcha hppritcha merged commit 593d8ac into open-mpi:main Mar 24, 2025
15 checks passed
@jsquyres
Copy link
Member

jsquyres commented Mar 25, 2025

@hppritcha Giles noted in 32db65f#commitcomment-154262549 that this might not be right -- the implicit string length argument is passed as an int, not an MPI_Fint.

Can you dig a little deeper and see if this is actually correct?

(oops -- should have posted this on #13160)

@ggouaillardet
Copy link
Contributor

FWIW, I would reverse that commit and fix it like that instead

diff --git a/ompi/mpi/fortran/use-mpi-f08/session_get_nth_pset_f08.F90 b/ompi/mpi/fortran/use-mpi-f08/session_get_nth_pset_f08.F90
index c34ed8d008..65dffc8852 100644
--- a/ompi/mpi/fortran/use-mpi-f08/session_get_nth_pset_f08.F90
+++ b/ompi/mpi/fortran/use-mpi-f08/session_get_nth_pset_f08.F90
@@ -14,6 +14,7 @@
 subroutine MPI_Session_get_nth_pset_f08(session, info, n, pset_len, pset_name, ierror)
    use :: mpi_f08_types, only : MPI_Session, MPI_Info, MPI_INFO_NULL
    use :: ompi_mpifh_bindings, only : ompi_session_get_nth_pset_f
+   use, intrinsic :: ISO_C_BINDING, only : C_INT
    implicit none
    TYPE(MPI_Session), INTENT(IN) :: session
    TYPE(MPI_Info), INTENT(IN) :: info
@@ -21,10 +22,12 @@ subroutine MPI_Session_get_nth_pset_f08(session, info, n, pset_len, pset_name, i
    INTEGER, OPTIONAL, INTENT(INOUT) :: pset_len
    CHARACTER(LEN=*), INTENT(OUT) :: pset_name
    INTEGER, OPTIONAL, INTENT(OUT) :: ierror
+   INTEGER(KIND=C_INT) :: pset_name_len
    integer :: c_ierror
 
+   pset_name_len = len(pset_name)
    call ompi_session_get_nth_pset_f(session%MPI_VAL, MPI_INFO_NULL%MPI_VAL, n,     &
-                                    pset_len, pset_name, c_ierror, len(pset_name))
+                                    pset_len, pset_name, c_ierror, pset_name_len)
    if (present(ierror)) ierror = c_ierror
 
 end subroutine MPI_Session_get_nth_pset_f08

@hppritcha
Copy link
Member Author

actually there a number of *.F90 files that do what the original commit did. check how ompi_comm_set_name_f and similar are used in the use-mpi-f08 folder. Not saying they're correct but that explains why the user is able to build with default KIND=8 for older releases but compilation fails. Anyway I'll use a different approach which is nicer (no new variable introduction).
I suppose there are a number of files which really need to be updated. I'll include them in a new PR.

@hppritcha
Copy link
Member Author

according to these discussions these files are incorrect along with the prototypes in bindings/mpi-f-interfaces-bind.h

add_error_string_f08.F90:   call ompi_add_error_string_f(errorcode, string, c_ierror, len(string))
close_port_f08.F90:   call ompi_close_port_f(port_name,c_ierror,len(port_name))
comm_accept_f08.F90:                           c_ierror,len(port_name))
comm_connect_f08.F90:                            c_ierror,len(port_name))
comm_create_from_group_f08.F90:                                      newcomm%MPI_VAL, c_ierror, len(stringtag))
comm_get_name_f08.F90:                             len(comm_name))
comm_set_name_f08.F90:   call ompi_comm_set_name_f(comm%MPI_VAL,comm_name,c_ierror,len(comm_name))
comm_spawn_f08.F90:                          len(command), len(argv))
comm_spawn_multiple_f08.F90:                                   len(array_of_commands), len(array_of_argv))
error_string_f08.F90:   call ompi_error_string_f(errorcode,string,resultlen,c_ierror,len(string))
file_delete_f08.F90:   call ompi_file_delete_f(filename,info%MPI_VAL,c_ierror,len(filename))
file_get_view_f08.F90:                             datarep,c_ierror,len(datarep))
file_open_f08.F90:                         c_ierror, len(filename))
file_set_view_f08.F90:                             datarep,info%MPI_VAL,c_ierror,len(datarep))
get_library_version_f08.F90:   call ompi_get_library_version_f(version,resultlen,c_ierror,len(version))
get_processor_name_f08.F90:   call ompi_get_processor_name_f(name,resultlen,c_ierror,len(name))
group_from_session_pset_f08.F90:   call ompi_group_from_session_pset_f(session%MPI_VAL, pset_name, newgroup%MPI_VAL, c_ierror, len(pset_name))
info_delete_f08.F90:   call ompi_info_delete_f(info%MPI_VAL,key,c_ierror,len(key))
info_get_nthkey_f08.F90:   call ompi_info_get_nthkey_f(info%MPI_VAL,n,key,c_ierror,len(key))
info_get_valuelen_f08.F90:   call PMPI_Info_get_valuelen(info%MPI_VAL,key,valuelen,flag,c_ierror)
info_set_f08.F90:   call ompi_info_set_f(info%MPI_VAL,key,value,c_ierror,len(key),len(value))
intercomm_create_from_groups_f08.F90:                                            newintercomm%MPI_VAL, c_ierror, len(stringtag))
lookup_name_f08.F90:                           len(service_name),len(port_name))
open_port_f08.F90:   call ompi_open_port_f(info%MPI_VAL,port_name,c_ierror,len(port_name))
pack_external_f08.F90:                             outsize,position,c_ierror,len(datarep))
pack_external_size_f08.F90:   call ompi_pack_external_size_f(datarep,incount,datatype%MPI_VAL,size,c_ierror,len(datarep))
publish_name_f08.F90:                            len(service_name), len(port_name))
register_datarep_f08.F90:                                fdtype_fn,extra_state,c_ierror,len(datarep))
session_get_nth_pset_f08.F90:                                    len(pset_name,KIND=C_INT))
session_get_pset_info_f08.F90:   call ompi_session_get_pset_info_f(session%MPI_VAL, pset_name, info%MPI_VAL, c_ierror, len(pset_name))
type_get_name_f08.F90:   call ompi_type_get_name_f(datatype%MPI_VAL,type_name,resultlen,c_ierror,len(type_name))
type_set_name_f08.F90:   call ompi_type_set_name_f(datatype%MPI_VAL,type_name,c_ierror,len(type_name))
unpack_external_f08.F90:                               outcount,datatype%MPI_VAL,c_ierror,len(datarep))
unpublish_name_f08.F90:                              len(service_name), len(port_name))
win_get_name_f08.F90:   call ompi_win_get_name_f(win%MPI_VAL,win_name,resultlen,c_ierror,len(win_name))
win_set_name_f08.F90:   call ompi_win_set_name_f(win%MPI_VAL,win_name,c_ierror,len(win_name))

@jsquyres

@jsquyres
Copy link
Member

Yikes. Thanks for tracking this down, @hppritcha!

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.

4 participants