From 1d2d9c18bd477a6fab1744d8da13bf5d760a0e03 Mon Sep 17 00:00:00 2001 From: Robert Maynard Date: Wed, 13 Dec 2017 11:14:55 -0500 Subject: cmMakefile: Refactor determining a targets C++ standard level The original code doesn't scale well as we add support for each new language level. Co-Author: Brad King --- Source/cmMakefile.cxx | 109 +++++++++++++++++--------------------------------- 1 file changed, 36 insertions(+), 73 deletions(-) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index a9de3f5..2aaff93 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -4401,8 +4401,10 @@ bool cmMakefile::HaveCxxStandardAvailable(cmTarget const* target, existingCxxStandard = defaultCxxStandard; } - if (std::find_if(cm::cbegin(CXX_STANDARDS), cm::cend(CXX_STANDARDS), - cmStrCmp(existingCxxStandard)) == cm::cend(CXX_STANDARDS)) { + const char* const* existingCxxLevel = + std::find_if(cm::cbegin(CXX_STANDARDS), cm::cend(CXX_STANDARDS), + cmStrCmp(existingCxxStandard)); + if (existingCxxLevel == cm::cend(CXX_STANDARDS)) { std::ostringstream e; e << "The CXX_STANDARD property on target \"" << target->GetName() << "\" contained an invalid value: \"" << existingCxxStandard << "\"."; @@ -4410,32 +4412,16 @@ bool cmMakefile::HaveCxxStandardAvailable(cmTarget const* target, return false; } - const char* const* existingCxxIt = existingCxxStandard - ? std::find_if(cm::cbegin(CXX_STANDARDS), cm::cend(CXX_STANDARDS), - cmStrCmp(existingCxxStandard)) - : cm::cend(CXX_STANDARDS); + /* clang-format off */ + const char* const* needCxxLevel = + needCxx17 ? &CXX_STANDARDS[3] + : needCxx14 ? &CXX_STANDARDS[2] + : needCxx11 ? &CXX_STANDARDS[1] + : needCxx98 ? &CXX_STANDARDS[0] + : nullptr; + /* clang-format on */ - if (needCxx17 && - existingCxxIt < std::find_if(cm::cbegin(CXX_STANDARDS), - cm::cend(CXX_STANDARDS), cmStrCmp("17"))) { - return false; - } - if (needCxx14 && - existingCxxIt < std::find_if(cm::cbegin(CXX_STANDARDS), - cm::cend(CXX_STANDARDS), cmStrCmp("14"))) { - return false; - } - if (needCxx11 && - existingCxxIt < std::find_if(cm::cbegin(CXX_STANDARDS), - cm::cend(CXX_STANDARDS), cmStrCmp("11"))) { - return false; - } - if (needCxx98 && - existingCxxIt < std::find_if(cm::cbegin(CXX_STANDARDS), - cm::cend(CXX_STANDARDS), cmStrCmp("98"))) { - return false; - } - return true; + return !needCxxLevel || needCxxLevel <= existingCxxLevel; } void cmMakefile::CheckNeededCxxLanguage(const std::string& feature, @@ -4481,10 +4467,12 @@ bool cmMakefile::AddRequiredTargetCxxFeature(cmTarget* target, needCxx17); const char* existingCxxStandard = target->GetProperty("CXX_STANDARD"); + const char* const* existingCxxLevel = nullptr; if (existingCxxStandard) { - if (std::find_if(cm::cbegin(CXX_STANDARDS), cm::cend(CXX_STANDARDS), - cmStrCmp(existingCxxStandard)) == - cm::cend(CXX_STANDARDS)) { + existingCxxLevel = + std::find_if(cm::cbegin(CXX_STANDARDS), cm::cend(CXX_STANDARDS), + cmStrCmp(existingCxxStandard)); + if (existingCxxLevel == cm::cend(CXX_STANDARDS)) { std::ostringstream e; e << "The CXX_STANDARD property on target \"" << target->GetName() << "\" contained an invalid value: \"" << existingCxxStandard << "\"."; @@ -4497,50 +4485,25 @@ bool cmMakefile::AddRequiredTargetCxxFeature(cmTarget* target, return false; } } - const char* const* existingCxxIt = existingCxxStandard - ? std::find_if(cm::cbegin(CXX_STANDARDS), cm::cend(CXX_STANDARDS), - cmStrCmp(existingCxxStandard)) - : cm::cend(CXX_STANDARDS); - - bool setCxx98 = needCxx98 && !existingCxxStandard; - bool setCxx11 = needCxx11 && !existingCxxStandard; - bool setCxx14 = needCxx14 && !existingCxxStandard; - bool setCxx17 = needCxx17 && !existingCxxStandard; - - if (needCxx17 && existingCxxStandard && - existingCxxIt < std::find_if(cm::cbegin(CXX_STANDARDS), - cm::cend(CXX_STANDARDS), cmStrCmp("17"))) { - setCxx17 = true; - } else if (needCxx14 && existingCxxStandard && - existingCxxIt < std::find_if(cm::cbegin(CXX_STANDARDS), - cm::cend(CXX_STANDARDS), - cmStrCmp("14"))) { - setCxx14 = true; - } else if (needCxx11 && existingCxxStandard && - existingCxxIt < std::find_if(cm::cbegin(CXX_STANDARDS), - cm::cend(CXX_STANDARDS), - cmStrCmp("11"))) { - setCxx11 = true; - } else if (needCxx98 && existingCxxStandard && - existingCxxIt < std::find_if(cm::cbegin(CXX_STANDARDS), - cm::cend(CXX_STANDARDS), - cmStrCmp("98"))) { - setCxx98 = true; - } - - if (setCxx17) { - target->SetProperty("CXX_STANDARD", "17"); - target->SetProperty("CUDA_STANDARD", "17"); - } else if (setCxx14) { - target->SetProperty("CXX_STANDARD", "14"); - target->SetProperty("CUDA_STANDARD", "14"); - } else if (setCxx11) { - target->SetProperty("CXX_STANDARD", "11"); - target->SetProperty("CUDA_STANDARD", "11"); - } else if (setCxx98) { - target->SetProperty("CXX_STANDARD", "98"); - target->SetProperty("CUDA_STANDARD", "98"); + + /* clang-format off */ + const char* const* needCxxLevel = + needCxx17 ? &CXX_STANDARDS[3] + : needCxx14 ? &CXX_STANDARDS[2] + : needCxx11 ? &CXX_STANDARDS[1] + : needCxx98 ? &CXX_STANDARDS[0] + : nullptr; + /* clang-format on */ + + if (needCxxLevel) { + // Ensure the C++ language level is high enough to support + // the needed C++ features. + if (!existingCxxLevel || existingCxxLevel < needCxxLevel) { + target->SetProperty("CXX_STANDARD", *needCxxLevel); + target->SetProperty("CUDA_STANDARD", *needCxxLevel); + } } + return true; } -- cgit v0.12 From 4b7618d17028da30bf0421e56aed6789bba4a12a Mon Sep 17 00:00:00 2001 From: Robert Maynard Date: Wed, 13 Dec 2017 11:15:06 -0500 Subject: CUDA: Fix CUDA_STANDARD selection via cxx_std_11 with CXX_STANDARD When C++ features require a certain C++/CUDA level, verify or update the standard level target property for each language independently. While at it, add missing rejection of invalid `CUDA_STANDARD` property values. Co-Author: Brad King Fixes: #17519 --- Source/cmMakefile.cxx | 26 ++++++++++++++++++++++++++ Tests/Cuda/CMakeLists.txt | 1 + Tests/Cuda/MixedStandardLevels/CMakeLists.txt | 14 ++++++++++++++ Tests/Cuda/MixedStandardLevels/main.cu | 10 ++++++++++ 4 files changed, 51 insertions(+) create mode 100644 Tests/Cuda/MixedStandardLevels/CMakeLists.txt create mode 100644 Tests/Cuda/MixedStandardLevels/main.cu diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 2aaff93..a1e2f63 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -4486,6 +4486,27 @@ bool cmMakefile::AddRequiredTargetCxxFeature(cmTarget* target, } } + const char* existingCudaStandard = target->GetProperty("CUDA_STANDARD"); + const char* const* existingCudaLevel = nullptr; + if (existingCudaStandard) { + existingCudaLevel = + std::find_if(cm::cbegin(CXX_STANDARDS), cm::cend(CXX_STANDARDS), + cmStrCmp(existingCudaStandard)); + if (existingCudaLevel == cm::cend(CXX_STANDARDS)) { + std::ostringstream e; + e << "The CUDA_STANDARD property on target \"" << target->GetName() + << "\" contained an invalid value: \"" << existingCudaStandard + << "\"."; + if (error) { + *error = e.str(); + } else { + this->GetCMakeInstance()->IssueMessage(cmake::FATAL_ERROR, e.str(), + this->Backtrace); + } + return false; + } + } + /* clang-format off */ const char* const* needCxxLevel = needCxx17 ? &CXX_STANDARDS[3] @@ -4500,6 +4521,11 @@ bool cmMakefile::AddRequiredTargetCxxFeature(cmTarget* target, // the needed C++ features. if (!existingCxxLevel || existingCxxLevel < needCxxLevel) { target->SetProperty("CXX_STANDARD", *needCxxLevel); + } + + // Ensure the CUDA language level is high enough to support + // the needed C++ features. + if (!existingCudaLevel || existingCudaLevel < needCxxLevel) { target->SetProperty("CUDA_STANDARD", *needCxxLevel); } } diff --git a/Tests/Cuda/CMakeLists.txt b/Tests/Cuda/CMakeLists.txt index de48501..8a43df5 100644 --- a/Tests/Cuda/CMakeLists.txt +++ b/Tests/Cuda/CMakeLists.txt @@ -2,6 +2,7 @@ ADD_TEST_MACRO(Cuda.Complex CudaComplex) ADD_TEST_MACRO(Cuda.ConsumeCompileFeatures CudaConsumeCompileFeatures) ADD_TEST_MACRO(Cuda.ObjectLibrary CudaObjectLibrary) +ADD_TEST_MACRO(Cuda.MixedStandardLevels MixedStandardLevels) ADD_TEST_MACRO(Cuda.ToolkitInclude CudaToolkitInclude) ADD_TEST_MACRO(Cuda.ProperLinkFlags ProperLinkFlags) ADD_TEST_MACRO(Cuda.WithC CudaWithC) diff --git a/Tests/Cuda/MixedStandardLevels/CMakeLists.txt b/Tests/Cuda/MixedStandardLevels/CMakeLists.txt new file mode 100644 index 0000000..683abe7 --- /dev/null +++ b/Tests/Cuda/MixedStandardLevels/CMakeLists.txt @@ -0,0 +1,14 @@ +cmake_minimum_required(VERSION 3.7) +project(CudaComplex CXX CUDA) + +string(APPEND CMAKE_CUDA_FLAGS " -gencode arch=compute_30,code=compute_30") + +set(CMAKE_CXX_STANDARD 11) + +add_executable(MixedStandardLevels main.cu) +target_compile_features(MixedStandardLevels PUBLIC cxx_std_11) + +if(APPLE) + # Help the static cuda runtime find the driver (libcuda.dyllib) at runtime. + set_property(TARGET MixedStandardLevels PROPERTY BUILD_RPATH ${CMAKE_CUDA_IMPLICIT_LINK_DIRECTORIES}) +endif() diff --git a/Tests/Cuda/MixedStandardLevels/main.cu b/Tests/Cuda/MixedStandardLevels/main.cu new file mode 100644 index 0000000..d57c05a --- /dev/null +++ b/Tests/Cuda/MixedStandardLevels/main.cu @@ -0,0 +1,10 @@ + +#include + +int main(int argc, char** argv) +{ + // Verify that issue #17519 Setting CXX_STANDARD breaks CUDA_STANDARD + // selection via cxx_std_11 has been corrected + using returnv = std::integral_constant; + return returnv::value; +} -- cgit v0.12