Skip to content

Commit

Permalink
[libc++] Remove internal "build-with-external-thread-library" configu…
Browse files Browse the repository at this point in the history
…ration

Our threading support layer is currently a huge mess. There are too many
configurations with too many confusing names, and none of them are tested
in the usual CI. Here's a list of names related to these configurations:

  LIBCXX_BUILD_EXTERNAL_THREAD_LIBRARY
  _LIBCPP_BUILDING_THREAD_LIBRARY_EXTERNAL

  LIBCXXABI_BUILD_EXTERNAL_THREAD_LIBRARY
  _LIBCPP_HAS_THREAD_LIBRARY_EXTERNAL

  LIBCXX_HAS_EXTERNAL_THREAD_API
  _LIBCPP_HAS_THREAD_API_EXTERNAL

This patch cleans this up by removing the ability to build libc++ with
an "external" threading library for testing purposes, removing 4 out of
6 "names" above. That setting was meant to be used by libc++ developers,
but we don't use it in-tree and it's not part of our CI.

I know the ability to use an external threading API is used by some folks
out-of-tree, and this patch doesn't change that. This only changes the
way they will have to test their external threading support. After this
patch, the intent would be for them to set `-DLIBCXX_HAS_EXTERNAL_THREAD_API=ON`
when building the library, and to provide their usual `<__external_threading>`
header when they are testing the library. This can be done easily now
that we support custom lit configuration files in test suites.

The motivation for this patch is that our threading support layer is
basically unmaintainable -- anything beyond adding a new "backend" in
the slot designed for it requires incredible attention. The complexity
added by this setting just doesn't pull its weigh considering the
available alternatives.

Concretely, this will also allow future patches to clean up
`<__threading_support>` significantly.

Differential Revision: https://reviews.llvm.org/D154466
  • Loading branch information
