Add more dialects to host only TU tests #346
Conversation
|
|
||
| set(cpp_std_versions 11 14 17 20) | ||
|
|
||
| set(cpp_11_exclusions) |
There was a problem hiding this comment.
As an example: set(cpp_11_exclusions "cuda/std/mdspan") will get excluded from C++11 builds where it is currently asserting on unsupported dialect.
| foreach(version IN LISTS cpp_std_versions) | ||
| string(REPLACE "/" "_" executable_name "${header_under_test}-${version}") | ||
| list(FIND cpp_${version}_exclusions ${header_under_test} header_excluded) | ||
| string(FIND ${CMAKE_CXX_COMPILER} "${cpp_${version}_exclusions}" compiler_excluded) |
There was a problem hiding this comment.
I don't think this going to do what you want.
if you had:
set(cpp_11_exclusions "cuda/std/mdspan" "g++")
It wouldn't stop when your compiler if g++ for the given reasons:
- You are using the full path of the compiler not the compiler id which isn't stable ( consider names like
x86_64-conda_cos6-linux-gnu-c++) - You are actually asking CMake to find the string `"cuda/std/mdspan;g++" in the compiler path which won't happen.
What you want to do is use the CMAKE_CXX_COMPILER_ID and use list(FIND to see if the compiler id is one of the elements in the cpp_${version}_exclusions list
There was a problem hiding this comment.
Oh thanks! I actually had dropped the idea since it wasn't working. I want to drop the compiler exclusions list for now as all the tests are passing without filtering anyway since I added the CXX_STANDARD_REQUIRED OFF property.
There was a problem hiding this comment.
I hacked together a solution using try_compile instead. It seems to do exactly what I'm looking for, but seems a little heavy handed.
There was a problem hiding this comment.
I am totally unprepared to say anything about the cmake changes @allisonvacanti maybe?
The product changes and switches to nvcc 12.0 seem fine.
That said, we should definitely have a policy for which nvcc version should also be smoke tested beyond that
| @@ -528,7 +528,7 @@ auto as_writable_bytes(span<_Tp, _Extent> __s) noexcept | |||
|
|
|||
| #endif // _LIBCUDACXX_STD_VER > 11 | |||
|
|
|||
| #if _LIBCUDACXX_STD_VER > 14 | |||
| #if _LIBCUDACXX_STD_VER > 14 && !defined(_LIBCUDACXX_HAS_NO_DEDUCTION_GUIDES) | |||
There was a problem hiding this comment.
@miscco GCC-6 strikes again. 🙃
There was a problem hiding this comment.
Yeah, I fixed that locally on my branch. That said, I thought we decided to settle on gcc-7 as the minimal gcc version, so we might as well just drop it completely
| try_compile( | ||
| dialect_supported | ||
| ${CMAKE_BINARY_DIR}/dialect_check_${version} "${CMAKE_CURRENT_SOURCE_DIR}/detect_dialect.cpp" | ||
| COMPILE_DEFINITIONS ${cxx_version} |
There was a problem hiding this comment.
I did try to use a CMake generator here switching on the compiler ID for MSVC, but it didn't seem to work in the subprocess. :(
There was a problem hiding this comment.
What is your end goal here? Is it to just verify that the given compiler supports a given standard level?
There was a problem hiding this comment.
The CMAKE_CXX_COMPILE_FEATURES variable will hold all the known language levels that the compiler supports.
So you can check if cxx_std_${cxx_version} is in that list.
There was a problem hiding this comment.
Would a dialect be included in that list if the compiler only has a subset of available features?
For example, GCC-6 supports 17, but not deduction guides and thus fails to compile some tests when CMake tries to use c++1z. There are also some things that fail on the compilers' side and are outside of our control.
There was a problem hiding this comment.
Yes it contains all dialects for a compiler which have a mode switch ( such as gcc-6 c++1z ).
I think it is better to contain a known list of bad configurations and exclude those compared to trying to build a test case that will smoke out all 'bad' compilers.
There was a problem hiding this comment.
This is something I wanted to use with NVCC as well which maintains its own compiler support matrix.
I'm not opposed to the changes I think you'd like to see, but a matrix of compiler, c++ dialect, and header files might be beyond my CMake-fu.
Can we make that a future PR?
There was a problem hiding this comment.
Sure a future PR works for me
|
This addresses NVIDIA/cccl#1006 |
TODO: Add TUs for NVCC as well.
I've added mechanisms for excluding headers that should not be compiled at a certain level... e.g. mdspan and possibly other friends.
This also bumps the version used for testing to CUDA 12.0.
docker compose --profile base -f "environments\linux\docker\compose.yml" buildwill now test every header in an isolated TU compiled with every C++ dialect. This is a convenient smoke test that libcu++ contributors should use before expecting their changes to get tested in CI.