Switch to C++17 #1466

bernhardmgruber · 2021-11-15T12:20:53Z

This PR switches to C++17, including:

Require C++17 in CMake
Remove nvcc version below 11 from GitHub CI
Remove CUDA SDK installer below 9.2 from install scripts
Update supported CUDA versions in README.md
Eliminate obsolete C++ version checks
Remove support for nvcc + clang-5
Update documentation
Fix warnings
Merge GitLab CI CUDA job description files
Add more GitLab CI tests with clang as CUDA compiler: clang-11/cuda11.0, clang-12/cuda11.0
Add GitHub CI test with clang-12 + nvcc 11.4
support passing CMAKE_CUDA_FLAGS variable through CI

j-stephan

Thanks! A few remarks:

cmake/alpakaCommon.cmake

.github/workflows/ci.yml

bernhardmgruber · 2021-11-15T15:14:07Z

It looks like we are going to need to drop clang-5 as well:

cd /home/runner/work/alpaka/alpaka/build/example/bufferCopy && /usr/local/cuda-11.0/bin/nvcc -forward-unknown-to-host-compiler -ccbin=/usr/bin/clang++ -DALPAKA_ACC_CPU_B_OMP2_T_SEQ_ENABLED -DALPAKA_ACC_CPU_B_SEQ_T_OMP2_ENABLED -DALPAKA_ACC_CPU_B_SEQ_T_SEQ_ENABLED -DALPAKA_ACC_CPU_B_SEQ_T_THREADS_ENABLED -DALPAKA_ACC_CPU_B_TBB_T_SEQ_ENABLED -DALPAKA_ACC_GPU_CUDA_ENABLED -DALPAKA_BLOCK_SHARED_DYN_MEMBER_ALLOC_KIB=47 -DALPAKA_CI -DALPAKA_DEBUG=0 -DALPAKA_DEBUG_OFFLOAD_ASSUME_HOST -DALPAKA_OFFLOAD_MAX_BLOCK_SIZE=256 -I/home/runner/work/alpaka/alpaka/include -isystem=/usr/local/cuda-11.0/targets/x86_64-linux/include -isystem=/home/runner/work/alpaka/alpaka/boost -isystem=/usr/local/cuda-11.0/include -g --generate-code=arch=compute_80,code=[compute_80,sm_80] --extended-lambda --expt-relaxed-constexpr -g -lineinfo -Xcompiler=-fopenmp -Xcudafe=--display_error_number -Xcudafe=--diag_suppress=esa_on_defaulted_function_ignored -Xcompiler -pthread -std=c++17 -MD -MT example/bufferCopy/CMakeFiles/bufferCopy.dir/src/bufferCopy.cpp.o -MF CMakeFiles/bufferCopy.dir/src/bufferCopy.cpp.o.d -x cu -c /home/runner/work/alpaka/alpaka/example/bufferCopy/src/bufferCopy.cpp -o CMakeFiles/bufferCopy.dir/src/bufferCopy.cpp.o
nvcc warning : The -std=c++17 flag is not supported with the configured host compiler. Flag will be ignored.
In file included from /home/runner/work/alpaka/alpaka/example/bufferCopy/src/bufferCopy.cpp:18:
In file included from /home/runner/work/alpaka/alpaka/include/alpaka/alpaka.hpp:18:
In file included from /home/runner/work/alpaka/alpaka/include/alpaka/acc/AccCpuOmp2Blocks.hpp:19:
In file included from /home/runner/work/alpaka/alpaka/include/alpaka/atomic/AtomicHierarchy.hpp:12:
In file included from /home/runner/work/alpaka/alpaka/include/alpaka/atomic/Traits.hpp:14:
In file included from /home/runner/work/alpaka/alpaka/include/alpaka/core/Concepts.hpp:12:
In file included from /usr/bin/../lib/gcc/x86_64-linux-gnu/7.5.0/../../../../include/c++/7.5.0/type_traits:35:
/usr/bin/../lib/gcc/x86_64-linux-gnu/7.5.0/../../../../include/c++/7.5.0/bits/c++0x_warning.h:32:2: error: This file requires compiler and library support for the ISO C++ 2011 standard. This support must be enabled with the -std=c++11 or -std=gnu++11 compiler options.
#error This file requires compiler and library support \

j-stephan · 2021-11-15T15:35:53Z

Weird, looks like nvcc doesn't know that clang 5 has C++17 support. Maybe we can add a -Xcompiler -std=c++17 here.

