Skip to content

Commit

Permalink
Refactor CMake: prefixing, use CMAKE_CUDA_ARCHITECTURES, general impr…
Browse files Browse the repository at this point in the history
…ovments

+ Prefixes CMake option and cache variables with FLAMEGPU_VISUALISER
+ Prefix CMake macros and functions with flamegpu_visualiser_ and use lower snake case
+ Remove unneeded reference to SEATBELTS from CMake
+ Mark many CMake variables as advanced to simplify the CMake GUI
+ Fix cmake_minimimum_requires statemeents and add a policy_max of 3.25
  + This requied a fix for a CMake 3.21 policy
+ Use more-modern CMake C++/CUDA standard setting
+ Modernise/simplify GLM CMake and use PUBLIC/PRIVATE target_link_libraries
+ Provide list of DLLs via flamegpu_visualiser_get_runtime_depenencies to simplify code in FLAMEGPU/FLAMEGPU2
  • Loading branch information
ptheywood committed Dec 1, 2022
1 parent dcfeb74 commit b4b3c3d
Show file tree
Hide file tree
Showing 28 changed files with 835 additions and 422 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/Lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ jobs:
- name: Configure cmake
run: >
cmake . -B "${{ env.BUILD_DIR }}"
-DALLOW_LINT_ONLY=ON
-DFLAMEGPU_ALLOW_LINT_ONLY=ON
- name: Lint
working-directory: ${{ env.BUILD_DIR }}
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/Manylinux2014.yml
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,8 @@ jobs:
-DCMAKE_BUILD_TYPE="${{ env.CONFIG }}"
-Werror=dev
-DCMAKE_WARN_DEPRECATED="OFF"
-DWARNINGS_AS_ERRORS="OFF"
-DCUDA_ARCH="${{ env.CUDA_ARCH }}"
-DFLAMEGPU_WARNINGS_AS_ERRORS="OFF"
-DCMAKE_CUDA_ARCHITECTURES="${{ env.CUDA_ARCH }}"
-DGLEW_USE_STATIC_LIBS="${{ env.USE_STATIC_GLEW }}"
-DOpenGL_GL_PREFERENCE:STRING=LEGACY
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/Ubuntu.yml
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,8 @@ jobs:
-DCMAKE_BUILD_TYPE="${{ env.CONFIG }}"
-Werror=dev
-DCMAKE_WARN_DEPRECATED="OFF"
-DWARNINGS_AS_ERRORS="ON"
-DCUDA_ARCH="${{ env.CUDA_ARCH }}"
-DFLAMEGPU_WARNINGS_AS_ERRORS="ON"
-DCMAKE_CUDA_ARCHITECTURES="${{ env.CUDA_ARCH }}"
- name: Build
working-directory: ${{ env.BUILD_DIR }}
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/Windows.yml
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,8 @@ jobs:
-G "${{ env.HOSTCXX }}" -A x64
-Werror=dev
-DCMAKE_WARN_DEPRECATED="OFF"
-DWARNINGS_AS_ERRORS="ON"
-DCUDA_ARCH="${{ env.CUDA_ARCH }}"
-DFLAMEGPU_WARNINGS_AS_ERRORS="ON"
-DCMAKE_CUDA_ARCHITECTURES="${{ env.CUDA_ARCH }}"
- name: Build
working-directory: ${{ env.BUILD_DIR }}
Expand Down
11 changes: 4 additions & 7 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# Parity with main FGPU2 CMake
cmake_minimum_required(VERSION VERSION 3.18 FATAL_ERROR)
cmake_minimum_required(VERSION 3.18...3.25 FATAL_ERROR)

project(FLAMEGPU2_VISUALISER LANGUAGES NONE)

Expand All @@ -13,18 +13,15 @@ include(${CMAKE_CURRENT_SOURCE_DIR}/cmake/OutOfSourceOnly.cmake)
# Define some CMake Options which apply to all (sub) projects

# Option to promote compilation warnings to error, useful for strict CI
option(WARNINGS_AS_ERRORS "Promote compilation warnings to errors" OFF)
option(FLAMEGPU_WARNINGS_AS_ERRORS "Promote compilation warnings to errors" OFF)

# Option to group CMake generated projects into folders in supported IDEs
option(CMAKE_USE_FOLDERS "Enable folder grouping of projects in IDEs." ON)
mark_as_advanced(CMAKE_USE_FOLDERS)

# Add a new cmake option, to allow lint_only configurations.
option(ALLOW_LINT_ONLY "Allow the project to be configured for lint-only builds" OFF)
mark_as_advanced(ALLOW_LINT_ONLY)

# Control target CUDA_ARCH to compile for
SET(CUDA_ARCH "${CUDA_ARCH}" CACHE STRING "List of CUDA Architectures to target. E.g. 61;70" FORCE)
option(FLAMEGPU_ALLOW_LINT_ONLY "Allow the project to be configured for lint-only builds" OFF)
mark_as_advanced(FLAMEGPU_ALLOW_LINT_ONLY)

# Add the src subdirectory cmake project.
add_subdirectory(src "${PROJECT_BINARY_DIR}/FLAMEGPU_visualiser")
8 changes: 4 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ To build the visualiser under linux in Release mode using the command line:
# Create a build directory
mkdir -p build
cd build
# Configure CMake, i.e. for Release builds for Pascal and newer GPUs
cmake .. -DCUDA_ARCH=61 -DCMAKE_BUILD_TYPE=Release
# Configure CMake, i.e. for Release builds for consumer Pascal and newer GPUs
cmake .. -DCMAKE_CUDA_ARCHITECTURES=61 -DCMAKE_BUILD_TYPE=Release
# Build all targets using 8 threads.
cmake --build . --target all -j 8
# Or directly via make
Expand Down Expand Up @@ -65,10 +65,10 @@ cmake --open .

The project can be configured to allow linting without the need for CUDA or OpenGL to be available (i.e. CI).

To do this, set the `ALLOW_LINT_ONLY` CMake option to `ON`. I.e.:
To do this, set the `FLAMEGPU_ALLOW_LINT_ONLY` CMake option to `ON`. I.e.:

```bash
cmake .. -DALLOW_LINT_ONLY=ON
cmake .. -DFLAMEGPU_ALLOW_LINT_ONLY=ON
cmake --build . --target lint_flamegpu_visualiser
```

Expand Down
299 changes: 299 additions & 0 deletions cmake/CUDAArchitectures.cmake

Large diffs are not rendered by default.

9 changes: 9 additions & 0 deletions cmake/CUDAArchitecturesProjectInclude.cmake
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# CMake file to be injected into a project via CMAKE_PROJECT<PROJECT-NAME>_INCLUDE
# If CUDA is enabled, call a CMake function which gracefully sets a library-useful

# Set a locally scoped cmake variable, to alter the error message within.
set(flamegpu_visualiser_IN_PROJECT_INCLUDE ON)
# Call the appropraite command to set CMAKE_CUDA_ARCHITECTURES to the user-provided value, the exising value, or a sane libray-provided defualt
flamegpu_visualiser_set_cuda_architectures()
# Unset the variable used to alter behaviour in set_cuda_architectures
unset(flamegpu_visualiser_IN_PROJECT_INCLUDE)
147 changes: 147 additions & 0 deletions cmake/CheckCompilerFunctionality.cmake
Original file line number Diff line number Diff line change
@@ -0,0 +1,147 @@
include_guard(GLOBAL)

# Define a cmake function which checks that CUDA and the host compiler are functional.
# Failure only results in CMake warnings not Errors, so that documentation only builds will function.
function(flamegpu_visualiser_check_compiler_functionality)
# If the result variable is already defined, this has already been called once, so don't check agian.
if(DEFINED FLAMEGPU_VISUALISER_CheckCompilerFunctionality_RESULT)
return()
endif()

# Ensure that required languages are avialable.
# Enabling the languages must however be perfomed in file scope, so do this in the root cmake after docs only checks have been found.
include(CheckLanguage)
check_language(CXX)
if(NOT CMAKE_CXX_COMPILER)
message(WARNING "CXX Language Support Not Found")
set(FLAMEGPU_VISUALISER_CheckCompilerFunctionality_RESULT "NO" PARENT_SCOPE)
return()
endif()
enable_language(CXX)
check_language(CUDA)
if(NOT CMAKE_CUDA_COMPILER)
message(WARNING "CUDA Language Support Not Found")
set(FLAMEGPU_VISUALISER_CheckCompilerFunctionality_RESULT "NO" PARENT_SCOPE)
return()
endif()
enable_language(CUDA)

