Fix compiling NVCC 11 with C++17 #3085

masterleinad · 2020-06-09T01:25:13Z

Fixes #3083. This is what I need to compile with NVCC 11 with Kokkos_CXX_STANDARD=17.

codecov-commenter · 2020-06-09T15:01:26Z

Codecov Report

Merging #3085 into develop will decrease coverage by 0.0%.
The diff coverage is n/a.

@@            Coverage Diff            @@
##           develop   #3085     +/-   ##
=========================================
- Coverage     85.6%   85.6%   -0.1%     
=========================================
  Files          122     122             
  Lines        10391   10391             
=========================================
- Hits          8905    8904      -1     
- Misses        1486    1487      +1

Flag	Coverage Δ
#clang	`76.0% <ø> (-0.2%)`	⬇️
#gcc	`86.4% <ø> (ø)`

Impacted Files	Coverage Δ
core/src/impl/Kokkos_ConcurrentBitset.hpp	`90.4% <0.0%> (-2.4%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 605ab03...65daf56. Read the comment docs.

masterleinad · 2020-06-09T19:10:50Z

For me, all tests pass locally using NVCC11.

jjwilke · 2020-06-09T22:50:02Z

bin/nvcc_wrapper

@@ -202,11 +202,11 @@ do
    cuda_args="$cuda_args $1"
    ;;
  #Handle unsupported standard flags
-  --std=c++1y|-std=c++1y|--std=c++1z|-std=c++1z|--std=gnu++1y|-std=gnu++1y|--std=gnu++1z|-std=gnu++1z|--std=c++2a|-std=c++2a|--std=c++17|-std=c++17)
+  --std=c++1y|-std=c++1y|--std=gnu++1y|-std=gnu++1y|--std=c++1z|-std=c++1z|--std=gnu++1z|-std=gnu++1z|--std=c++2a|-std=c++2a)


This isn't going to work for compilers that support C++17 with CUDA < 11. CMake is going to do a version test expecting to find C++17 support and will then fail the feature tests.

I think we are going to have to move c++17 into its own special case and make this version-dependent.

I tested with

CUDA 9.1.85: gcc-6.5.0 CUDA 10.2: gcc-6.5.0, gcc-7.5.0, gcc-8.4.0 CUDA 11.0.167: gcc-6.5.0, gcc-7.5.0, gcc-8.4.0, gcc-9.3.0

and they all configure correctly.

jjwilke · 2020-06-09T22:51:01Z

bin/nvcc_wrapper

@@ -224,7 +224,7 @@ do
    std_flag=$corrected_std_flag
    shared_args="$shared_args $std_flag"
  ;;
-  --std=c++11|-std=c++11|--std=c++14|-std=c++14)
+  --std=c++11|-std=c++11|--std=c++14|-std=c++14|--std=c++17|-std=c++17)


If you build with, e.g. CUDA 10 and GCC 8, CMake expects to have C++17 support but nvcc won't allow it. This undoes our trick of converting c++17 to c++14 for CMake feature tests.

Hmm... I don't have a problem with this combination

-- The CXX compiler identification is GNU 8.4.0 -- Check for working CXX compiler: /tmp/kokkos_new/bin/nvcc_wrapper -- Check for working CXX compiler: /tmp/kokkos_new/bin/nvcc_wrapper -- works -- Detecting CXX compiler ABI info -- Detecting CXX compiler ABI info - done -- Detecting CXX compile features -- Detecting CXX compile features - done -- The project name is: Kokkos -- Compiler Version: 10.2.89 -- SERIAL backend is being turned on to ensure there is at least one Host space. To change this, you must enable another host execution space and configure with -DKokkos_ENABLE_SERIAL=OFF or change CMakeCache.txt -- Using -std=c++14 for C++14 standard as feature -- Execution Spaces: -- Device Parallel: CUDA -- Host Parallel: NONE -- Host Serial: SERIAL -- -- Architectures: -- VOLTA70 -- Found CUDAToolkit: /tmp/cuda-10.2/include (found version "10.2.89") -- Looking for C++ include pthread.h -- Looking for C++ include pthread.h - found -- Looking for pthread_create -- Looking for pthread_create - found -- Found Threads: TRUE -- Found TPLCUDA: TRUE -- Found TPLLIBDL: /usr/lib/x86_64-linux-gnu/libdl.so -- Configuring done -- Generating done -- Build files have been written to: /tmp/kokkos_new/build_nvcc

Didn't we fix it in the cmake, where we do something special with the feature test stuff?

This is fixed in CMake 3.17 after they changed how feature detection works. This doesn't work in < 3.16, which is what we are stuck on.

I'm guessing we don't have a CMake < 3.16, GCC 8, CUDA 10 build in Jenkins/Travis

Hmm... I still can't reproduce any problem:

$ cmake --version cmake version 3.10.2 CMake suite maintained and supported by Kitware (kitware.com/cmake).

$ cmake -DKokkos_ENABLE_CUDA=ON -DKokkos_ARCH_VOLTA70=ON -DKokkos_CXX_STANDARD=14 .. -- The CXX compiler identification is GNU 8.4.0 -- Check for working CXX compiler: /tmp/kokkos_new/bin/nvcc_wrapper -- Check for working CXX compiler: /tmp/kokkos_new/bin/nvcc_wrapper -- works -- Detecting CXX compiler ABI info -- Detecting CXX compiler ABI info - done -- Detecting CXX compile features -- Detecting CXX compile features - done -- The project name is: Kokkos -- Compiler Version: 10.2.89 -- SERIAL backend is being turned on to ensure there is at least one Host space. To change this, you must enable another host execution space and configure with -DKokkos_ENABLE_SERIAL=OFF or change CMakeCache.txt -- Using -std=c++14 for C++14 standard as feature -- Execution Spaces: -- Device Parallel: CUDA -- Host Parallel: NONE -- Host Serial: SERIAL -- -- Architectures: -- VOLTA70 -- Found CUDAToolkit: /tmp/cuda-10.2/include (found version "10.2.89") -- Looking for C++ include pthread.h -- Looking for C++ include pthread.h - found -- Looking for pthread_create -- Looking for pthread_create - found -- Found Threads: TRUE -- Found TPLCUDA: TRUE -- Found TPLLIBDL: /usr/lib/x86_64-linux-gnu/libdl.so -- Configuring done -- Generating done -- Build files have been written to: /tmp/kokkos_new/build_nvcc

-- Compiler Version: 10.1.243 -- SERIAL backend is being turned on to ensure there is at least one Host space. To change this, you must enable another host execution space and configure with -DKokkos_ENABLE_SERIAL=OFF or change CMakeCache.txt -- Using -std=c++11 for C++11 standard as feature CMake Error at cmake/kokkos_test_cxx_std.cmake:47 (MESSAGE): Compiler NVIDIA should support cxx_std_11, but CMake reports feature not supported Call Stack (most recent call first): cmake/kokkos_test_cxx_std.cmake:63 (kokkos_set_cxx_standard_feature) cmake/kokkos_tribits.cmake:201 (INCLUDE) CMakeLists.txt:161 (KOKKOS_SETUP_BUILD_ENVIRONMENT)

Fascinating. Does not happen with 3.10. Does happen with 3.12.

Alright, I can confirm that it fails for 3.12, 3.13 and 3.14. I now pushed another case to nvcc_wrapper that handles -std=c++17 for NVCC version < 11 and version >= 11.

jjwilke

Looks good. Thanks for taking care of this.

Squash to single commit?

masterleinad · 2020-06-18T22:13:59Z

The CI failure is unrelated and fixed in #3114.
CI even hit the right machine for testing such that the changed CI configuration passed. Nevertheless, we need to wait with merging this until we can confirm that all machines have a compatible driver version.

Checking NVCC 11 with older compilers, I saw that CMake happily tries to use intermediate standard flags when requesting C++17 support (even though we disallow extensions and require the respective standard). For now, I just pushed a commit that checks if that's happening and abort configuring. That should not be a regression because it doesn't seem to happen for earlier standards.

masterleinad · 2020-06-18T22:15:02Z

I'll squash when people are happy with this (and the correct driver versions are installed everywhere).

masterleinad · 2020-06-19T04:49:33Z

Retest this please.

scripts/docker/Dockerfile.nvcc

dalg24 · 2020-06-22T14:21:41Z

Please cleanup history and I will merge

masterleinad · 2020-06-22T16:00:59Z

Please cleanup history and I will merge

Rebased, squashed and CI passing.

dalg24 requested a review from jjwilke June 9, 2020 19:21

jjwilke suggested changes Jun 9, 2020

View reviewed changes

jjwilke suggested changes Jun 18, 2020

View reviewed changes

masterleinad force-pushed the fix_compiling_nvcc11_c++17 branch 2 times, most recently from 89a82fc to e2165a5 Compare June 18, 2020 17:18

crtrott approved these changes Jun 18, 2020

View reviewed changes

masterleinad force-pushed the fix_compiling_nvcc11_c++17 branch 3 times, most recently from 6528089 to 79bfe90 Compare June 21, 2020 12:41

dalg24 reviewed Jun 22, 2020

View reviewed changes

scripts/docker/Dockerfile.nvcc Outdated Show resolved Hide resolved

dalg24 reviewed Jun 22, 2020

View reviewed changes

scripts/docker/Dockerfile.nvcc Outdated Show resolved Hide resolved

masterleinad added 5 commits June 22, 2020 10:41

Allow using C++17 with NVCC11

01f1633

Relax CUDA version detection

89fa2ab

Double check compatibility of version flags provided by CMake with NVCC

3270409

Workaround 'enclosing function must allow its address to be taken'

3f22fdc

Add CI for NVCC11 with C++17

0c7d8f1

masterleinad force-pushed the fix_compiling_nvcc11_c++17 branch from c7c2832 to 0c7d8f1 Compare June 22, 2020 14:46

dalg24 merged commit 639e0a2 into kokkos:develop Jun 22, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix compiling NVCC 11 with C++17 #3085

Fix compiling NVCC 11 with C++17 #3085

masterleinad commented Jun 9, 2020

codecov-commenter commented Jun 9, 2020 •

edited

masterleinad commented Jun 9, 2020

jjwilke Jun 9, 2020

masterleinad Jun 10, 2020

jjwilke Jun 9, 2020

masterleinad Jun 10, 2020

crtrott Jun 10, 2020

jjwilke Jun 17, 2020

jjwilke Jun 17, 2020

masterleinad Jun 17, 2020

jjwilke Jun 18, 2020

masterleinad Jun 18, 2020

jjwilke left a comment

masterleinad commented Jun 18, 2020

masterleinad commented Jun 18, 2020

masterleinad commented Jun 19, 2020

dalg24 commented Jun 22, 2020

masterleinad commented Jun 22, 2020

Fix compiling NVCC 11 with C++17 #3085

Fix compiling NVCC 11 with C++17 #3085

Conversation

masterleinad commented Jun 9, 2020

codecov-commenter commented Jun 9, 2020 • edited

Codecov Report

masterleinad commented Jun 9, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jjwilke left a comment

Choose a reason for hiding this comment

masterleinad commented Jun 18, 2020

masterleinad commented Jun 18, 2020

masterleinad commented Jun 19, 2020

dalg24 commented Jun 22, 2020

masterleinad commented Jun 22, 2020

codecov-commenter commented Jun 9, 2020 •

edited