[Openmp-commits] [PATCH] D149726: [OpenMP] Use CMAKE_CXX_STANDARD for setting the C++ version

[Martin Storsjö via Phabricator via Openmp-commits]

mstorsjo created this revision.
mstorsjo added reviewers: hans, MaskRay, tianshilei1992, AndreyChurbanov.
Herald added subscribers: sunshaoce, guansong, yaxunl.
Herald added a project: All.
mstorsjo requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: jplehr, sstefan1.
Herald added a project: OpenMP.

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 <https://reviews.llvm.org/rG46262cab24312c71717ca70a9d0700481aa59152> 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 <https://reviews.llvm.org/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 <https://reviews.llvm.org/rG46262cab24312c71717ca70a9d0700481aa59152>.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D149726

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


Index: openmp/cmake/config-ix.cmake
===================================================================
--- openmp/cmake/config-ix.cmake
+++ openmp/cmake/config-ix.cmake
@@ -36,5 +36,3 @@
 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)
Index: openmp/cmake/HandleOpenMPOptions.cmake
===================================================================
--- openmp/cmake/HandleOpenMPOptions.cmake
+++ openmp/cmake/HandleOpenMPOptions.cmake
@@ -35,5 +35,3 @@
 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)
Index: openmp/CMakeLists.txt
===================================================================
--- openmp/CMakeLists.txt
+++ openmp/CMakeLists.txt
@@ -44,6 +44,9 @@
   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)
 else()
   set(OPENMP_ENABLE_WERROR ${LLVM_ENABLE_WERROR})
   # If building in tree, we honor the same install suffix LLVM uses.
@@ -56,6 +59,11 @@
     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)
 endif()
 
 # Check and set up common compiler flags.
@@ -75,7 +83,7 @@
 # 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 CMAKE_SIZEOF_VOID_P EQUAL 8)
   set(ENABLE_LIBOMPTARGET OFF)
 endif()
 
@@ -99,8 +107,6 @@
   # 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)
-    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!")
   endif()


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D149726.518988.patch
Type: text/x-patch
Size: 2887 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/openmp-commits/attachments/20230503/de12b9be/attachment-0001.bin>