# We need c++17 std::filesytem, but not all compilers which claim to implement c++17 provide filesystem (GCC 7)
if(NOT DEFINED CUDA_STD_FILESYSTEM)
# Disable CMAKE_CUDA_ARCHTIECTURES if not already controlled. This is scoped to the function so safe to control.
if(NOT DEFINED CMAKE_CUDA_ARCHITECTURES OR "${CMAKE_CUDA_ARCHITECTURES}" STREQUAL "")
set(CMAKE_CUDA_ARCHITECTURES "OFF")
endif()
try_compile(
CUDA_STD_FILESYSTEM
"${CMAKE_CURRENT_BINARY_DIR}/try_compile"
"${CMAKE_CURRENT_LIST_DIR}/CheckCompilerFunctionality/CheckStdFilesystem.cu"
CXX_STANDARD 17
CUDA_STANDARD 17
CXX_STANDARD_REQUIRED "ON"
)
endif()
# If an error occured while building the <filesystem> snippet, report a warning
if(NOT CUDA_STD_FILESYSTEM)
# If the GCC versions is known to be bad, give an appropriate error
if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU" AND CMAKE_CXX_COMPILER_VERSION VERSION_LESS 8.1)
message(WARNING
"${CMAKE_CXX_COMPILER_ID} ${CMAKE_CXX_COMPILER_VERSION} does not provide <std::filesystem> even in --std=c++17 mode.\n"
" Please use GCC >= 8.1.\n"
" \n")
else()
# If the gcc version is not a known problem, emit a generic error.
message(WARNING
"<std::filesystem> error with ${CMAKE_CUDA_COMPILER_ID} ${CMAKE_CUDA_COMPILER_VERSION} and ${CMAKE_CXX_COMPILER_ID} ${CMAKE_CXX_COMPILER_VERSION}.")
endif()
# Set the result variable to a false-like value
set(FLAMEGPU_VISUALISER_CheckCompilerFunctionality_RESULT "NO" PARENT_SCOPE)
return()
endif()

