Fixes to broken CUDA compiler flags
authorMark Abraham <mark.j.abraham@gmail.com>
Wed, 7 Aug 2019 13:03:51 +0000 (15:03 +0200)
committerPaul Bauer <paul.bauer.q@gmail.com>
Mon, 23 Sep 2019 09:18:49 +0000 (11:18 +0200)
Because FindCUDA.cmake is non-idiomatic CMake, recent changes to CUDA
compilation lost most of our custom compilation options for host code.
These are now restored.

Compilation options for build configurations deriving from the Release
build configuration are also restored.

Refs #3099

Change-Id: If030a256481faed425379dab00ea658e3c215867

cmake/gmxCFlags.cmake
cmake/gmxManageNvccConfig.cmake
src/CMakeLists.txt
src/gromacs/CMakeLists.txt

index c42cfaed9e47d9b3d56bc8cd23b200717549775a..3b025a32b797b6697cb6e4041dbf5a281ec535ce 100644 (file)
@@ -54,32 +54,19 @@ MACRO(GMX_TEST_CXXFLAG VARIABLE FLAGS CXXFLAGSVAR)
     ENDIF ()
 ENDMACRO(GMX_TEST_CXXFLAG VARIABLE FLAGS CXXFLAGSVAR)
 
-# Set the real CMake variables for compiler flags. This should be a function
-# so we can have proper local variables while avoiding duplicating code.
-function(gmx_set_cmake_compiler_flags)
-    foreach(language C CXX)
-        # Copy the flags for the release build type to the build types
-        # that are modified forms of it. Ideally, the list of build
-        # types that are modifications of the Release build type would
-        # be set up elsewhere and passed to this function, but it is
-        # inconvenient in CMake to pass more than one list, and such a
-        # list is only used here.
-        foreach(build_type RELWITHDEBINFO RELWITHASSERT MINSIZEREL PROFILE)
-            set(GMXC_${language}FLAGS_${build_type} "${GMXC_${language}FLAGS_RELEASE}" "${GMXC_${language}FLAGS_${build_type}}")
-        endforeach()
-        # Copy the flags that are only used by the real Release build
-        # type. Used for, e.g., -Wno-array-bounds in Release to work around
-        # gcc-4.8 being a little too vocal about some perfectly good code,
-        # while using RelWithAssert (ie. without that suppression) in Jenkins.
-        #
-        # TODO check the behaviour of more recent gcc on the improved code
-        set(GMXC_${language}FLAGS_RELEASE "${GMXC_${language}FLAGS_RELEASE}" "${GMXC_${language}FLAGS_RELEASE_ONLY}")
-    endforeach()
+# Prepare some local variables so CUDA and non-CUDA code in targets
+# works the same way.
+function(gmx_target_compile_options_inner)
+    set (CFLAGS "${SIMD_C_FLAGS};${MPI_COMPILE_FLAGS};${EXTRA_C_FLAGS};${GMXC_CFLAGS}" PARENT_SCOPE)
+    set (CXXFLAGS "${SIMD_CXX_FLAGS};${MPI_COMPILE_FLAGS};${EXTRA_CXX_FLAGS};${GMXC_CXXFLAGS}" PARENT_SCOPE)
 endfunction()
 
-# Add compiler flags expected for all GROMACS build configurations,
-# and those expected for the current CMake build type (e.g. Release)
-# to TARGET.
+# Implementation function to add compiler flags expected for all
+# GROMACS build configurations, and those expected for the current
+# CMake build type (e.g. Release) to TARGET. Other GROMACS CMake code
+# is expected to use either gmx_target_compile_options(name_of_target)
+# or gmx_cuda_target_compile_options(name_of_variable) because CUDA
+# compilation has special requirements.
 #
 # Most targets (ie. libraries, executables) need compiler flags that
 # are characteristic of the build configuration. This function
@@ -88,20 +75,72 @@ endfunction()
 # that setting will apply globally, which means it applies also to
 # "external" code that the build of GROMACS might also build.
 function(gmx_target_compile_options TARGET)
