Skip to content

Commit 308b785

Browse files
griwesbrycelelbach
authored andcommitted
Address review comments.
1 parent 99a61ac commit 308b785

File tree

1 file changed

+41
-36
lines changed

1 file changed

+41
-36
lines changed

CMakeLists.txt

Lines changed: 41 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ project(Thrust CXX)
55
set(THRUST_SOURCE ${CMAKE_SOURCE_DIR})
66
include(cmake/common_variables.cmake)
77

8-
# Default to a release build.
98
if ("" STREQUAL "${CMAKE_BUILD_TYPE}")
109
set(CMAKE_BUILD_TYPE "RelWithDebInfo" CACHE STRING "Choose the type of build." FORCE)
1110

@@ -15,7 +14,6 @@ if ("" STREQUAL "${CMAKE_BUILD_TYPE}")
1514
)
1615
endif ()
1716

18-
# CONFIGURE_DEPENDS helper
1917
if (CMAKE_VERSION VERSION_GREATER_EQUAL 3.12)
2018
set(CMAKE_CONFIGURE_DEPENDS CONFIGURE_DEPENDS)
2119
endif ()
@@ -65,7 +63,8 @@ endif ()
6563

6664
add_definitions(-DTHRUST_DEVICE_SYSTEM=THRUST_DEVICE_SYSTEM_${THRUST_DEVICE_SYSTEM})
6765

68-
set(CMAKE_CXX_STANDARD 98 CACHE STRING "The C++ version to be used.")
66+
# Please note this also sets the default for the CUDA C++ version; see the comment below.
67+
set(CMAKE_CXX_STANDARD 11 CACHE STRING "The C++ version to be used.")
6968
set(CMAKE_CXX_EXTENSIONS OFF)
7069

7170
message("-- C++ Standard version: ${CMAKE_CXX_STANDARD}")
@@ -82,14 +81,18 @@ if ("CUDA" STREQUAL "${THRUST_DEVICE_SYSTEM}")
8281

8382
enable_language(CUDA)
8483

85-
# force CUDA C++ standard to be the same as the C++ standard used
84+
# Force CUDA C++ standard to be the same as the C++ standard used.
8685
#
87-
# now, CMake is unaligned with reality on standard versions: https://gitlab.kitware.com/cmake/cmake/issues/18597
86+
# Now, CMake is unaligned with reality on standard versions: https://gitlab.kitware.com/cmake/cmake/issues/18597
8887
# which means that using standard CMake methods, it's impossible to actually sync the CXX and CUDA versions for pre-11
8988
# versions of C++; CUDA accepts 98 but translates that to 03, while CXX doesn't accept 03 (and doesn't translate that to 03).
90-
# in case this gives You, dear user, any trouble, please escalate the above CMake bug, so we can support reality properly.
91-
unset (CMAKE_CUDA_STANDARD CACHE)
92-
set (CMAKE_CUDA_STANDARD ${CMAKE_CXX_STANDARD})
89+
# In case this gives You, dear user, any trouble, please escalate the above CMake bug, so we can support reality properly.
90+
if (DEFINED CMAKE_CUDA_STANDARD)
91+
message(WARNING "You've set CMAKE_CUDA_STANDARD; please note that this variable is ignored, and CMAKE_CXX_STANDARD"
92+
" is used as the C++ standard version for both C++ and CUDA.")
93+
endif()
94+
unset(CMAKE_CUDA_STANDARD CACHE)
95+
set(CMAKE_CUDA_STANDARD ${CMAKE_CXX_STANDARD})
9396

9497
set(THRUST_HIGHEST_COMPUTE_ARCH 75)
9598
set(THRUST_KNOWN_COMPUTE_ARCHS 30 32 35 50 52 53 60 61 62 70 72 75)
@@ -141,13 +144,13 @@ if ("TBB" STREQUAL "${THRUST_DEVICE_SYSTEM}")
141144
set (THRUST_ADDITIONAL_LIBRARIES "${TBB_LIBRARIES}")
142145
endif ()
143146

144-
# there's a ton of these in the TBB backend, even though the code is correct
147+
# There's a ton of these in the TBB backend, even though the code is correct.
145148
# TODO: silence these warnings in code instead
146149
append_option_if_available("-Wno-unused-parameter" THRUST_CXX_WARNINGS)
147150
endif ()
148151

149152
if ("MSVC" STREQUAL "${CMAKE_CXX_COMPILER_ID}")
150-
if (CMAKE_CXX_COMPILER_VERSION VERSION_LESS 1700)
153+
if (CMAKE_CXX_COMPILER_VERSION VERSION_LESS 1900)
151154
message(FATAL_ERROR "This version of MSVC no longer supported.")
152155
endif ()
153156
endif ()
@@ -212,8 +215,8 @@ if ("GNU" STREQUAL "${CMAKE_CXX_COMPILER_ID}")
212215
endif ()
213216

214217
if (CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL 8.1 AND CMAKE_CXX_STANDARD EQUAL 98)
215-
# thrust::complex can't really be made trivially copyable in pre-11
216-
# disable a warning about a non-trivially-copyable type being memmoved that was added to GCC 8
218+
# thrust::complex can't really be made trivially copyable in pre-11.
219+
# Disable a warning about a non-trivially-copyable type being memmoved that was added to GCC 8.
217220
append_option_if_available("-Wno-class-memaccess" THRUST_CXX_WARNINGS)
218221
endif ()
219222
endif ()
@@ -245,6 +248,8 @@ endif ()
245248
# For every public header, build a translation unit containing `#include <header>`
246249
# to let the compiler try to figure out warnings in that header if it is not otherwise
247250
# included in tests, and also to verify if the headers are modular enough.
251+
# .inl files are not globbed for, because they are not supposed to be used as public
252+
# entrypoints.
248253
list(APPEND THRUST_HEADER_GLOBS thrust/*.h)
249254
list(APPEND THRUST_HEADER_EXCLUDE_SYSTEMS_GLOBS thrust/system/*/*)
250255

@@ -293,7 +298,7 @@ file(
293298
)
294299
list(REMOVE_ITEM THRUST_HEADERS ${THRUST_HEADER_EXCLUDE_DETAILS})
295300

296-
# list of headers that aren't implemented for all backends, but are implemented for CUDA
301+
# List of headers that aren't implemented for all backends, but are implemented for CUDA.
297302
set(THRUST_PARTIALLY_IMPLEMENTED_HEADERS_CUDA
298303
async/copy.h
299304
async/for_each.h
@@ -304,19 +309,19 @@ set(THRUST_PARTIALLY_IMPLEMENTED_HEADERS_CUDA
304309
future.h
305310
)
306311

307-
# list of headers that aren't implemented for all backends, but are implemented for CPP
312+
# List of headers that aren't implemented for all backends, but are implemented for CPP.
308313
set(THRUST_PARTIALLY_IMPLEMENTED_HEADERS_CPP
309314
)
310315

311-
# list of headers that aren't implemented for all backends, but are implemented for TBB
316+
# List of headers that aren't implemented for all backends, but are implemented for TBB.
312317
set(THRUST_PARTIALLY_IMPLEMENTED_HEADERS_TBB
313318
)
314319

315-
# list of headers that aren't implemented for all backends, but are implemented for OMP
320+
# List of headers that aren't implemented for all backends, but are implemented for OMP.
316321
set(THRUST_PARTIALLY_IMPLEMENTED_HEADERS_OMP
317322
)
318323

319-
# list of all partially implemented headers
324+
# List of all partially implemented headers.
320325
set(THRUST_PARTIALLY_IMPLEMENTED_HEADERS
321326
emptylistguard
322327
${THRUST_PARTIALLY_IMPLEMENTED_HEADERS_CUDA}
@@ -329,9 +334,9 @@ list(REMOVE_DUPLICATES THRUST_PARTIALLY_IMPLEMENTED_HEADERS)
329334

330335
foreach (THRUST_HEADER IN LISTS THRUST_HEADERS)
331336
if ("${THRUST_HEADER}" IN_LIST THRUST_PARTIALLY_IMPLEMENTED_HEADERS)
332-
# this header is partially implemented on _some_ backends
337+
# This header is partially implemented on _some_ backends...
333338
if (NOT "${THRUST_HEADER}" IN_LIST THRUST_PARTIALLY_IMPLEMENTED_HEADERS_${THRUST_DEVICE_SYSTEM})
334-
# but not on the selected one
339+
# ...but not on the selected one.
335340
continue()
336341
endif ()
337342
endif ()
@@ -356,7 +361,7 @@ target_include_directories(
356361
include(CTest)
357362
enable_testing()
358363

359-
# Handle tests
364+
# Handle tests.
360365

361366
set(THRUST_TEST_RUN_ARGUMENTS
362367
-DTHRUST_SOURCE=${CMAKE_SOURCE_DIR}
@@ -366,7 +371,7 @@ list(APPEND THRUST_TESTFRAMEWORK_FILES testing/unittest/testframework.cu)
366371
if ("CUDA" STREQUAL "${THRUST_DEVICE_SYSTEM}")
367372
list(APPEND THRUST_TESTFRAMEWORK_FILES testing/unittest/cuda/testframework.cu)
368373
else ()
369-
# when CUDA is disabled, explain to CMake that testframework.cu is actually a C++ file
374+
# When CUDA is disabled, explain to CMake that testframework.cu is actually a C++ file.
370375
set_source_files_properties(testing/unittest/testframework.cu
371376
PROPERTIES
372377
LANGUAGE CXX
@@ -397,7 +402,7 @@ file(
397402
${THRUST_TEST_GLOBS}
398403
)
399404

400-
# list of tests that aren't implemented for all backends, but are implemented for CUDA
405+
# List of tests that aren't implemented for all backends, but are implemented for CUDA.
401406
set(THRUST_PARTIALLY_IMPLEMENTED_CUDA
402407
async_copy
403408
async_for_each
@@ -409,19 +414,19 @@ set(THRUST_PARTIALLY_IMPLEMENTED_CUDA
409414
future
410415
)
411416

412-
# list of tests that aren't implemented for all backends, but are implemented for CPP
417+
# List of tests that aren't implemented for all backends, but are implemented for CPP.
413418
set(THRUST_PARTIALLY_IMPLEMENTED_CPP
414419
)
415420

416-
# list of tests that aren't implemented for all backends, but are implemented for TBB
421+
# List of tests that aren't implemented for all backends, but are implemented for TBB.
417422
set(THRUST_PARTIALLY_IMPLEMENTED_TBB
418423
)
419424

420-
# list of tests that aren't implemented for all backends, but are implemented for OMP
425+
# List of tests that aren't implemented for all backends, but are implemented for OMP.
421426
set(THRUST_PARTIALLY_IMPLEMENTED_OMP
422427
)
423428

424-
# list of all partially implemented tests
429+
# List of all partially implemented tests.
425430
set(THRUST_PARTIALLY_IMPLEMENTED
426431
${THRUST_PARTIALLY_IMPLEMENTED_CUDA}
427432
${THRUST_PARTIALLY_IMPLEMENTED_CPP}
@@ -431,7 +436,7 @@ set(THRUST_PARTIALLY_IMPLEMENTED
431436

432437
if ("CUDA" STREQUAL "${THRUST_DEVICE_SYSTEM}")
433438
if (14 EQUAL ${CMAKE_CXX_STANDARD})
434-
# temporarily disable until NVBug 2492786 is fixed
439+
# Temporarily disable until NVBug 2492786 is fixed.
435440
list(APPEND THRUST_PARTIALLY_IMPLEMENTED tuple_algorithms)
436441
endif()
437442
endif ()
@@ -452,9 +457,9 @@ foreach (THRUST_TEST_SOURCE IN LISTS THRUST_TESTS)
452457
get_filename_component(THRUST_TEST_NAME ${THRUST_TEST_SOURCE} NAME_WE)
453458

454459
if ("${THRUST_TEST_NAME}" IN_LIST THRUST_PARTIALLY_IMPLEMENTED)
455-
# this test is partially implemented on _some_ backends
460+
# This test is partially implemented on _some_ backends...
456461
if (NOT "${THRUST_TEST_NAME}" IN_LIST THRUST_PARTIALLY_IMPLEMENTED_${THRUST_DEVICE_SYSTEM})
457-
# but not on the selected one
462+
# ...but not on the selected one.
458463
set(THRUST_TEST_CREATION_ADDITIONAL EXCLUDE_FROM_ALL)
459464
set(THRUST_TEST_ADD_TO_CTEST OFF)
460465
endif ()
@@ -463,8 +468,8 @@ foreach (THRUST_TEST_SOURCE IN LISTS THRUST_TESTS)
463468
set(THRUST_TEST "thrust.test.${THRUST_TEST_CATEGORY}${THRUST_TEST_NAME}")
464469

465470
if (NOT "CUDA" STREQUAL "${THRUST_DEVICE_SYSTEM}")
466-
# test files are generally .cu; if CUDA is not enabled, CMake doesn't know what to
467-
# do with them. but since they are pretty much just C++, we can compile them with
471+
# Test files are generally .cu; if CUDA is not enabled, CMake doesn't know what to
472+
# do with them. But since they are pretty much just C++, we can compile them with
468473
# non-nvcc C++ compilers... but we need to tell CMake that they are, in fact, just C++.
469474
set_source_files_properties(${PROJECT_SOURCE_DIR}/testing/${THRUST_TEST_SOURCE}
470475
PROPERTIES
@@ -475,7 +480,7 @@ foreach (THRUST_TEST_SOURCE IN LISTS THRUST_TESTS)
475480
add_executable(
476481
${THRUST_TEST}
477482
${THRUST_TEST_CREATION_ADDITIONAL}
478-
483+
# THRUST_TEST_CREATION_ADDITIONAL is actually a CMake keyword (sometimes).
479484
${PROJECT_SOURCE_DIR}/testing/${THRUST_TEST_SOURCE}
480485
)
481486

@@ -502,7 +507,7 @@ foreach (THRUST_TEST_SOURCE IN LISTS THRUST_TESTS)
502507
add_executable(
503508
${THRUST_TEST_RDC}
504509
${THRUST_TEST_CREATION_ADDITIONAL}
505-
510+
# THRUST_TEST_CREATION_ADDITIONAL is actually a CMake keyword (sometimes).
506511
${PROJECT_SOURCE_DIR}/testing/${THRUST_TEST_SOURCE}
507512
)
508513

@@ -528,7 +533,7 @@ foreach (THRUST_TEST_SOURCE IN LISTS THRUST_TESTS)
528533
endif ()
529534
endforeach ()
530535

531-
# Handle examples
536+
# Handle examples.
532537

533538
option(THRUST_EXAMPLE_FILECHECK_PATH "Path to the LLVM FileCheck utility." "")
534539
option(THRUST_ENABLE_EXAMPLES_WITH_RDC "Also build all examples with RDC." OFF)
@@ -591,8 +596,8 @@ foreach (THRUST_EXAMPLE_SOURCE IN LISTS THRUST_EXAMPLES)
591596
set(THRUST_EXAMPLE "thrust.example.${THRUST_EXAMPLE_CATEGORY}${THRUST_EXAMPLE_NAME}")
592597

593598
if (NOT "CUDA" STREQUAL "${THRUST_DEVICE_SYSTEM}")
594-
# example files are generally .cu; if CUDA is not enabled, CMake doesn't know what to
595-
# do with them. but since they are pretty much just C++, we can compile them with
599+
# Example files are generally .cu; if CUDA is not enabled, CMake doesn't know what to
600+
# do with them. But since they are pretty much just C++, we can compile them with
596601
# non-nvcc C++ compilers... but we need to tell CMake that they are, in fact, just C++.
597602
set_source_files_properties(${PROJECT_SOURCE_DIR}/examples/${THRUST_EXAMPLE_SOURCE}
598603
PROPERTIES

0 commit comments

Comments
 (0)