Skip to content

Commit 8ec5655

Browse files
mabrahamacmnpv
authored andcommittedFeb 20, 2025
Use CMake target-style MPI dependencies
The former global flags are a legacy from CMake 2.6 when we did not use CMake targets. One should generally use a wrapper compiler and the CMake target, and as such have no need of appending to global flags. Likewise when using a CMake target, the MPI target should be added on an as-needs basis. There's no need to add MPI C flags
1 parent 60d5e5b commit 8ec5655

File tree

3 files changed

+11
-31
lines changed

3 files changed

+11
-31
lines changed
 

‎CMakeLists.txt

+6-16
Original file line numberDiff line numberDiff line change
@@ -952,31 +952,21 @@ endif()
952952

953953
# # # # # # # # # # NO MORE TESTS AFTER THIS LINE! # # # # # # # # # # #
954954
# these are set after everything else
955-
if (GMX_LIB_MPI)
956-
# TODO: Restrict the scope of MPI dependence.
957-
# Targets that actually need MPI headers and build tool flags should
958-
# manage their own `target_link_libraries` locally. Such a change is beyond
959-
# the scope of the bug fix for #4678.
960-
set(_gmx_mpi_cxx_link_flags "${MPI_CXX_LINK_FLAGS}")
961-
set(_gmx_mpi_c_compile_flags "${MPI_C_COMPILE_OPTIONS};${MPI_C_COMPILE_DEFINITIONS}")
962-
set(_gmx_mpi_cxx_compile_flags "${MPI_CXX_COMPILE_OPTIONS};${MPI_CXX_COMPILE_DEFINITIONS}")
963-
endif ()
964955
if (NOT GMX_SKIP_DEFAULT_CFLAGS)
965-
#TODO(#3672): Use target_link_libraries(... MPI::MPI_CXX) instead of ${MPI_CXX_LINK_FLAGS}
966-
set(CMAKE_EXE_LINKER_FLAGS "${FFT_LINKER_FLAGS} ${_gmx_mpi_cxx_link_flags} ${CMAKE_EXE_LINKER_FLAGS}")
967-
set(CMAKE_SHARED_LINKER_FLAGS "${FFT_LINKER_FLAGS} ${_gmx_mpi_cxx_link_flags} ${CMAKE_SHARED_LINKER_FLAGS}")
956+
set(CMAKE_EXE_LINKER_FLAGS "${FFT_LINKER_FLAGS} ${CMAKE_EXE_LINKER_FLAGS}")
957+
set(CMAKE_SHARED_LINKER_FLAGS "${FFT_LINKER_FLAGS} ${CMAKE_SHARED_LINKER_FLAGS}")
968958
else()
969959
message("Recommended flags which are not added because GMX_SKIP_DEFAULT_CFLAGS=yes:")
970-
message("CMAKE_C_FLAGS: ${SIMD_C_FLAGS};${_gmx_mpi_c_compile_flags};${EXTRA_C_FLAGS};${GMXC_CFLAGS}")
960+
message("CMAKE_C_FLAGS: ${SIMD_C_FLAGS};${EXTRA_C_FLAGS};${GMXC_CFLAGS}")
971961
foreach(build_type ${build_types_with_explicit_flags})
972962
message("CMAKE_C_FLAGS_${build_type}: ${GMXC_CFLAGS_${build_type}}")
973963
endforeach()
974-
message("CMAKE_CXX_FLAGS: ${SIMD_CXX_FLAGS};${_gmx_mpi_cxx_compile_flags};${EXTRA_CXX_FLAGS};${GMXC_CXXFLAGS}")
964+
message("CMAKE_CXX_FLAGS: ${SIMD_CXX_FLAGS};${EXTRA_CXX_FLAGS};${GMXC_CXXFLAGS}")
975965
foreach(build_type ${build_types_with_explicit_flags})
976966
message("CMAKE_CXX_FLAGS_${build_type}: ${GMXC_CXXFLAGS_${build_type}}")
977967
endforeach()
978-
message("CMAKE_EXE_LINKER_FLAGS: ${FFT_LINKER_FLAGS} ${_gmx_mpi_cxx_link_flags}")
979-
message("CMAKE_SHARED_LINKER_FLAGS: ${FFT_LINKER_FLAGS} ${_gmx_mpi_cxx_link_flags}")
968+
message("CMAKE_EXE_LINKER_FLAGS: ${FFT_LINKER_FLAGS}")
969+
message("CMAKE_SHARED_LINKER_FLAGS: ${FFT_LINKER_FLAGS}")
980970
endif()
981971
# Allow `admin` directory to be easily conveyed to nested CMake commands.
982972
set(GMX_ADMIN_DIR ${CMAKE_SOURCE_DIR}/admin)

‎cmake/gmxCFlags.cmake

-14
Original file line numberDiff line numberDiff line change
@@ -105,20 +105,6 @@ function(gmx_target_compile_options TARGET)
105105
$<$<AND:$<COMPILE_LANGUAGE:CXX>,$<CONFIG:${build_type}>>:${GMXC_CXXFLAGS_${build_type}}>
106106
)
107107
endforeach()
108-
# TODO: Restrict the scope of MPI dependence.
109-
# Targets that actually need MPI headers and build tool flags should
110-
# manage their own `target_link_libraries` locally. Such a change is beyond
111-
# the scope of the bug fix for #4678.
112-
if (GMX_LIB_MPI AND TARGET ${TARGET})
113-
target_link_libraries(
114-
${TARGET} PRIVATE
115-
$<$<LINK_LANGUAGE:CXX>:MPI::MPI_CXX>
116-
# We don't know whether we have sought the MPI::C component at all, or at least
117-
# by the time we process these lines.
118-
$<$<AND:$<TARGET_EXISTS:MPI::MPI_C>,$<LINK_LANGUAGE:C>>:MPI::MPI_C>
119-
$<$<LINK_LANGUAGE:CUDA>:MPI::MPI_CXX>
120-
)
121-
endif ()
122108
# Add the release-configuration compiler options to build
123109
# configurations that derive from it.
124110
foreach(build_type RELWITHDEBINFO RELWITHASSERT MINSIZEREL PROFILE)

‎src/gromacs/CMakeLists.txt

+5-1
Original file line numberDiff line numberDiff line change
@@ -234,7 +234,11 @@ if(GMX_GPU_FFT_ONEMATH)
234234
endif()
235235

236236
if (TARGET MPI::MPI_CXX)
237-
target_link_libraries(libgromacs PRIVATE MPI::MPI_CXX)
237+
target_link_libraries(libgromacs PRIVATE
238+
$<$<LINK_LANGUAGE:CXX>:MPI::MPI_CXX>
239+
$<$<LINK_LANGUAGE:CUDA>:MPI::MPI_CXX>
240+
$<$<LINK_LANGUAGE:HIP>:MPI::MPI_CXX>
241+
)
238242
endif ()
239243

240244
target_link_libraries(libgromacs PRIVATE $<BUILD_INTERFACE:common>)

0 commit comments

Comments
 (0)
Please sign in to comment.