-    if (NOT GMX_SKIP_DEFAULT_CFLAGS)
+    if (GMX_SKIP_DEFAULT_CFLAGS)
+        return()
+    endif()
+
+    # Prepare the generic compiler options
+    gmx_target_compile_options_inner()
+    target_compile_options(${TARGET}
+        PRIVATE
+        $<$<COMPILE_LANGUAGE:C>:${CFLAGS}>
+        $<$<COMPILE_LANGUAGE:CXX>:${CXXFLAGS}>
+        )
+    # Add compiler options for the build types
+    foreach(build_type ${build_types_with_explicit_flags})
         target_compile_options(${TARGET}
-            PRIVATE
-            $<$<COMPILE_LANGUAGE:C>:${SIMD_C_FLAGS};${MPI_COMPILE_FLAGS};${EXTRA_C_FLAGS};${GMXC_CFLAGS}>
-            $<$<COMPILE_LANGUAGE:CXX>:${SIMD_CXX_FLAGS};${MPI_COMPILE_FLAGS};${EXTRA_CXX_FLAGS};${GMXC_CXXFLAGS}>
+            BEFORE PRIVATE
+            $<$<AND:$<COMPILE_LANGUAGE:C>,$<CONFIG:${build_type}>>:${GMXC_CFLAGS_${build_type}}>
+            $<$<AND:$<COMPILE_LANGUAGE:CXX>,$<CONFIG:${build_type}>>:${GMXC_CXXFLAGS_${build_type}}>
             )
-        foreach(build_type ${build_types_with_explicit_flags})
-            target_compile_options(${TARGET}
-                BEFORE PRIVATE
-                $<$<AND:$<COMPILE_LANGUAGE:C>,$<CONFIG:${build_type}>>:${GMXC_CFLAGS_${build_type}}>
-                $<$<AND:$<COMPILE_LANGUAGE:CXX>,$<CONFIG:${build_type}>>:${GMXC_CXXFLAGS_${build_type}}>
-                )
-        endforeach()
+    endforeach()
+    # Add the release-configuration compiler options to build
+    # configurations that derive from it.
+    foreach(build_type RELWITHDEBINFO RELWITHASSERT MINSIZEREL PROFILE)
+        target_compile_options(${TARGET}
+            BEFORE PRIVATE
+            $<$<AND:$<COMPILE_LANGUAGE:C>,$<CONFIG:${build_type}>>:${GMXC_CFLAGS_RELEASE}>
+            $<$<AND:$<COMPILE_LANGUAGE:CXX>,$<CONFIG:${build_type}>>:${GMXC_CXXFLAGS_RELEASE}>
+            )
+    endforeach()
+    # Add those flags that we only want in the proper release build
+    # configuration.
+    target_compile_options(${TARGET}
+        BEFORE PRIVATE
+        $<$<AND:$<COMPILE_LANGUAGE:C>,$<CONFIG:RELEASE>>:${GMXC_CFLAGS_RELEASE_ONLY}>
+        $<$<AND:$<COMPILE_LANGUAGE:CXX>,$<CONFIG:RELEASE>>:${GMXC_CXXFLAGS_RELEASE_ONLY}>
+        )
+endfunction()
+
+# The approach taken by FindCUDA.cmake is to require that the compiler
+# flags are known and present in a variable before creating the target
+# for the library. (Those get embedded in files that are generated at
+# the point of calling cuda_add_library, which does not create a
+# target that one could later call target_compile_options upon.) So,
+# this function instead returns appropriate content in
+# ${VARIABLE_NAME}, along with other such variables that are
+# specialized for the various build_types. Hopefully this will improve
+# when we use native CUDA language support in our CMake.
+function(gmx_cuda_target_compile_options VARIABLE_NAME)
+    if (GMX_SKIP_DEFAULT_CFLAGS)
+        set (CXXFLAGS "")
+    else()
+        gmx_target_compile_options_inner()
     endif()
+
+    # Only C++ compilation is supported with CUDA code in GROMACS
+    set(${VARIABLE_NAME} ${CXXFLAGS} PARENT_SCOPE)
+
+    # Now organize the flags for different build
+    # configurations. First, the debug configuration.
+    set(${VARIABLE_NAME}_DEBUG "${GMXC_CXXFLAGS_DEBUG}" PARENT_SCOPE)
+    # Add those flags that we only want in the proper release build
+    # configuration.
+    set(${VARIABLE_NAME}_RELEASE "${GMXC_CXXFLAGS_RELEASE};${GMXC_CXXFLAGS_RELEASE_ONLY}" PARENT_SCOPE)
+    # Add the release flags to build configurations that derive from it
+    foreach(build_type RELWITHDEBINFO RELWITHASSERT MINSIZEREL PROFILE)
+        set(${VARIABLE_NAME}_${build_type} "${GMXC_CXXFLAGS_RELEASE};${GMXC_CXXFLAGS_${build_type}}" PARENT_SCOPE)
+    endforeach()
 endfunction()
 
 # Add the WARNING_FLAG to the compile options for TARGET if the
