[libcxx-commits] [libcxx] e018435 - [libc++] Link against libatomic when it is found
Louis Dionne via libcxx-commits
libcxx-commits at lists.llvm.org
Fri Jun 5 06:28:50 PDT 2020
Author: Louis Dionne
Date: 2020-06-05T09:28:44-04:00
New Revision: e0184357fc781e939f4e4368fc8aff692ce227ed
URL: https://github.com/llvm/llvm-project/commit/e0184357fc781e939f4e4368fc8aff692ce227ed
DIFF: https://github.com/llvm/llvm-project/commit/e0184357fc781e939f4e4368fc8aff692ce227ed.diff
LOG: [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
Added:
libcxx/test/libcxx/atomics/atomics.align/align.pass.pass.cpp
Modified:
libcxx/CMakeLists.txt
libcxx/cmake/config-ix.cmake
libcxx/test/libcxx/selftest/dsl/dsl.sh.py
libcxx/test/lit.site.cfg.in
libcxx/utils/libcxx/test/config.py
libcxx/utils/libcxx/test/dsl.py
libcxx/utils/libcxx/test/features.py
libcxx/utils/libcxx/test/target_info.py
Removed:
libcxx/cmake/Modules/CheckLibcxxAtomic.cmake
libcxx/test/libcxx/atomics/atomics.align/align.pass.sh.cpp
################################################################################
diff --git a/libcxx/CMakeLists.txt b/libcxx/CMakeLists.txt
index 1a7e0a0bc759..4eebfb8e8120 100644
--- 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
index 7fe5a6278297..000000000000
--- 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 cbb4dfd16dac..894f637f814f 100644
--- 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 604473d5e2f3..ebe8fc82775c 100644
--- 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 c8f99846c373..9cf0ac95a0b8 100644
--- a/libcxx/test/libcxx/selftest/dsl/dsl.sh.py
+++ b/libcxx/test/libcxx/selftest/dsl/dsl.sh.py
@@ -97,6 +97,24 @@ def test_multiple_flags(self):
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 7e1cfe51e18c..f00a255b5431 100644
--- 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 be8ee3be6129..11f2cb1edab1 100644
--- a/libcxx/utils/libcxx/test/config.py
+++ b/libcxx/utils/libcxx/test/config.py
@@ -325,9 +325,6 @@ def configure_features(self):
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 1110ded97967..0e21256a2de0 100644
--- a/libcxx/utils/libcxx/test/dsl.py
+++ b/libcxx/utils/libcxx/test/dsl.py
@@ -56,6 +56,26 @@ def __enter__(self): return self
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 b6c2ba7f9fa8..6d30a096da87 100644
--- a/libcxx/utils/libcxx/test/features.py
+++ b/libcxx/utils/libcxx/test/features.py
@@ -32,6 +32,12 @@
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 9b6697b612a5..7c034e2640cd 100644
--- a/libcxx/utils/libcxx/test/target_info.py
+++ b/libcxx/utils/libcxx/test/target_info.py
@@ -257,8 +257,8 @@ def add_cxx_link_flags(self, flags):
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:
More information about the libcxx-commits
mailing list