Skip to content

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

Merged
merged 9 commits into from
Apr 14, 2016

Conversation

kawashima-fj
Copy link
Member

@jsquyres Please review.

This PR fixes many Fortran binding issues. Please see each commit message.

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.
@kawashima-fj kawashima-fj added this to the v2.0.0 milestone Apr 12, 2016
#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
Copy link
Member

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!)

@jsquyres
Copy link
Member

@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?

@jsquyres jsquyres merged commit 4566286 into open-mpi:master Apr 14, 2016
@kawashima-fj
Copy link
Member Author

I pushed a commit to fix the order issue. Thanks.
I'll make a PR to v2.0.0 soon.

@jsquyres @ggouaillardet
I looked only mpif-h and use-mpi-ignore-tkr this time.
use-mpi-f08 etc. may have similar issues possibly.
I also think auto-generating is desired.
Yes, possibly I can collaborate.

@kawashima-fj kawashima-fj deleted the pr/fortran-binding-fix branch April 15, 2016 06:25
@jsquyres
Copy link
Member

@kawashima-fj I think something was wrong with this PR. Last night, I got a build failure with MTT:

$ ./configure "CFLAGS=-g -pipe" --enable-picky --enable-debug --enable-mpirun-prefix-by-default --disable-dlopen --disable-weak-symbols

...lots of output...

$ make -j 32

... lots of output...

  ^profile/.libs/libmpi_mpifh_pmpi.a(palloc_mem_f.o): In function `ompi_alloc_mem_cptr_f':
/home/mpiteam/scratches/community/2016-04-20cron/Wniq/mpi-install/rAXy/src/openmpi-v1.10.2-154-gfe1dc36/ompi/mpi/fortran/mpif-h/profile/palloc_mem_f.c:115:
multiple definition of `ompi_alloc_mem_cptr_f'
.libs/alloc_mem_f.o:/home/mpiteam/scratches/community/2016-04-20cron/Wniq/mpi-install/rAXy/src/openmpi-v1.10.2-154-gfe1dc36/ompi/mpi/fortran/mpif-h/alloc_mem_f.c:115:
first defined herecollect2: error: ld returned 1 exit status
make[3]: *** [libmpi_mpifh.la] Error 1
make[2]: *** [all-recursive] Error 1
make[1]: *** [all-recursive] Error 1
make: *** [all-recursive] Error 1

It looks like something is wrong in alloc_mem_f.f.

Can you investigate?

@jsquyres
Copy link
Member

@kawashima-fj Here's the link to the MTT failure: https://mtt.open-mpi.org/index.php?do_redir=2292

@kawashima-fj
Copy link
Member Author

I'll take a look.

@kawashima-fj
Copy link
Member Author

@jsquyres I opened a PR to fix this issue. Please review open-mpi/ompi-release#1091.

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

Successfully merging this pull request may close these issues.

2 participants