[libc++] Link against libatomic when it is found

Before this patch, we tried detecting whether small atomics were available
without linking against libatomic. However, that's not really what we want
to know -- instead, we want to know what's required in order to support
atomics fully, which is to link against libatomic when it's provided.

That is both much simpler, and it doesn't suffer the problem that we would
not link against libatomic when small atomics didn't require it, which
lead to non-lockfree atomics never working.

Furthermore, because we understand that some platforms might not want to
(or be able to) ship non-lockfree atomics, we add that notion to the test
suite, independently of a potential extern library.

After this patch, we therefore:
(1) Link against libatomic when it is provided
(2) Independently detect whether non-lockfree atomics are supported in
    the test suite, regardless of whether that means we're linking against
    an external library or not (which is an implementation detail).

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

diff --git a/libcxx/CMakeLists.txt b/libcxx/CMakeLists.txt
index 1a7e0a0..4eebfb8 100644 (file)
--- a/libcxx/CMakeLists.txt
+++ b/libcxx/CMakeLists.txt
@@ -762,7 +762,7 @@ function(cxx_link_system_libraries target)
     target_link_libraries(${target} PRIVATE gcc_s)
   endif()
 
-  if (LIBCXX_HAVE_CXX_ATOMICS_WITH_LIB)
+  if (LIBCXX_HAS_ATOMIC_LIB)
     target_link_libraries(${target} PRIVATE atomic)
   endif()
 