ldionne committed Jul 17, 2023
1 parent 062fce6 commit 8ac71b0
Show file tree
Hide file tree
Showing 8 changed files with 2 additions and 111 deletions.
28 changes: 0 additions & 28 deletions libcxx/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -272,9 +272,6 @@ option(LIBCXX_HAS_WIN32_THREAD_API "Ignore auto-detection and force use of win32
option(LIBCXX_HAS_EXTERNAL_THREAD_API
"Build libc++ with an externalized threading API.
This option may only be set to ON when LIBCXX_ENABLE_THREADS=ON." OFF)
option(LIBCXX_BUILD_EXTERNAL_THREAD_LIBRARY
"Build libc++ with an externalized threading library.
This option may only be set to ON when LIBCXX_ENABLE_THREADS=ON" OFF)

if (LIBCXX_ENABLE_THREADS)
set(LIBCXX_PSTL_CPU_BACKEND "std_thread" CACHE STRING "Which PSTL CPU backend to use")
Expand Down Expand Up @@ -327,10 +324,6 @@ if(NOT LIBCXX_ENABLE_THREADS)
message(FATAL_ERROR "LIBCXX_HAS_EXTERNAL_THREAD_API can only be set to ON"
" when LIBCXX_ENABLE_THREADS is also set to ON.")
endif()
if (LIBCXX_BUILD_EXTERNAL_THREAD_LIBRARY)
message(FATAL_ERROR "LIBCXX_BUILD_EXTERNAL_THREAD_LIBRARY can only be set "
"to ON when LIBCXX_ENABLE_THREADS is also set to ON.")
endif()
if (LIBCXX_HAS_WIN32_THREAD_API)
message(FATAL_ERROR "LIBCXX_HAS_WIN32_THREAD_API can only be set to ON"
" when LIBCXX_ENABLE_THREADS is also set to ON.")
Expand All @@ -339,11 +332,6 @@ if(NOT LIBCXX_ENABLE_THREADS)
endif()

if (LIBCXX_HAS_EXTERNAL_THREAD_API)
if (LIBCXX_BUILD_EXTERNAL_THREAD_LIBRARY)
message(FATAL_ERROR "The options LIBCXX_BUILD_EXTERNAL_THREAD_LIBRARY and "
"LIBCXX_HAS_EXTERNAL_THREAD_API cannot both be ON at "
"the same time")
endif()
if (LIBCXX_HAS_PTHREAD_API)
message(FATAL_ERROR "The options LIBCXX_HAS_EXTERNAL_THREAD_API"
"and LIBCXX_HAS_PTHREAD_API cannot be both"
Expand Down Expand Up @@ -573,17 +561,6 @@ function(cxx_add_rtti_flags target)
endif()
endfunction()

# Threading flags =============================================================
if (LIBCXX_BUILD_EXTERNAL_THREAD_LIBRARY AND LIBCXX_ENABLE_SHARED)
# Need to allow unresolved symbols if this is to work with shared library builds
if (APPLE)
add_link_flags("-undefined dynamic_lookup")
else()
# Relax this restriction from HandleLLVMOptions
string(REPLACE "-Wl,-z,defs" "" CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS}")
endif()
endif()

# Modules flags ===============================================================
# FIXME The libc++ sources are fundamentally non-modular. They need special
# versions of the headers in order to provide C++03 and legacy ABI definitions.
Expand Down Expand Up @@ -764,7 +741,6 @@ endif()
config_define_if(LIBCXX_HAS_PTHREAD_API _LIBCPP_HAS_THREAD_API_PTHREAD)
config_define_if(LIBCXX_HAS_EXTERNAL_THREAD_API _LIBCPP_HAS_THREAD_API_EXTERNAL)
config_define_if(LIBCXX_HAS_WIN32_THREAD_API _LIBCPP_HAS_THREAD_API_WIN32)
config_define_if(LIBCXX_BUILD_EXTERNAL_THREAD_LIBRARY _LIBCPP_HAS_THREAD_LIBRARY_EXTERNAL)
config_define_if(LIBCXX_HAS_MUSL_LIBC _LIBCPP_HAS_MUSL_LIBC)
config_define_if(LIBCXX_NO_VCRUNTIME _LIBCPP_NO_VCRUNTIME)
config_define_if_not(LIBCXX_ENABLE_FILESYSTEM _LIBCPP_HAS_NO_FILESYSTEM)
Expand Down Expand Up @@ -860,10 +836,6 @@ endif()

set(LIBCXX_TEST_DEPS "cxx_experimental")

if (LIBCXX_BUILD_EXTERNAL_THREAD_LIBRARY)
list(APPEND LIBCXX_TEST_DEPS cxx_external_threads)
endif()

if (LIBCXX_ENABLE_CLANG_TIDY)
list(APPEND LIBCXX_TEST_DEPS cxx-tidy)
endif()
Expand Down
20 changes: 0 additions & 20 deletions libcxx/docs/DesignDocs/ThreadingSupportAPI.rst
Original file line number Diff line number Diff line change
Expand Up @@ -42,14 +42,6 @@ On a production setting, this would be achieved through a custom
``<__external_threading>`` header, which declares the libc++ internal threading
API but leaves out the implementation.

The ``-DLIBCXX_BUILD_EXTERNAL_THREAD_LIBRARY`` option allows building libc++ in
such a configuration while allowing it to be tested on a platform that supports
any of the threading systems (e.g. pthread) supported in ``__threading_support``
header. Therefore, the main purpose of this option is to allow testing of this
particular configuration of the library without being tied to a vendor-specific
threading system. This option is only meant to be used by libc++ library
developers.

Threading Configuration Macros
==============================

Expand All @@ -69,15 +61,3 @@ Threading Configuration Macros
**_LIBCPP_HAS_THREAD_API_WIN32**
This macro is defined when libc++ should use Win32 threads to implement the
internal threading API.

**_LIBCPP_HAS_THREAD_LIBRARY_EXTERNAL**
This macro is defined when libc++ expects the definitions of the internal
threading API to be provided by an external library. When defined
``<__threading_support>`` will only provide the forward declarations and
typedefs for the internal threading API.

**_LIBCPP_BUILDING_THREAD_LIBRARY_EXTERNAL**
This macro is used to build an external threading library using the
``<__threading_support>``. Specifically it exposes the threading API
definitions in ``<__threading_support>`` as non-inline definitions meant to
be compiled into a library.
1 change: 0 additions & 1 deletion libcxx/include/__config_site.in
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
#cmakedefine _LIBCPP_HAS_THREAD_API_PTHREAD
#cmakedefine _LIBCPP_HAS_THREAD_API_EXTERNAL
#cmakedefine _LIBCPP_HAS_THREAD_API_WIN32
#cmakedefine _LIBCPP_HAS_THREAD_LIBRARY_EXTERNAL
#cmakedefine _LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS
#cmakedefine _LIBCPP_HAS_NO_VENDOR_AVAILABILITY_ANNOTATIONS
#cmakedefine _LIBCPP_NO_VCRUNTIME
Expand Down
9 changes: 1 addition & 8 deletions libcxx/include/__threading_support
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,7 @@
# include <threads.h>
#endif

#if defined(_LIBCPP_HAS_THREAD_LIBRARY_EXTERNAL) || \
defined(_LIBCPP_BUILDING_THREAD_LIBRARY_EXTERNAL) || \
defined(_LIBCPP_HAS_THREAD_API_WIN32)
#if defined(_LIBCPP_HAS_THREAD_API_WIN32)
#define _LIBCPP_THREAD_ABI_VISIBILITY _LIBCPP_EXPORTED_FROM_ABI
#else
#define _LIBCPP_THREAD_ABI_VISIBILITY inline _LIBCPP_INLINE_VISIBILITY
Expand Down Expand Up @@ -248,9 +246,6 @@ int __libcpp_tls_set(__libcpp_tls_key __key, void *__p);

#endif // !defined(_LIBCPP_HAS_THREAD_API_EXTERNAL)

#if (!defined(_LIBCPP_HAS_THREAD_LIBRARY_EXTERNAL) || \
defined(_LIBCPP_BUILDING_THREAD_LIBRARY_EXTERNAL))

#if defined(_LIBCPP_HAS_THREAD_API_PTHREAD)

int __libcpp_recursive_mutex_init(__libcpp_recursive_mutex_t *__m)
Expand Down Expand Up @@ -585,8 +580,6 @@ int __libcpp_tls_set(__libcpp_tls_key __key, void *__p)

#endif

#endif // !_LIBCPP_HAS_THREAD_LIBRARY_EXTERNAL || _LIBCPP_BUILDING_THREAD_LIBRARY_EXTERNAL

#endif // !_LIBCPP_HAS_NO_THREADS

_LIBCPP_END_NAMESPACE_STD
Expand Down
21 changes: 0 additions & 21 deletions libcxx/src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -346,27 +346,6 @@ set_target_properties(cxx_experimental
cxx_add_common_build_flags(cxx_experimental)
target_compile_options(cxx_experimental PUBLIC -D_LIBCPP_ENABLE_EXPERIMENTAL)


if (LIBCXX_BUILD_EXTERNAL_THREAD_LIBRARY)
set(LIBCXX_EXTERNAL_THREADING_SUPPORT_SOURCES
"${CMAKE_CURRENT_SOURCE_DIR}/../test/support/external_threads.cpp")

if (LIBCXX_ENABLE_SHARED)
add_library(cxx_external_threads SHARED ${LIBCXX_EXTERNAL_THREADING_SUPPORT_SOURCES})
else()
add_library(cxx_external_threads STATIC ${LIBCXX_EXTERNAL_THREADING_SUPPORT_SOURCES})
endif()

set_target_properties(cxx_external_threads
PROPERTIES
LINK_FLAGS "${LIBCXX_LINK_FLAGS}"
COMPILE_FLAGS "${LIBCXX_COMPILE_FLAGS}"
OUTPUT_NAME "c++external_threads"
)

target_link_libraries(cxx_external_threads PRIVATE cxx-headers)
endif()

if (LIBCXX_INSTALL_SHARED_LIBRARY)
install(TARGETS cxx_shared
ARCHIVE DESTINATION ${LIBCXX_INSTALL_LIBRARY_DIR} COMPONENT cxx
Expand Down
9 changes: 0 additions & 9 deletions libcxx/test/support/external_threads.cpp

This file was deleted.

21 changes: 1 addition & 20 deletions libcxxabi/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,6 @@ option(LIBCXXABI_HAS_WIN32_THREAD_API "Ignore auto-detection and force use of wi
option(LIBCXXABI_HAS_EXTERNAL_THREAD_API
"Build libc++abi with an externalized threading API.
This option may only be set to ON when LIBCXXABI_ENABLE_THREADS=ON." OFF)
option(LIBCXXABI_BUILD_EXTERNAL_THREAD_LIBRARY
"Build libc++abi with an externalized threading library.
This option may only be set to ON when LIBCXXABI_ENABLE_THREADS=ON" OFF)
option(LIBCXXABI_ENABLE_FORGIVING_DYNAMIC_CAST
"Make dynamic_cast more forgiving when type_info's mistakenly have hidden \
visibility, and thus multiple type_infos can exist for a single type. \
Expand Down Expand Up @@ -330,11 +327,6 @@ if (NOT LIBCXXABI_ENABLE_THREADS)
" be set to ON when LIBCXXABI_ENABLE_THREADS"
" is also set to ON.")
endif()
if (LIBCXXABI_BUILD_EXTERNAL_THREAD_LIBRARY)
message(FATAL_ERROR "LIBCXXABI_BUILD_EXTERNAL_THREAD_LIBRARY can only"
" be set to ON when LIBCXXABI_ENABLE_THREADS"
" is also set to ON.")
endif()
add_definitions(-D_LIBCXXABI_HAS_NO_THREADS)
endif()

Expand All @@ -349,11 +341,6 @@ if (LIBCXXABI_HAS_EXTERNAL_THREAD_API)
" and LIBCXXABI_HAS_WIN32_THREAD_API cannot be both"
" set to ON at the same time.")
endif()
if (LIBCXXABI_BUILD_EXTERNAL_THREAD_LIBRARY)
message(FATAL_ERROR "The options LIBCXXABI_BUILD_EXTERNAL_THREAD_LIBRARY"
" and LIBCXXABI_HAS_EXTERNAL_THREAD_API cannot be both"
" set to ON at the same time.")
endif()
endif()

if (LIBCXXABI_HAS_PTHREAD_API)
Expand All @@ -371,9 +358,7 @@ if (LLVM_ENABLE_MODULES)
endif()

set(LIBCXXABI_HAS_UNDEFINED_SYMBOLS OFF)
if ((NOT LIBCXXABI_ENABLE_NEW_DELETE_DEFINITIONS)
OR (LIBCXXABI_BUILD_EXTERNAL_THREAD_LIBRARY AND LIBCXXABI_ENABLE_SHARED)
OR MINGW)
if ((NOT LIBCXXABI_ENABLE_NEW_DELETE_DEFINITIONS) OR MINGW)
set(LIBCXXABI_HAS_UNDEFINED_SYMBOLS ON)
endif()

Expand All @@ -399,10 +384,6 @@ if (LIBCXXABI_HAS_EXTERNAL_THREAD_API)
add_definitions(-D_LIBCPP_HAS_THREAD_API_EXTERNAL)
endif()

if (LIBCXXABI_BUILD_EXTERNAL_THREAD_LIBRARY)
add_definitions(-D_LIBCPP_HAS_THREAD_LIBRARY_EXTERNAL)
endif()

if (MSVC)
add_definitions(-D_CRT_SECURE_NO_WARNINGS)
endif()
Expand Down
4 changes: 0 additions & 4 deletions libcxxabi/test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,6 @@ else()
set(LIBCXXABI_TEST_DEPS cxxabi_static)
endif()

if (LIBCXXABI_BUILD_EXTERNAL_THREAD_LIBRARY)
list(APPEND LIBCXXABI_TEST_DEPS cxx_external_threads)
endif()

list(APPEND LIBCXXABI_TEST_DEPS cxx)
if (LIBCXXABI_USE_LLVM_UNWINDER AND TARGET unwind)
list(APPEND LIBCXXABI_TEST_DEPS unwind)
Expand Down

0 comments on commit 8ac71b0

Please sign in to comment.