@@ -384,9 +423,4 @@ GMX_TEST_CFLAG(CFLAGS_WARN "/W3;/wd161;/wd177;/wd411;/wd593;/wd981;/wd1418;/wd14
         GMX_TEST_CXXFLAG(CXXFLAG_OPT "-Kfast,reduction,swp,simd=2,uxsimd,fsimple;-x100" GMXC_CXXFLAGS)
     endif()
 
-    # now actually set the flags:
-    if (NOT GMX_SKIP_DEFAULT_CFLAGS)
-        gmx_set_cmake_compiler_flags()
-    endif()
-
 endmacro()
index 63fe528599d4b3a43f55177cf9be19bd82c8c575..67f42e828ef61f09312832b5d1f3fba078429aee 100644 (file)
@@ -201,3 +201,27 @@ endif() # GMX_CHECK_NVCC
 macro(GMX_SET_CUDA_NVCC_FLAGS)
     set(CUDA_NVCC_FLAGS "${GMX_CUDA_NVCC_FLAGS};${CUDA_NVCC_FLAGS}")
 endmacro()
+
+# This helper function creates a temporary scope in which we can set
+# the definitions, include directories and magic host-compiler-flag
+# variables that have to be set in advance of calling
+# cuda_add_library(). This is the only way cuda_add_library() can
+# modify the command line used for host compilation. It is not
+# possible to use the standard CMake mechanisms like
+# target_compile_options() to add such things to targets after they
+# are created.
+function(gmx_cuda_add_library TARGET)
+    add_definitions(-DHAVE_CONFIG_H)
+    # Source files generated by NVCC can include gmxmpi.h, and so
+    # need access to thread-MPI.
+    include_directories(SYSTEM ${PROJECT_SOURCE_DIR}/src/external/thread_mpi/include)
+
+    # Now add all the compilation options
+    gmx_cuda_target_compile_options(CUDA_${TARGET}_CXXFLAGS)
+    list(APPEND CMAKE_CXX_FLAGS ${CUDA_${TARGET}_CXXFLAGS})
+    foreach(build_type ${build_types_with_explicit_flags})
+        list(APPEND CMAKE_CXX_FLAGS_${build_type} ${CUDA_${TARGET}_CXXFLAGS_${build_type}})
+    endforeach()
+
+    cuda_add_library(${TARGET} ${ARGN})
+endfunction()
index 4c246b291ed6e59d05c91d312066d95eb30fd8f1..2c7a63bf7d5c7025f4c145ac52c375fa57594505 100644 (file)
@@ -41,9 +41,6 @@ get_compiler_info(CXX BUILD_CXX_COMPILER BUILD_CXXFLAGS)
 if(GMX_USE_CUDA)
     if(NOT GMX_CLANG_CUDA)
         GMX_SET_CUDA_NVCC_FLAGS()
-        # Source files generated by NVCC can include gmxmpi.h, and so
-        # need access to thread-MPI.
-        set(CUDA_INCLUDE_DIRS ${PROJECT_SOURCE_DIR}/src/external/thread_mpi/include)
     endif()
 
     get_cuda_compiler_info(CUDA_COMPILER_INFO CUDA_COMPILER_FLAGS)
index 8e0d5e06a0b6cb9d8eab9504ba8a87122b2e477f..70260da57e44f8e4c3e93122595d08f9597a2806 100644 (file)
@@ -179,7 +179,7 @@ if (GMX_USE_CUDA)
     # with keywords otherwise...
     set(CUDA_LIBRARIES PRIVATE ${CUDA_LIBRARIES})
     if (NOT GMX_CLANG_CUDA)
-        cuda_add_library(libgromacs ${LIBGROMACS_SOURCES})
+        gmx_cuda_add_library(libgromacs ${LIBGROMACS_SOURCES})
     else()
         add_library(libgromacs ${LIBGROMACS_SOURCES})
     endif()