bernhardmgruber · 2021-11-15T16:02:26Z

Weird, looks like nvcc doesn't know that clang 5 has C++17 support. Maybe we can add a -Xcompiler -std=c++17 here.

We could. clang 5 should support it. But then there is no official way to query what host compiler is used via cmake. See also: https://gitlab.kitware.com/cmake/cmake/-/issues/20901.

psychocoderHPC · 2021-11-15T17:01:40Z

IMO clang5 should be dropped instead of adding workarounds.

j-stephan · 2021-11-15T17:03:51Z

IMO clang5 should be dropped instead of adding workarounds.

Should we keep it as a normal C++ compiler and just disable it for CUDA?

j-stephan · 2021-11-15T17:04:16Z

@bernhardmgruber Please also remember to update the support matrix in README.md.

bernhardmgruber · 2021-11-15T17:18:02Z

@bernhardmgruber Please also remember to update the support matrix in README.md.

Did I update the support matrix incorrectly?

j-stephan · 2021-11-15T17:19:19Z

Ah, no, you didn't. I just somehow skipped over it during my last review. My bad.

bernhardmgruber · 2021-11-15T17:31:59Z

Switching to C++17 does not fix compilation with nvcc 11.0/11.1 and MSVC. It seems we can therefore not fix #1331 with this PR.

j-stephan · 2021-11-15T17:39:00Z

The issue is different, though. The last time it complained about missing ( everywhere, now it is some template instantiation error. So I guess #1331 is technically fixed.

j-stephan · 2021-11-15T17:43:24Z

I'm happy to ditch CUDA 11.0 and 11.1, though. @psychocoderHPC, what do you think?

psychocoderHPC · 2021-11-15T18:17:11Z

I'm happy to ditch CUDA 11.0 and 11.1, though

Removing the support for CUDA 11.0/11.1 on Windows is IMO no problem.
For linux I would not remove these versions!

j-stephan · 2021-11-15T23:12:34Z

The first few lines of README.md (language badge and introductory text) also still refer to C++14.

bernhardmgruber · 2021-11-16T09:49:28Z

During the alpaka VC we decided to not add nvcc 11.0/11.1 + MSVC support again and also drop support for nvcc with clang-5 as host compiler (clang-5 stays supported without nvcc).

bernhardmgruber · 2021-11-17T10:36:53Z

It seems that with a minimum CUDA SDK of 11.0, we can no longer use clang < 12 as CUDA compiler.

I tried several workarounds locally suggested by @psychocoderHPC, passing --no-cuda-version-check and --cuda-gpu-arch=sm_50 via CMAKE_CUDA_FLAGS without success.

bernhardmgruber · 2021-11-18T09:55:49Z

I can reproduce the issue locally as well. Just run:
CXX=clang++-9 cmake -DCMAKE_CUDA_COMPILER=clang++-9 -DALPAKA_ACC_GPU_CUDA_ENABLE=ON '-DCMAKE_CUDA_ARCHITECTURES=50;70' -DCMAKE_CUDA_FLAGS=--no-cuda-version-check ..
With CUDA 11.0 or higher installed, it will error with:

CMake Error at /usr/share/cmake-3.20/Modules/CMakeDetermineCompilerId.cmake:676 (message):
  Compiling the CUDA compiler identification source file
  "CMakeCUDACompilerId.cu" failed.

  Compiler: /usr/bin/clang++-9

  Build flags:

  Id flags:
  --cuda-path=/usr/local/cuda;--cuda-gpu-arch=sm_50;--cuda-gpu-arch=sm_70 -v

  

  The output was:

  1

  clang version 9.0.1-16.1ubuntu1

  Target: x86_64-pc-linux-gnu

  Thread model: posix

  InstalledDir: /usr/bin

  Found candidate GCC installation: /usr/bin/../lib/gcc/x86_64-linux-gnu/10

  Found candidate GCC installation: /usr/bin/../lib/gcc/x86_64-linux-gnu/11

  Found candidate GCC installation: /usr/bin/../lib/gcc/x86_64-linux-gnu/9

  Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/10

  Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/11

  Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/9

  Selected GCC installation: /usr/bin/../lib/gcc/x86_64-linux-gnu/11

  Candidate multilib: .;@m64

  Selected multilib: .;@m64

  Found CUDA installation: /usr/local/cuda, version 7.0

  clang: error: cannot find libdevice for sm_50.  Provide path to different
  CUDA installation via --cuda-path, or pass -nocudalib to build without
  linking with libdevice.

  clang: error: cannot find libdevice for sm_70.  Provide path to different
  CUDA installation via --cuda-path, or pass -nocudalib to build without
  linking with libdevice.

  clang: error: GPU arch sm_70 is supported by CUDA versions between 9.0 and
  10.1 (inclusive), but installation at /usr/local/cuda is 7.0.  Use
  --cuda-path to specify a different CUDA install, pass a different GPU arch
  with --cuda-gpu-arch, or pass --no-cuda-version-check.

  

  

Call Stack (most recent call first):
  /usr/share/cmake-3.20/Modules/CMakeDetermineCompilerId.cmake:6 (CMAKE_DETERMINE_COMPILER_ID_BUILD)
  /usr/share/cmake-3.20/Modules/CMakeDetermineCompilerId.cmake:48 (__determine_compiler_id_test)
  /usr/share/cmake-3.20/Modules/CMakeDetermineCUDACompiler.cmake:278 (CMAKE_DETERMINE_COMPILER_ID)
  cmake/alpakaCommon.cmake:348 (enable_language)
  CMakeLists.txt:87 (include)


-- Configuring incomplete, errors occurred!

bernhardmgruber · 2021-11-18T12:48:51Z

It seems we can also not test clang-6 with nvcc 11 in the Gitlab CI, because libomp-6-dev is not available on either Ubuntu 18.04 or 20.04. I upgrade the job to clang-7 therefore.

bernhardmgruber · 2021-11-18T12:50:09Z

After feedback from @psychocoderHPC, I removed clang as CUDA compiler runs with clang < 11. See comment here: #1011 (comment).

bernhardmgruber · 2021-11-18T19:02:48Z

@psychocoderHPC suggested to me to keep CUDA < 11 for clang as CUDA compiler, since the objective is just to move to C++17. I therefore readded the runs with clang and older CUDA versions.

* Require C++17 in CMake * Remove nvcc version below 11 from GitHub CI * Remove CUDA SDK installer below 9.2 from install scripts * Update supported CUDA versions in README.md * Eliminate obsolete C++ version checks * Remove support for nvcc + clang-5 * Update documentation * Fix warnings * Merge GitLab CI CUDA job description files * Add a GitLab CI test with clang-12 as CUDA compiler and cuda11.0 * Add GitHub CI test with clang-12 + nvcc 11.4 * support passing CMAKE_CUDA_FLAGS variable through CI

bernhardmgruber · 2021-11-19T11:05:54Z

OK, CI passing, seems we are good! :)

