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

Bugfix/windows fortran builds #506

Merged
merged 109 commits into from
Jun 19, 2024
Merged

Conversation

balos1
Copy link
Member

@balos1 balos1 commented Jun 14, 2024

No description provided.

@balos1
Copy link
Member Author

balos1 commented Jun 18, 2024

@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
Copy link
Member Author

balos1 commented Jun 18, 2024

@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
Copy link
Contributor

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

doc/shared/RecentChanges.rst Outdated Show resolved Hide resolved
cmake/SundialsSetupFortran.cmake Show resolved Hide resolved
cmake/macros/SundialsAddLibrary.cmake Show resolved Hide resolved
examples/kinsol/F2003_serial/kinLaplace_bnd_f2003.f90 Outdated Show resolved Hide resolved
src/sunadaptcontroller/imexgus/CMakeLists.txt Outdated Show resolved Hide resolved
src/sunadaptcontroller/soderlind/CMakeLists.txt Outdated Show resolved Hide resolved
@balos1
Copy link
Member Author

balos1 commented Jun 19, 2024

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

Copy link
Member

@gardner48 gardner48 left a comment

Choose a reason for hiding this comment

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

Other than keeping the controllers as only object libraries the changes look good.

@balos1 balos1 requested a review from gardner48 June 19, 2024 20:58
@gardner48 gardner48 merged commit f458e59 into develop Jun 19, 2024
24 checks passed
@gardner48 gardner48 deleted the bugfix/windows-fortran-builds branch June 19, 2024 23:04
@balos1 balos1 added this to the SUNDIALS Next milestone Jun 20, 2024
gardner48 pushed a commit that referenced this pull request Jun 20, 2024
Fixes various issues related to building Fortran interfaces and examples on Windows
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.

3 participants