Skip to content

Conversation

@ggouaillardet
Copy link
Contributor

in order to solve an egg and the chicken problem, in which mpiext need mpi-f08-types.mod
and/but use-mpi-f08[-desc] needs mpiext, add an extra step

  • build fortran 2008 modules only
  • build fortran 2008 mpi extensions
  • and then build fortran 2008 bindings

Fixes #3605

Signed-off-by: Gilles Gouaillardet gilles@rist.or.jp

@ggouaillardet
Copy link
Contributor Author

@jsquyres could you please review this before it lands into master ?

btw, can you point me to a compiler that will use use-mpi-f08-desc ?
the comments suggest intel compilers can do that, but nor ifort 2017 nor 2018 beta are able to do it.

@ggouaillardet
Copy link
Contributor Author

just to be clear, i blindingly updated use-mpi-f08-desc since i am unable to test it.

@ibm-ompi
Copy link

The IBM CI (PGI Compiler) build failed! Please review the log, linked below.

Gist: https://gist.github.com/b263311298475efed1441e9f6fde6e04

@ibm-ompi
Copy link

The IBM CI (XL Compiler) build failed! Please review the log, linked below.

Gist: https://gist.github.com/511b9c2765392e4c916c9b06a914a328

@ibm-ompi
Copy link

The IBM CI (GNU Compiler) build failed! Please review the log, linked below.

Gist: https://gist.github.com/fbc67eac58ea60a3c8bea7c80a21cf6d

@t-kurita
Copy link
Contributor

t-kurita commented Aug 8, 2017

I confirmed use-mpi-f08-desc can be compiled successfully with gfortran 5.3.0 and the --enable-mpi-f08-subarray-prototype option.
When compiling, I applied the following patch and changed the extension of a file.
(ompi/mpi/fortran/use-mpi-f08-desc/mod/mpi-f08-types.f90 -> ompi/mpi/fortran/use-mpi-f08-desc/mod/mpi-f08-types.F90)

