[Openmp-commits] [openmp] d187cee - [OpenMP] Use CMAKE_CXX_STANDARD for setting the C++ version

[Martin Storsjö via Openmp-commits]

Author: Martin Storsjö
Date: 2023-05-16T10:43:38+03:00
New Revision: d187ceee3b400b8b235630c6cddf64bf517620c7

URL: https://github.com/llvm/llvm-project/commit/d187ceee3b400b8b235630c6cddf64bf517620c7
DIFF: https://github.com/llvm/llvm-project/commit/d187ceee3b400b8b235630c6cddf64bf517620c7.diff

LOG: [OpenMP] Use CMAKE_CXX_STANDARD for setting the C++ version

Previously, we tried to check whether the -std=c++17 option was
supported and manually add the flag. That doesn't work for compilers
that do support C++17 but use a different option syntax, like
clang-cl.

OpenMP itself probably doesn't specifically require C++17, therefore
CXX_STANDARD_REQUIRED is left off, but in some cases, we may
have code that only works in C++17 mode.

In particular, 46262cab24312c71717ca70a9d0700481aa59152 made a
refactoring that works when built with Clang in C++17 mode, but not
in C++14 mode. MSVC accepts the construct in both language modes.

For libomptarget, we've had specific checks that require C++17
(or the -std=c++17 option) to be supported. It's doubtful that
libomptarget has got any code which more specifically requires C++17;
this seems to be a remnant from when libomptarget was added
originally in 2467df6e4f04e3d0e8e78d662473ba1b87c0a885 / D14031.
At that point, the rest of OpenMP didn't require C++11, while
libomptarget did require it. Now, it's unlikely that anyone attempts
building it with a toolchain that doesn't support C++11.

At this point, we could also probably just set CXX_STANDARD_REQUIRED
to true, requiring C++17 as baseline for all the OpenMP libraries.

This fixes building OpenMP with clang-cl after
46262cab24312c71717ca70a9d0700481aa59152.

Differential Revision: https://reviews.llvm.org/D149726

Added: 
    

Modified: 
    openmp/CMakeLists.txt
    openmp/cmake/HandleOpenMPOptions.cmake
    openmp/cmake/config-ix.cmake

Removed: 
    


################################################################################
diff  --git a/openmp/CMakeLists.txt b/openmp/CMakeLists.txt
index 50246b663289a..9f989266d6ae6 100644
--- a/openmp/CMakeLists.txt
+++ b/openmp/CMakeLists.txt
@@ -37,6 +37,10 @@ if (OPENMP_STANDALONE_BUILD)
   set(OPENMP_TEST_CXX_COMPILER ${CMAKE_CXX_COMPILER} CACHE STRING
     "C++ compiler to use for testing OpenMP runtime libraries.")
   set(OPENMP_LLVM_TOOLS_DIR "" CACHE PATH "Path to LLVM tools for testing.")
+
+  set(CMAKE_CXX_STANDARD 17 CACHE STRING "C++ standard to conform to")
+  set(CMAKE_CXX_STANDARD_REQUIRED NO)
+  set(CMAKE_CXX_EXTENSIONS NO)
 else()
   set(OPENMP_ENABLE_WERROR ${LLVM_ENABLE_WERROR})
   # If building in tree, we honor the same install suffix LLVM uses.
@@ -49,6 +53,12 @@ else()
     set(OPENMP_TEST_C_COMPILER ${LLVM_RUNTIME_OUTPUT_INTDIR}/clang.exe)
     set(OPENMP_TEST_CXX_COMPILER ${LLVM_RUNTIME_OUTPUT_INTDIR}/clang++.exe)
   endif()
+
+  # If not standalone, set CMAKE_CXX_STANDARD but don't set the global cache value,
+  # only set it locally for OpenMP.
+  set(CMAKE_CXX_STANDARD 17)
+  set(CMAKE_CXX_STANDARD_REQUIRED NO)
+  set(CMAKE_CXX_EXTENSIONS NO)
 endif()
 
 # Check and set up common compiler flags.
@@ -68,7 +78,7 @@ set(ENABLE_LIBOMPTARGET ON)
 # Since the device plugins are only supported on Linux anyway,
 # there is no point in trying to compile libomptarget on other OSes.
 # 32-bit systems are not supported either.
-if (APPLE OR WIN32 OR NOT OPENMP_HAVE_STD_CPP17_FLAG OR NOT CMAKE_SIZEOF_VOID_P EQUAL 8)
+if (APPLE OR WIN32 OR NOT "cxx_std_17" IN_LIST CMAKE_CXX_COMPILE_FEATURES OR NOT CMAKE_SIZEOF_VOID_P EQUAL 8)
   set(ENABLE_LIBOMPTARGET OFF)
 endif()
 
@@ -92,7 +102,7 @@ if (OPENMP_ENABLE_LIBOMPTARGET)
   # Check that the library can actually be built.
   if (APPLE OR WIN32)
     message(FATAL_ERROR "libomptarget cannot be built on Windows and MacOS X!")
-  elseif (NOT OPENMP_HAVE_STD_CPP17_FLAG)
+  elseif (NOT "cxx_std_17" IN_LIST CMAKE_CXX_COMPILE_FEATURES)
     message(FATAL_ERROR "Host compiler must support C++17 to build libomptarget!")
   elseif (NOT CMAKE_SIZEOF_VOID_P EQUAL 8)
     message(FATAL_ERROR "libomptarget on 32-bit systems are not supported!")

diff  --git a/openmp/cmake/HandleOpenMPOptions.cmake b/openmp/cmake/HandleOpenMPOptions.cmake
index 53a2d053e4245..4d6782a6f0cbe 100644
--- a/openmp/cmake/HandleOpenMPOptions.cmake
+++ b/openmp/cmake/HandleOpenMPOptions.cmake
@@ -35,5 +35,3 @@ append_if(OPENMP_HAVE_WENUM_CONSTEXPR_CONVERSION_FLAG "-Wno-enum-constexpr-conve
 append_if(OPENMP_HAVE_WEXTRA_FLAG "-Wno-extra" CMAKE_C_FLAGS CMAKE_CXX_FLAGS)
 append_if(OPENMP_HAVE_WPEDANTIC_FLAG "-Wno-pedantic" CMAKE_C_FLAGS CMAKE_CXX_FLAGS)
 append_if(OPENMP_HAVE_WMAYBE_UNINITIALIZED_FLAG "-Wno-maybe-uninitialized" CMAKE_C_FLAGS CMAKE_CXX_FLAGS)
-
-append_if(OPENMP_HAVE_STD_CPP17_FLAG "-std=c++17" CMAKE_CXX_FLAGS)

diff  --git a/openmp/cmake/config-ix.cmake b/openmp/cmake/config-ix.cmake
index 857cfe5da8d61..49e374286ac78 100644
--- a/openmp/cmake/config-ix.cmake
+++ b/openmp/cmake/config-ix.cmake
@@ -36,5 +36,3 @@ check_cxx_compiler_flag(-Wenum-constexpr-conversion OPENMP_HAVE_WENUM_CONSTEXPR_
 check_cxx_compiler_flag(-Wextra OPENMP_HAVE_WEXTRA_FLAG)
 check_cxx_compiler_flag(-Wpedantic OPENMP_HAVE_WPEDANTIC_FLAG)
 check_cxx_compiler_flag(-Wmaybe-uninitialized OPENMP_HAVE_WMAYBE_UNINITIALIZED_FLAG)
-
-check_cxx_compiler_flag(-std=c++17 OPENMP_HAVE_STD_CPP17_FLAG)