# Original releases of GCC 10.3.0 and 11.1.0 included a bug preventing the use of <chrono> in <nvcc>.
# This was patched in subsequent versions, and backported in the release branches, but the broken version is still distributed in some cases (i.e. Ubuntu 20.04, but not 21.04).
# See https://github.com/FLAMEGPU/FLAMEGPU2/issues/575, https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100102
if (CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
# Try to compile the test case file for inclusion of chrono.
if(NOT DEFINED GCC_CUDA_STDCHRONO)
# Disable CMAKE_CUDA_ARCHTIECTURES if not already controlled. This is scoped to the function so safe to control.
if(NOT DEFINED CMAKE_CUDA_ARCHITECTURES OR "${CMAKE_CUDA_ARCHITECTURES}" STREQUAL "")
set(CMAKE_CUDA_ARCHITECTURES "OFF")
endif()
try_compile(
GCC_CUDA_STDCHRONO
"${CMAKE_CURRENT_BINARY_DIR}/try_compile"
"${CMAKE_CURRENT_LIST_DIR}/CheckCompilerFunctionality/CheckStdChrono.cu"
CXX_STANDARD 17
CUDA_STANDARD 17
CXX_STANDARD_REQUIRED "ON"
)
endif()
# If an error occured while building the <chrono> snippet, report a warning
if(NOT GCC_CUDA_STDCHRONO)
# If the GCC versions is known to be bad, give an appropriate error
if(CMAKE_CXX_COMPILER_VERSION VERSION_EQUAL 10.3.0 OR CMAKE_CXX_COMPILER_VERSION VERSION_EQUAL 11.1.0)
message(WARNING
"GCC ${CMAKE_CXX_COMPILER_VERSION} is incompatible with CUDA due to a bug in the <chrono> implementation.\n"
" Please use an alternative GCC, or a patched version of GCC ${CMAKE_CXX_COMPILER_VERSION}.\n"
" \n"
" See the following for more information:\n"
" https://github.com/FLAMEGPU/FLAMEGPU2/issues/575\n"
" https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100102\n")
else()
# If the gcc version is not a known problem, emit a generic error.
message(WARNING
"<std::chrono> not usable with ${CMAKE_CUDA_COMPILER_ID} ${CMAKE_CUDA_COMPILER_VERSION} and ${CMAKE_CXX_COMPILER_ID} ${CMAKE_CXX_COMPILER_VERSION}.")
endif()
# Set the result variable to a false-like value
set(FLAMEGPU_VISUALISER_CheckCompilerFunctionality_RESULT "NO" PARENT_SCOPE)
return()
endif()
endif()

# GCC 9 + CUDA 11.0 + std=c++17 errors when attempting to compile std::vector<std::tuple<>>::push_back.
# This is the only known bad combination, but let's check more often just in case.
# We no longer use this (intentionally) so if the error occurs emit a warning but not error?
# See https://github.com/FLAMEGPU/FLAMEGPU2/issues/575, https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100102
if (CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
# Try to compile the test case file for inclusion of chrono.
if(NOT DEFINED GCC_CUDA_VECTOR_TUPLE_PUSHBACK)
# Disable CMAKE_CUDA_ARCHTIECTURES if not already controlled. This is scoped to the function so safe to control.
if(NOT DEFINED CMAKE_CUDA_ARCHITECTURES OR "${CMAKE_CUDA_ARCHITECTURES}" STREQUAL "")
set(CMAKE_CUDA_ARCHITECTURES "OFF")
endif()
try_compile(
GCC_CUDA_VECTOR_TUPLE_PUSHBACK
"${CMAKE_CURRENT_BINARY_DIR}/try_compile"
"${CMAKE_CURRENT_LIST_DIR}/CheckCompilerFunctionality/CheckVectorTuplePushBack.cu"
CXX_STANDARD 17
CUDA_STANDARD 17
CXX_STANDARD_REQUIRED "ON"
)
endif()
# If an error occured while building MWE emit a dev warning.
if(NOT GCC_CUDA_VECTOR_TUPLE_PUSHBACK)
# If the GCC versions is known to be bad, give an appropriate error
message(AUTHOR_WARNING
"std::vector<std::tuple<>>::push_back cannot be compiled with ${CMAKE_CUDA_COMPILER_ID} ${CMAKE_CUDA_COMPILER_VERSION} and ${CMAKE_CXX_COMPILER_ID} ${CMAKE_CXX_COMPILER_VERSION} with --std=c++17.\n"
"Consider using a different ${CMAKE_CUDA_COMPILER_ID} and ${CMAKE_CXX_COMPILER_ID} combination as errors may be encountered.\n"
"See https://github.com/FLAMEGPU/FLAMEGPU2/issues/650")
# Not erroring, so don't change the output value, just emit the above developer warning

# The compilation error is somewhat tempremental, so always emit a warning for the known bad combination
elseif(CMAKE_CUDA_COMPILER_VERSION VERSION_GREATER_EQUAL "11.0" AND CMAKE_CUDA_COMPILER_VERSION VERSION_LESS "11.1" AND CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL "9" AND CMAKE_CXX_COMILER_VERSION VERSION_LESS "10")
message(AUTHOR_WARNING
"CUDA 11.0 with g++ 9 in c++17 mode may encounter compiler segmentation faults with 'std::vector<std::tuple<...>>::push_back'.\n"
"Consider using CUDA 11.1+ or gcc 8 to avoid potential issues.\n"
"See https://github.com/FLAMEGPU/FLAMEGPU2/issues/650")
endif()
endif()

# If we made it this far, set the result variable to be truthy
set(FLAMEGPU_VISUALISER_CheckCompilerFunctionality_RESULT "YES" PARENT_SCOPE)
endfunction()


# Call the function imediately, so the file only needs to be included.
flamegpu_visualiser_check_compiler_functionality()
7 changes: 7 additions & 0 deletions cmake/CheckCompilerFunctionality/CheckStdChrono.cu
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// Including chrono is enough to trigger the compilation error being tested for. See:
// https://github.com/FLAMEGPU/FLAMEGPU2/issues/575
// https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100102
#include <chrono>

// CMake try_compile performs a link step, so main is required
int main() { return 0; }
4 changes: 4 additions & 0 deletions cmake/CheckCompilerFunctionality/CheckStdFilesystem.cu
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// We require std::filesytem, but just requireing std=c++17 does not enforce this for all compilers, so check it works. (I.e. GCC < 8 is a problem.)
// CMake doesn't appear to have knowledge of this feature.
#include <filesystem>
int main() { return 0; }
11 changes: 11 additions & 0 deletions cmake/CheckCompilerFunctionality/CheckVectorTuplePushBack.cu
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// GCC 9 + CUDA 11.0 + --std=c++17 errors during complation of std::vector<std::tuple<>>::push_back.
// Test this for the configured CUDA, incase it is OK on the given system
// See https://github.com/FLAMEGPU/FLAMEGPU2/issues/650 for more information
#include <tuple>
#include <vector>

int main (int argc, char * argv[]) {
std::vector<std::tuple<float>> v;
std::tuple<float> t = {1.f};
v.push_back(t); // segmentation fault
}
16 changes: 4 additions & 12 deletions cmake/CommonCompilerSettings.cmake
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
include_guard(GLOBAL)
# Define a function which can be used to set common compiler options for a target
# We do not want to force these options on end users (although they should be used ideally), hence not just public properties on the library target
# Function to suppress compiler warnings for a given target
function(CommonCompilerSettings)
function(flamegpu_visualiser_common_compiler_settings)
# Parse the expected arguments, prefixing variables.
cmake_parse_arguments(
CCS
Expand All @@ -13,9 +14,9 @@ function(CommonCompilerSettings)

# Ensure that a target has been passed, and that it is a valid target.
if(NOT CCS_TARGET)
message( FATAL_ERROR "function(CommonCompilerSettings): 'TARGET' argument required")
message( FATAL_ERROR "flamegpu_visualiser_common_compiler_settings: 'TARGET' argument required")
elseif(NOT TARGET ${CCS_TARGET} )
message( FATAL_ERROR "function(CommonCompilerSettings): TARGET '${CCS_TARGET}' is not a valid target")
message( FATAL_ERROR "flamegpu_visualiser_common_compiler_settings: TARGET '${CCS_TARGET}' is not a valid target")
endif()

# Add device debugging symbols to device builds of CUDA objects
Expand All @@ -34,15 +35,6 @@ function(CommonCompilerSettings)
target_compile_definitions(${CCS_TARGET} PRIVATE NOMINMAX)
endif()

# Pass the SEATBELTS macro, which when set to off/0 (for non debug builds) removes expensive operations.
if (SEATBELTS)
# If on, all build configs have seatbelts
target_compile_definitions(${CCS_TARGET} PRIVATE SEATBELTS=1)
else()
# Id off, debug builds have seatbelts, non debug builds do not.
target_compile_definitions(${CCS_TARGET} PRIVATE $<IF:$<CONFIG:Debug>,SEATBELTS=1,SEATBELTS=0>)
endif()

# MSVC handling of SYSTEM for external includes.
if(CMAKE_CXX_COMPILER_ID STREQUAL "MSVC" AND CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL 19.10)
# These flags don't currently have any effect on how CMake passes system-private includes to msvc (VS 2017+)
Expand Down
6 changes: 4 additions & 2 deletions cmake/OutOfSourceOnly.cmake
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
include_guard(GLOBAL)

# Define a cmake function which emits a fatal error if the source directory and binary directory are the same.
function(EnforceOutOfSourceBuilds)
function(flamegpu_visualiser_enforce_out_of_source_builds)
# Resolve paths before comparioson to ensure comparions are accurate
get_filename_component(source_dir "${CMAKE_SOURCE_DIR}" REALPATH)
get_filename_component(binary_dir "${CMAKE_BINARY_DIR}" REALPATH)
Expand All @@ -15,5 +17,5 @@ function(EnforceOutOfSourceBuilds)
endfunction()

# Call the function imediately, so the file only needs to be included.
EnforceOutOfSourceBuilds()
flamegpu_visualiser_enforce_out_of_source_builds()

17 changes: 17 additions & 0 deletions cmake/SetTargetFolder.cmake
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
include_guard(GLOBAL)

#-----------------------------------------------------------------------
# a macro that only sets the FOLDER target property if it's
# "appropriate"
# Borrowed from cmake's own CMakeLists.txt
#-----------------------------------------------------------------------
macro(flamegpu_visualiser_set_target_folder tgt folder)
if(CMAKE_USE_FOLDERS)
set_property(GLOBAL PROPERTY USE_FOLDERS ON)
if(TARGET ${tgt}) # AND MSVC # AND MSVC stops all lint from being set with folder
set_property(TARGET "${tgt}" PROPERTY FOLDER "${folder}")
endif()
else()
set_property(GLOBAL PROPERTY USE_FOLDERS OFF)
endif()
endmacro()
Loading

0 comments on commit b4b3c3d

Please sign in to comment.