diff --git a/config/ompi_configure_options.m4 b/config/ompi_configure_options.m4
index 3301df0..5bde2b2 100644
--- a/config/ompi_configure_options.m4
+++ b/config/ompi_configure_options.m4
@@ -247,8 +247,7 @@ AS_IF([test $OMPI_TRY_FORTRAN_BINDINGS -lt $OMPI_FORTRAN_USEMPIF08_BINDINGS],
              [OMPI_BUILD_FORTRAN_F08_SUBARRAYS=1
               AC_MSG_RESULT([extra crispy (subarray prototype)])
               AC_MSG_WARN([Sorry, the subarray prototype is no longer available])
-              AC_MSG_WARN([Contact your favorite OMPI developer and ask for it to be re-enabled])
-              AC_MSG_ERROR([Cannot continue])],
+              AC_MSG_WARN([Contact your favorite OMPI developer and ask for it to be re-enabled])]
              [AC_MSG_RESULT([regular (no subarray support)])])
       ])
 AC_DEFINE_UNQUOTED([OMPI_BUILD_FORTRAN_F08_SUBARRAYS],
diff --git a/config/ompi_ext.m4 b/config/ompi_ext.m4
index ae0e91b..1054a74 100644
--- a/config/ompi_ext.m4
+++ b/config/ompi_ext.m4
@@ -216,8 +216,7 @@ EOF
 
     # Only build this mpi_f08_ext module if we're building the "use
     # mpi_f08" module *and* it's the non-descriptor one.
-    AS_IF([test $OMPI_BUILD_FORTRAN_BINDINGS -ge $OMPI_FORTRAN_USEMPIF08_BINDINGS && \
-           test $OMPI_BUILD_FORTRAN_F08_SUBARRAYS -eq 0],
+    AS_IF([test $OMPI_BUILD_FORTRAN_BINDINGS -ge $OMPI_FORTRAN_USEMPIF08_BINDINGS],
           [OMPI_BUILD_FORTRAN_USEMPIF08_EXT=1],
           [OMPI_BUILD_FORTRAN_USEMPIF08_EXT=0])
     AM_CONDITIONAL(OMPI_BUILD_FORTRAN_USEMPIF08_EXT,
diff --git a/ompi/mpi/fortran/use-mpi-f08-desc/Makefile.am b/ompi/mpi/fortran/use-mpi-f08-desc/Makefile.am
index f7d1275..559012f 100644
--- a/ompi/mpi/fortran/use-mpi-f08-desc/Makefile.am
+++ b/ompi/mpi/fortran/use-mpi-f08-desc/Makefile.am
@@ -59,7 +59,7 @@ MOSTLYCLEANFILES = *.mod
 # manually here.  Bummer!
 #
 
-OMPI_Fortran_binding.lo: OMPI_Fortran_binding.f90 mpi-f08-types.lo
+OMPI_Fortran_binding.lo: OMPI_Fortran_binding.f90 mod/mpi-f08-types.lo
 
 
 #
diff --git a/ompi/mpi/fortran/use-mpi-f08-desc/mod/Makefile.am b/ompi/mpi/fortran/use-mpi-f08-desc/mod/Makefile.am
index ec5a76f..2e47016 100644
--- a/ompi/mpi/fortran/use-mpi-f08-desc/mod/Makefile.am
+++ b/ompi/mpi/fortran/use-mpi-f08-desc/mod/Makefile.am
@@ -27,7 +27,7 @@ noinst_LTLIBRARIES = $(module_sentinel_file)
 # f08 support modules
 
 libforce_usempif08_internal_modules_to_be_built_la_SOURCES = \
-        mpi-f08-types.f90 \
+        mpi-f08-types.F90 \
         mpi-f08-interfaces.F90
 
 #
@@ -41,7 +41,7 @@ MOSTLYCLEANFILES = *.mod
 # manually here.  Bummer!
 #
 
-mpi-f08-types.lo: mpi-f08-types.f90
+mpi-f08-types.lo: mpi-f08-types.F90
 mpi-f08-interfaces.lo: mpi-f08-interfaces.F90 mpi-f08-types.lo

Will it be merged to the master?

@ggouaillardet
Copy link
Contributor Author

first thing first, @jsquyres can you please review this PR ?

@t-kurita do you really need the use-mpi-f08-desc interface ?
(e.g. is the use-mpi-f08 not good enough for you ?)

Jeff, do you have any recollection on why we

Only build this mpi_f08_ext module if we're building the "use
mpi_f08" module and it's the non-descriptor one.

regarding the inline patch, at first glance, i'd rather add yet an other configure option in order to enable the subarray prototype.
out of curiosity, why did you change the extension of mpi-f08-types.f90 to .F90 ?

@kawashima-fj
Copy link
Member

@t-kurita is my colleague. I'll explain.

In Open MPI master (and release branches), use-mpi-f08-desc is disabled currently because the implementation is incomplete. In @t-kurita's patch, the purpose of changes in ompi_configure_options.m4 and ompi_ext.m4 is temporarily enabling use-mpi-f08-desc to confirm @ggouaillardet's patch. For this PR, only changes in Makefile.am and the filename change are needed.

mpi-f08-types.f90 has a #include preprocessor directive (instead of include Fortran statement). So it need to be handled by the preprocessor. Otherwise, compilation will fail.

@jsquyres
Copy link
Member

jsquyres commented Aug 8, 2017

Let me ask a different question: should we delete the descriptor-based mpi_f08 module?

It has sat dormant for years, and I certainly have no cycles to develop it further. Does anyone else? If not, we should probably delete it (it'll always be in git history if someone wants to dig it out / continue the work).

in order to solve an egg and the chicken problem,  in which mpiext need mpi-f08-types.mod
and/but use-mpi-f08[-desc] needs mpiext, add an extra step
- build fortran 2008 modules only
- build fortran 2008 mpi extensions
- and then build fortran 2008 bindings

Fixes open-mpi#3605

Signed-off-by: Gilles Gouaillardet <gilles@rist.or.jp>
Signed-off-by: Gilles Gouaillardet <gilles@rist.or.jp>
so Open MPI can be configure'd with --enable-mpi-f08-subarray-prototype

Signed-off-by: Gilles Gouaillardet <gilles@rist.or.jp>
@ggouaillardet ggouaillardet force-pushed the topic/f08_mpiext branch 2 times, most recently from 344f000 to 2c71c27 Compare August 9, 2017 04:44
@ggouaillardet
Copy link
Contributor Author

@jsquyres i pushed two commits into master, so this PR is now really only for the last one
(i do not know why github still thinks there are 3 commits in this PR)

@t-kurita @kawashima-fj just to be clear, is the inline patch something you need ?
or is this only here as a proof of concept to help me fix this PR ?
i pushed the fixes, but did not re-enable the --enable-mpi-f08-subarray-prototype option.

i have no strong opinion whether we should (or not) remove use-mpi-f08-desc support

@kawashima-fj
Copy link
Member

@ggouaillardet Thanks for your work. We Fujitsu don't need the inline patch and we also have no strong option about use-mpi-f08-desc support. The inline patch is created for your comment:

just to be clear, i blindingly updated use-mpi-f08-desc since i am unable to test it.

@jsquyres
Copy link
Member

jsquyres commented Aug 9, 2017

If no one has any strong feelings, then we should remove the use-mpi-f08-desc directory/support.

@jsquyres
Copy link
Member

Per #4068, let's merge this PR and then #4068 will entirely remove the proof of concept descriptor-based mpi_f08 module.

@jsquyres jsquyres merged commit 1a70e5b into open-mpi:master Aug 10, 2017
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.

5 participants