diff --git a/libcxx/cmake/Modules/CheckLibcxxAtomic.cmake b/libcxx/cmake/Modules/CheckLibcxxAtomic.cmake
deleted file mode 100644 (file)
index 7fe5a62..0000000
--- a/libcxx/cmake/Modules/CheckLibcxxAtomic.cmake
+++ /dev/null
@@ -1,56 +0,0 @@
-INCLUDE(CheckCXXSourceCompiles)
-
-# Sometimes linking against libatomic is required for atomic ops, if
-# the platform doesn't support lock-free atomics.
-#
-# We could modify LLVM's CheckAtomic module and have it check for 64-bit
-# atomics instead. However, we would like to avoid careless uses of 64-bit
-# atomics inside LLVM over time on 32-bit platforms.
-
-function(check_cxx_atomics varname)
-  set(OLD_CMAKE_REQUIRED_FLAGS ${CMAKE_REQUIRED_FLAGS})
-  set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -nodefaultlibs -std=c++11 -nostdinc++ -isystem ${LIBCXX_SOURCE_DIR}/include")
-  if (${LIBCXX_GCC_TOOLCHAIN})
-    set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} --gcc-toolchain=${LIBCXX_GCC_TOOLCHAIN}")
-  endif()
-  if (CMAKE_C_FLAGS MATCHES -fsanitize OR CMAKE_CXX_FLAGS MATCHES -fsanitize)
-    set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -fno-sanitize=all")
-  endif()
-  if (CMAKE_C_FLAGS MATCHES -fsanitize-coverage OR CMAKE_CXX_FLAGS MATCHES -fsanitize-coverage)
-    set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -fno-sanitize-coverage=edge,trace-cmp,indirect-calls,8bit-counters")
-  endif()
-  check_cxx_source_compiles("
-#include <cstdint>
-#include <atomic>
-std::atomic<uintptr_t> x;
-std::atomic<uintmax_t> y;
-int main(int, char**) {
-  return x + y;
-}
-" ${varname})
-  set(CMAKE_REQUIRED_FLAGS ${OLD_CMAKE_REQUIRED_FLAGS})
-endfunction(check_cxx_atomics)
-
-# Perform the check for 64bit atomics without libatomic. It may have been
-# added to the required libraries during in the configuration of LLVM, which
-# would cause the check for CXX atomics without libatomic to incorrectly pass.
-if (CMAKE_REQUIRED_LIBRARIES)
-  set(OLD_CMAKE_REQUIRED_LIBRARIES ${CMAKE_REQUIRED_LIBRARIES})
-  list(REMOVE_ITEM CMAKE_REQUIRED_LIBRARIES "atomic")
-  check_cxx_atomics(LIBCXX_HAVE_CXX_ATOMICS_WITHOUT_LIB)
-  set(CMAKE_REQUIRED_LIBRARIES ${OLD_CMAKE_REQUIRED_LIBRARIES})
-endif()
-
-check_library_exists(atomic __atomic_fetch_add_8 "" LIBCXX_HAS_ATOMIC_LIB)
-# If not, check if the library exists, and atomics work with it.
-if(NOT LIBCXX_HAVE_CXX_ATOMICS_WITHOUT_LIB)
-  if(LIBCXX_HAS_ATOMIC_LIB)
-    list(APPEND CMAKE_REQUIRED_LIBRARIES "atomic")
-    check_cxx_atomics(LIBCXX_HAVE_CXX_ATOMICS_WITH_LIB)
-    if (NOT LIBCXX_HAVE_CXX_ATOMICS_WITH_LIB)
-      message(WARNING "Host compiler must support std::atomic!")
-    endif()
-  else()
-    message(WARNING "Host compiler appears to require libatomic, but cannot find it.")
-  endif()
-endif()

diff --git a/libcxx/cmake/config-ix.cmake b/libcxx/cmake/config-ix.cmake
index cbb4dfd..894f637 100644 (file)
--- a/libcxx/cmake/config-ix.cmake
+++ b/libcxx/cmake/config-ix.cmake
@@ -78,10 +78,6 @@ int main() { return 0; }
   cmake_pop_check_state()
 endif()
 
-if(NOT WIN32 OR MINGW)
-  include(CheckLibcxxAtomic)
-endif()
-
 # Check libraries
 if(WIN32 AND NOT MINGW)
   # TODO(compnerd) do we want to support an emulation layer that allows for the
@@ -90,19 +86,23 @@ if(WIN32 AND NOT MINGW)
   set(LIBCXX_HAS_M_LIB NO)
   set(LIBCXX_HAS_RT_LIB NO)
   set(LIBCXX_HAS_SYSTEM_LIB NO)
+  set(LIBCXX_HAS_ATOMIC_LIB NO)
 elseif(APPLE)
   check_library_exists(System write "" LIBCXX_HAS_SYSTEM_LIB)
   set(LIBCXX_HAS_PTHREAD_LIB NO)
   set(LIBCXX_HAS_M_LIB NO)
   set(LIBCXX_HAS_RT_LIB NO)
+  set(LIBCXX_HAS_ATOMIC_LIB NO)
 elseif(FUCHSIA)
   set(LIBCXX_HAS_M_LIB NO)
   set(LIBCXX_HAS_PTHREAD_LIB NO)
   set(LIBCXX_HAS_RT_LIB NO)
   set(LIBCXX_HAS_SYSTEM_LIB NO)
+  check_library_exists(atomic __atomic_fetch_add_8 "" LIBCXX_HAS_ATOMIC_LIB)
 else()
   check_library_exists(pthread pthread_create "" LIBCXX_HAS_PTHREAD_LIB)
   check_library_exists(m ccos "" LIBCXX_HAS_M_LIB)
   check_library_exists(rt clock_gettime "" LIBCXX_HAS_RT_LIB)
   set(LIBCXX_HAS_SYSTEM_LIB NO)
+  check_library_exists(atomic __atomic_fetch_add_8 "" LIBCXX_HAS_ATOMIC_LIB)
 endif()

diff --git a/libcxx/test/libcxx/atomics/atomics.align/align.pass.sh.cpp b/libcxx/test/libcxx/atomics/atomics.align/align.pass.pass.cpp
similarity index 97%
rename from libcxx/test/libcxx/atomics/atomics.align/align.pass.sh.cpp
rename to libcxx/test/libcxx/atomics/atomics.align/align.pass.pass.cpp
index 604473d..ebe8fc8 100644 (file)
--- a/libcxx/test/libcxx/atomics/atomics.align/align.pass.sh.cpp
+++ b/libcxx/test/libcxx/atomics/atomics.align/align.pass.pass.cpp
@@ -7,11 +7,8 @@
 //===----------------------------------------------------------------------===//
 //
 // UNSUPPORTED: libcpp-has-no-threads, c++03
-// REQUIRES: libatomic
-// FILE_DEPENDENCIES: %t.exe
-// RUN: %{build} -latomic
-// RUN: %{run}
-//
+// REQUIRES: non-lockfree-atomics
+
 // GCC currently fails because it needs -fabi-version=6 to fix mangling of
 // std::atomic when used with __attribute__((vector(X))).
 // XFAIL: gcc

diff --git a/libcxx/test/libcxx/selftest/dsl/dsl.sh.py b/libcxx/test/libcxx/selftest/dsl/dsl.sh.py
index c8f9984..9cf0ac9 100644 (file)
--- a/libcxx/test/libcxx/selftest/dsl/dsl.sh.py
+++ b/libcxx/test/libcxx/selftest/dsl/dsl.sh.py
@@ -97,6 +97,24 @@ class TestHasCompileFlag(SetupConfigs):
         self.assertTrue(dsl.hasCompileFlag(self.config, '-O1 -Dhello'))
 
 
+class TestSourceBuilds(SetupConfigs):
+    """
+    Tests for libcxx.test.dsl.sourceBuilds
+    """
+    def test_valid_program_builds(self):
+        source = """int main(int, char**) { }"""
+        self.assertTrue(dsl.sourceBuilds(self.config, source))
+
+    def test_compilation_error_fails(self):
+        source = """in main(int, char**) { }"""
+        self.assertFalse(dsl.sourceBuilds(self.config, source))
+
+    def test_link_error_fails(self):
+        source = """extern void this_isnt_defined_anywhere();
+                    int main(int, char**) { this_isnt_defined_anywhere(); }"""
+        self.assertFalse(dsl.sourceBuilds(self.config, source))
+
+
 class TestHasLocale(SetupConfigs):
     """
     Tests for libcxx.test.dsl.hasLocale

diff --git a/libcxx/test/lit.site.cfg.in b/libcxx/test/lit.site.cfg.in
index 7e1cfe5..f00a255 100644 (file)
--- a/libcxx/test/lit.site.cfg.in
+++ b/libcxx/test/lit.site.cfg.in
@@ -29,7 +29,6 @@ config.executor                 = "@LIBCXX_EXECUTOR@"
 config.llvm_unwinder            = @LIBCXXABI_USE_LLVM_UNWINDER@
 config.builtins_library         = "@LIBCXX_BUILTINS_LIBRARY@"
 config.has_libatomic            = @LIBCXX_HAS_ATOMIC_LIB@
-config.use_libatomic            = @LIBCXX_HAVE_CXX_ATOMICS_WITH_LIB@
 config.debug_build              = @LIBCXX_DEBUG_BUILD@
 config.libcxxabi_shared         = @LIBCXX_LINK_TESTS_WITH_SHARED_LIBCXXABI@
 config.cxx_ext_threads          = @LIBCXX_BUILD_EXTERNAL_THREAD_LIBRARY@

diff --git a/libcxx/utils/libcxx/test/config.py b/libcxx/utils/libcxx/test/config.py
index be8ee3b..11f2cb1 100644 (file)
--- a/libcxx/utils/libcxx/test/config.py
+++ b/libcxx/utils/libcxx/test/config.py
@@ -325,9 +325,6 @@ class Configuration(object):
         if not self.get_lit_bool('enable_filesystem', default=True):
             self.config.available_features.add('c++filesystem-disabled')
 
-        if self.get_lit_bool('has_libatomic', False):
-            self.config.available_features.add('libatomic')
-
         if self.target_info.is_windows():
             self.config.available_features.add('windows')
             if self.cxx_stdlib_under_test == 'libc++':

diff --git a/libcxx/utils/libcxx/test/dsl.py b/libcxx/utils/libcxx/test/dsl.py
index 1110ded..0e21256 100644 (file)
--- a/libcxx/utils/libcxx/test/dsl.py
+++ b/libcxx/utils/libcxx/test/dsl.py
@@ -56,6 +56,26 @@ def _makeConfigTest(config):
     def __exit__(self, *args): os.remove(tmp.name)
   return TestWrapper(suite, pathInSuite, config)
 
+def sourceBuilds(config, source):
+  """
+  Return whether the program in the given string builds successfully.
+
+  This is done by compiling and linking a program that consists of the given
+  source with the %{cxx} substitution, and seeing whether that succeeds.
+  """
+  with _makeConfigTest(config) as test:
+    with open(test.getSourcePath(), 'w') as sourceFile:
+      sourceFile.write(source)
+    commands = [
+      "mkdir -p %T",
+      "%{cxx} -xc++ %s %{flags} %{compile_flags} %{link_flags} -o %t.exe"
+    ]
+    commands = libcxx.test.newformat.parseScript(test, preamble=commands, fileDependencies=['%t.exe'])
+    out, err, exitCode, timeoutInfo = _executeScriptInternal(test, commands)
+    cleanup = libcxx.test.newformat.parseScript(test, preamble=['rm %t.exe'], fileDependencies=[])
+    _executeScriptInternal(test, cleanup)
+    return exitCode == 0
+
 def hasCompileFlag(config, flag):
   """
   Return whether the compiler in the configuration supports a given compiler flag.

diff --git a/libcxx/utils/libcxx/test/features.py b/libcxx/utils/libcxx/test/features.py
index b6c2ba7..6d30a09 100644 (file)
--- a/libcxx/utils/libcxx/test/features.py
+++ b/libcxx/utils/libcxx/test/features.py
@@ -32,6 +32,12 @@ features = [
   Feature(name='objective-c++',                 when=lambda cfg: hasCompileFlag(cfg, '-xobjective-c++ -fobjc-arc')),
   Feature(name='diagnose-if-support',           when=lambda cfg: hasCompileFlag(cfg, '-Wuser-defined-warnings'), compileFlag='-Wuser-defined-warnings'),
   Feature(name='modules-support',               when=lambda cfg: hasCompileFlag(cfg, '-fmodules')),
+  Feature(name='non-lockfree-atomics',          when=lambda cfg: sourceBuilds(cfg, """
+                                                                  #include <atomic>
+                                                                  struct Large { int storage[100]; };
+                                                                  std::atomic<Large> x;
+                                                                  int main(int, char**) { return x.load(), x.is_lock_free(); }
+                                                                """)),
 
   Feature(name='apple-clang',                                                                                                      when=_isAppleClang),
   Feature(name=lambda cfg: 'apple-clang-{__clang_major__}'.format(**compilerMacros(cfg)),                                          when=_isAppleClang),

diff --git a/libcxx/utils/libcxx/test/target_info.py b/libcxx/utils/libcxx/test/target_info.py
index 9b6697b..7c034e2 100644 (file)
--- a/libcxx/utils/libcxx/test/target_info.py
+++ b/libcxx/utils/libcxx/test/target_info.py
@@ -257,8 +257,8 @@ class LinuxLocalTI(DefaultTargetInfo):
             flags += [builtins_lib]
         else:
             flags += ['-lgcc']
-        use_libatomic = self.full_config.get_lit_bool('use_libatomic', False)
-        if use_libatomic:
+        has_libatomic = self.full_config.get_lit_bool('has_libatomic', False)
+        if has_libatomic:
             flags += ['-latomic']
         san = self.full_config.get_lit_conf('use_sanitizer', '').strip()
         if san: