LLNL / sundials

Official development repository for SUNDIALS - a SUite of Nonlinear and DIfferential/ALgebraic equation Solvers. Pull requests are welcome for bug fixes and minor changes.
https://computing.llnl.gov/projects/sundials
BSD 3-Clause "New" or "Revised" License
523 stars 131 forks source link

Bugfix/windows fortran builds #506

Closed balos1 closed 5 months ago

mmuetzel commented 5 months ago

To automatically get exports for the symbols in Fortran libraries on Windows, the following change might work:

diff --git a/cmake/macros/SundialsAddLibrary.cmake b/cmake/macros/SundialsAddLibrary.cmake
index 84505e4f1..e80110ea0 100644
--- a/cmake/macros/SundialsAddLibrary.cmake
+++ b/cmake/macros/SundialsAddLibrary.cmake
@@ -464,6 +464,7 @@ macro(sundials_add_f2003_library target)
     VERSION ${sundials_add_f2003_library_VERSION}
     SOVERSION ${sundials_add_f2003_library_SOVERSION}
     ${sundials_add_f2003_library_UNPARSED_ARGUMENTS}
+    WINDOWS_EXPORT_ALL_SYMBOLS ON
   )

 endmacro()
balos1 commented 5 months ago

@mmuetzel Any thoughts on why the msys toolchain is failing to link with shared libraries still? I do not have a system with msys setup to be able to inspect it easily and it seems like you are more familiar with the MSYS/Windows toolchain than myself.

balos1 commented 5 months ago

@gardner48 this is ready for review now (the shared library MinGW build will fail, but if we dont find a fix shortly I will turn it off for now).

mmuetzel commented 5 months ago

@mmuetzel Any thoughts on why the msys toolchain is failing to link with shared libraries still? I do not have a system with msys setup to be able to inspect it easily and it seems like you are more familiar with the MSYS/Windows toolchain than myself.

It looks like WINDOWS_EXPORT_ALL_SYMBOLS doesn't work with the two-step creation of shared libraries in SUNDIALS. (I wasn't aware of that.) IIUC, an object library is created from the source files. And in a second step a shared library is created from that object library. It looks like WINDOWS_EXPORT_ALL_SYMBOLS only exports symbols from the object files corresponding to a shared library (not from an object library).

Alternatively, symbols could be exported from a Fortran library using ATTRIBUTES. I tried to add dllexport attributes manually to some of the functions and subroutines in libsundials_fcore_mod like this:

diff --git "a/src/sundials/fmod_int64/fsundials_core_mod.f90" "b/src/sundials/fmod_int64/fsundials_core_mod.f90"
index 7096d0c6c..9361fa509 100644
--- "a/src/sundials/fmod_int64/fsundials_core_mod.f90"
+++ "b/src/sundials/fmod_int64/fsundials_core_mod.f90"
@@ -2311,6 +2311,8 @@ type(C_PTR), target, intent(inout) :: ctx
 integer(C_INT) :: fresult 
 type(C_PTR) :: farg1 

+!GCC$ ATTRIBUTES DLLEXPORT :: FSUNContext_Free
+
 farg1 = c_loc(ctx)
 fresult = swigc_FSUNContext_Free(farg1)
 swig_result = fresult
@@ -2838,6 +2840,8 @@ type(N_Vector), target, intent(inout) :: z
 real(C_DOUBLE) :: farg1 
 type(C_PTR) :: farg2 

+!GCC$ ATTRIBUTES DLLEXPORT :: FN_VConst
+
 farg1 = c
 farg2 = c_loc(z)
 call swigc_FN_VConst(farg1, farg2)
@@ -4656,6 +4660,8 @@ type(SUNAdaptController), target, intent(inout) :: c
 integer(C_INT) :: fresult 
 type(C_PTR) :: farg1 

+!GCC$ ATTRIBUTES DLLEXPORT :: FSUNAdaptController_Destroy
+
 farg1 = c_loc(c)
 fresult = swigc_FSUNAdaptController_Destroy(farg1)
 swig_result = fresult

With these added, I can see that they are exported from the .dll with that change.

But iiuc, these attributes are compiler-specific. (I was using gfortran for the test.) Intel compilers would require, e.g., !DEC$ ATTRIBUTES DLLEXPORT :: FSUNAdaptController_Destroy (DEC instead of GCC).

What is the motivation for the two step process to create shared libraries?

balos1 commented 5 months ago

@mmuetzel

It looks like WINDOWS_EXPORT_ALL_SYMBOLS doesn't work with the two-step creation of shared libraries in SUNDIALS. (I wasn't aware of that.)

It seemed to work with the Intel oneAPI compilers, but not gfortran under MSYS.

@gardner48 I have opened #507 to document this problem, and we can address it later on. For now I have disabled the Fortran interfaces in the CI for the MSYS+gfortran+shared toolchain. I think we should go ahead and merge this PR since it resolves several other problems including #67 and the Intel compiler toolchain on Windows.