j-stephan

Looking good, thanks! I'll remove the deleted CI jobs from the branch protection rules. Three questions:

.github/workflows/ci.yml

README.md

jkelling · 2021-11-19T11:21:53Z

Should todos that have been marked for the C++17 move be addressed in this PR?

There is at least this one: https://github.com/jkelling/alpaka/blob/develop/include/alpaka/rand/RandDefault.hpp#L135-L140

j-stephan · 2021-11-19T11:23:16Z

IMHO it shouldn't. This PR just enables C++17. We can (and should) do the cleanup of the code base in a follow up PR. This is already large enough as-is.

bernhardmgruber linked an issue Nov 15, 2021 that may be closed by this pull request

Considerations for C++17-only release #1011

Closed

j-stephan added this to the Version 0.9.0 (I/2022) milestone Nov 15, 2021

j-stephan added Type:Enhancement Type:Install installation & packaging Type:Testing labels Nov 15, 2021

j-stephan requested changes Nov 15, 2021

View reviewed changes

cmake/alpakaCommon.cmake Outdated Show resolved Hide resolved

.github/workflows/ci.yml Show resolved Hide resolved

.github/workflows/ci.yml Show resolved Hide resolved

bernhardmgruber force-pushed the cpp17 branch 2 times, most recently from 5618594 to cde7240 Compare November 15, 2021 13:01

j-stephan previously approved these changes Nov 15, 2021

View reviewed changes

j-stephan linked an issue Nov 16, 2021 that may be closed by this pull request

Windows CI broken for CUDA builds. #1331

Closed

bernhardmgruber dismissed j-stephan’s stale review via 55fed54 November 16, 2021 09:47

bernhardmgruber force-pushed the cpp17 branch from cde7240 to 55fed54 Compare November 16, 2021 09:47

bernhardmgruber force-pushed the cpp17 branch from fea6022 to 762c53b Compare November 17, 2021 09:54

bernhardmgruber changed the title ~~Switch to C++17~~ Switch to C++17 and CUDA 11+ Nov 17, 2021

bernhardmgruber mentioned this pull request Nov 18, 2021

Considerations for C++17-only release #1011

Closed

bernhardmgruber force-pushed the cpp17 branch from 762c53b to d2fd2cd Compare November 18, 2021 12:47

bernhardmgruber force-pushed the cpp17 branch 7 times, most recently from 4d881cd to 57ed7f8 Compare November 18, 2021 19:00

bernhardmgruber changed the title ~~Switch to C++17 and CUDA 11+~~ Switch to C++17 Nov 18, 2021

bernhardmgruber force-pushed the cpp17 branch from 57ed7f8 to 4467954 Compare November 19, 2021 08:56

j-stephan reviewed Nov 19, 2021

View reviewed changes

.github/workflows/ci.yml Show resolved Hide resolved

.github/workflows/ci.yml Show resolved Hide resolved

README.md Show resolved Hide resolved

This was referenced Nov 19, 2021

Add CI Jobs for CUDA 11.5 #1475

Closed

Add clang-13 to CI #1476

Closed

j-stephan approved these changes Nov 19, 2021

View reviewed changes

j-stephan merged commit 0ef767d into alpaka-group:develop Nov 19, 2021

bernhardmgruber deleted the cpp17 branch November 19, 2021 13:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Switch to C++17 #1466

Switch to C++17 #1466

bernhardmgruber commented Nov 15, 2021 •

edited

Loading

j-stephan left a comment

bernhardmgruber commented Nov 15, 2021

j-stephan commented Nov 15, 2021

bernhardmgruber commented Nov 15, 2021

psychocoderHPC commented Nov 15, 2021

j-stephan commented Nov 15, 2021

j-stephan commented Nov 15, 2021

bernhardmgruber commented Nov 15, 2021

j-stephan commented Nov 15, 2021

bernhardmgruber commented Nov 15, 2021

j-stephan commented Nov 15, 2021

j-stephan commented Nov 15, 2021

psychocoderHPC commented Nov 15, 2021

j-stephan commented Nov 15, 2021

bernhardmgruber commented Nov 16, 2021

bernhardmgruber commented Nov 17, 2021

bernhardmgruber commented Nov 18, 2021 •

edited

Loading

bernhardmgruber commented Nov 18, 2021 •

edited

Loading

bernhardmgruber commented Nov 18, 2021

bernhardmgruber commented Nov 18, 2021

bernhardmgruber commented Nov 19, 2021

j-stephan left a comment •

edited

Loading

jkelling commented Nov 19, 2021

j-stephan commented Nov 19, 2021

Switch to C++17 #1466

Switch to C++17 #1466

Conversation

bernhardmgruber commented Nov 15, 2021 • edited Loading

j-stephan left a comment

Choose a reason for hiding this comment

bernhardmgruber commented Nov 15, 2021

j-stephan commented Nov 15, 2021

bernhardmgruber commented Nov 15, 2021

psychocoderHPC commented Nov 15, 2021

j-stephan commented Nov 15, 2021

j-stephan commented Nov 15, 2021

bernhardmgruber commented Nov 15, 2021

j-stephan commented Nov 15, 2021

bernhardmgruber commented Nov 15, 2021

j-stephan commented Nov 15, 2021

j-stephan commented Nov 15, 2021

psychocoderHPC commented Nov 15, 2021

j-stephan commented Nov 15, 2021

bernhardmgruber commented Nov 16, 2021

bernhardmgruber commented Nov 17, 2021

bernhardmgruber commented Nov 18, 2021 • edited Loading

bernhardmgruber commented Nov 18, 2021 • edited Loading

bernhardmgruber commented Nov 18, 2021

bernhardmgruber commented Nov 18, 2021

bernhardmgruber commented Nov 19, 2021

j-stephan left a comment • edited Loading

Choose a reason for hiding this comment

jkelling commented Nov 19, 2021

j-stephan commented Nov 19, 2021

bernhardmgruber commented Nov 15, 2021 •

edited

Loading

bernhardmgruber commented Nov 18, 2021 •

edited

Loading

bernhardmgruber commented Nov 18, 2021 •

edited

Loading

j-stephan left a comment •

edited

Loading