This PR introduces a `std::__is_sufficiently_aligned` helper function that
can be used for the `atomic_ref` C++20 constructors while the public
C++26 `std::is_sufficiently_aligned` delegates to the new helper
function.
Fixes#179803
Assisted-by: Claude Code
Signed-off-by: mlevine55 <mlevine55@bloomberg.net>
This PR fixes two issues regarding the alignment of native wait:
- In the internal platform call, the local variable is copied from a
potentially non-aligned buffer
- Under the unstable ABI, the predicate to test eligibility of a type
being able to do native wait is purely on size. We should test also the
alignment of such type is qualified for platform call
---------
Co-authored-by: Louis Dionne <ldionne.2@gmail.com>
This patch gets rid of bool result values which were confusing, and
instead replaces them with enum class values describing the meaning of
each return value.
It makes sense to decouple traits from the things they describe, since
that's the purpose of traits. Furthermore, this will make it possible to
reuse these traits from other places in the library (e.g. <semaphore>)
outside of the synchronization library.
Pre-C++20 support is removed as a drive-by since the synchronization
library does not support pre-C++20 anymore. This makes it possible to
simplify a few constructs by using concepts.
Requested in https://github.com/llvm/llvm-project/pull/172214#discussion_r2635968291
This is to address #146145
The issue before was that, for `std::atomic::wait/notify`, we only
support `uint64_t` to go through the native `ulock_wait` directly. Any
other types will go through the global contention table's `atomic`,
increasing the chances of spurious wakeup. This PR tries to allow any
types that are of size 4 or 8 to directly go to the `ulock_wait`.
This PR is just proof of concept. If we like this idea, I can go further
to update the Linux/FreeBSD branch and add ABI macros so the existing
behaviours are reserved under the stable ABI
Here are some benchmark results
```
Benchmark Time CPU Time Old Time New CPU Old CPU New
----------------------------------------------------------------------------------------------------------------------------------------------------
BM_stop_token_single_thread_reg_unreg_callback/1024 -0.1113 -0.1165 51519 45785 51397 45408
BM_stop_token_single_thread_reg_unreg_callback/4096 -0.2727 -0.1447 249685 181608 211865 181203
BM_stop_token_single_thread_reg_unreg_callback/65536 -0.1241 -0.1237 3308930 2898396 3300986 2892608
BM_stop_token_single_thread_reg_unreg_callback/262144 +0.0335 -0.1920 13237682 13681632 13208849 10673254
OVERALL_GEOMEAN -0.1254 -0.1447 0 0 0 0
```
```
Benchmark Time CPU Time Old Time New CPU Old CPU New
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------
BM_1_atomic_1_waiter_1_notifier<KeepNotifying, NumHighPrioTasks<0>>/65536 -0.3344 -0.2424 5960741 3967212 5232250 3964085
BM_1_atomic_1_waiter_1_notifier<KeepNotifying, NumHighPrioTasks<0>>/131072 -0.1474 -0.1475 9144356 7796745 9137547 7790193
BM_1_atomic_1_waiter_1_notifier<KeepNotifying, NumHighPrioTasks<0>>/262144 -0.1336 -0.1340 18333441 15883805 18323711 15868500
OVERALL_GEOMEAN -0.2107 -0.1761 0 0 0 0
```
```
Benchmark Time CPU Time Old Time New CPU Old CPU New
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
BM_1_atomic_multi_waiter_1_notifier<KeepNotifying, NumWaitingThreads<2>, NumHighPrioTasks<0>>/16384 +0.2321 -0.0081 836618 1030772 833197 826476
BM_1_atomic_multi_waiter_1_notifier<KeepNotifying, NumWaitingThreads<2>, NumHighPrioTasks<0>>/32768 -0.3034 -0.1329 2182721 1520569 1747211 1515028
BM_1_atomic_multi_waiter_1_notifier<KeepNotifying, NumWaitingThreads<2>, NumHighPrioTasks<0>>/65536 -0.0924 -0.0924 3389098 3075897 3378486 3066448
BM_1_atomic_multi_waiter_1_notifier<KeepNotifying, NumWaitingThreads<8>, NumHighPrioTasks<0>>/4096 +0.0464 +0.0474 664233 695080 657736 688892
BM_1_atomic_multi_waiter_1_notifier<KeepNotifying, NumWaitingThreads<8>, NumHighPrioTasks<0>>/8192 -0.0279 -0.0267 1336041 1298794 1324270 1288953
BM_1_atomic_multi_waiter_1_notifier<KeepNotifying, NumWaitingThreads<8>, NumHighPrioTasks<0>>/16384 +0.0270 +0.0304 2543004 2611786 2517471 2593975
BM_1_atomic_multi_waiter_1_notifier<KeepNotifying, NumWaitingThreads<32>, NumHighPrioTasks<0>>/1024 +0.0423 +0.0941 473621 493657 325604 356245
BM_1_atomic_multi_waiter_1_notifier<KeepNotifying, NumWaitingThreads<32>, NumHighPrioTasks<0>>/2048 +0.0420 +0.0675 906266 944349 636253 679169
BM_1_atomic_multi_waiter_1_notifier<KeepNotifying, NumWaitingThreads<32>, NumHighPrioTasks<0>>/4096 +0.0359 +0.0378 1761584 1824783 1015092 1053439
OVERALL_GEOMEAN -0.0097 -0.0007 0 0 0 0
```
```
Benchmark Time CPU Time Old Time New CPU Old CPU New
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
BM_N_atomics_N_waiter_N_notifier<KeepNotifying, NumberOfAtomics<2>, NumHighPrioTasks<0>>/4096 -0.0990 -0.1001 371100 334370 369984 332955
BM_N_atomics_N_waiter_N_notifier<KeepNotifying, NumberOfAtomics<2>, NumHighPrioTasks<0>>/8192 -0.0305 -0.0314 698228 676908 696418 674585
BM_N_atomics_N_waiter_N_notifier<KeepNotifying, NumberOfAtomics<2>, NumHighPrioTasks<0>>/16384 -0.0258 -0.0268 1383530 1347894 1380665 1343680
BM_N_atomics_N_waiter_N_notifier<KeepNotifying, NumberOfAtomics<8>, NumHighPrioTasks<0>>/1024 +0.0465 +0.4702 937821 981388 472087 694082
BM_N_atomics_N_waiter_N_notifier<KeepNotifying, NumberOfAtomics<8>, NumHighPrioTasks<0>>/2048 +0.1596 +0.9140 1704819 1976899 616419 1179852
BM_N_atomics_N_waiter_N_notifier<KeepNotifying, NumberOfAtomics<8>, NumHighPrioTasks<0>>/4096 -0.1018 -0.2316 3793976 3407609 1912209 1469331
BM_N_atomics_N_waiter_N_notifier<KeepNotifying, NumberOfAtomics<32>, NumHighPrioTasks<0>>/256 +0.0395 +0.5818 30102662 31292982 174650 276270
BM_N_atomics_N_waiter_N_notifier<KeepNotifying, NumberOfAtomics<32>, NumHighPrioTasks<0>>/512 -0.0065 +1.2860 33079634 32863968 162150 370680
BM_N_atomics_N_waiter_N_notifier<KeepNotifying, NumberOfAtomics<32>, NumHighPrioTasks<0>>/1024 -0.0325 +0.4683 36581740 35392385 282320 414520
OVERALL_GEOMEAN -0.0084 +0.2878 0 0 0 0
```
---------
Co-authored-by: Louis Dionne <ldionne.2@gmail.com>
Apple dropped support for some older platforms, so we can also remove
the annotations for them.
See https://developer.apple.com/support/xcode/ for the supported
versions.
We've built up quite a few links directly to github within the code
base. We should instead use `llvm.org/PR<issue-number>` to link to bugs,
since that is resilient to the bug tracker changing in the future. This
is especially relevant for tests linking to bugs, since they will
probably be there for decades to come. A nice side effect is that these
links are significantly shorter than the GH links, making them much less
of an eyesore.
This patch also replaces a few links that linked to the old bugzilla
instance on llvm.org.
This reverts commit c861fe8a71e64f3d2108c58147e7375cd9314521.
Unfortunately, this use of hidden visibility attributes causes
user-defined specializations of standard-library types to also be marked
hidden by default, which is incorrect. See discussion thread on #131156.
...and also reverts the follow-up commits:
Revert "[libc++] Add explicit ABI annotations to functions from the block runtime declared in <__functional/function.h> (#140592)"
This reverts commit 3e4c9dc299c35155934688184319d391b298fff7.
Revert "[libc++] Make ABI annotations explicit for windows-specific code (#140507)"
This reverts commit f73287e623a6c2e4a3485832bc3e10860cd26eb5.
Revert "[libc++][NFC] Replace a few "namespace std" with the correct macro (#140510)"
This reverts commit 1d411f27c769a32cb22ce50b9dc4421e34fd40dd.
This patch introduces `_LIBCPP_{BEGIN,END}_EXPLICIT_ABI_ANNOTATIONS`,
which allow us to have implicit annotations for most functions, and just
where it's not "hide_from_abi everything" we add explicit annotations.
This allows us to drop the `_LIBCPP_HIDE_FROM_ABI` macro from most
functions in libc++.
This avoids instantiating multiple classes, reducing compile times. This
patch also introduces `__underyling_type_t` for internal use, similar to
other type traits.
When attempting to instantiate `std::atomic` with a non trivially
copyable type, one gets errors from instantiating internals before the
actual static assertion that check the template parameter type
requirements.
The `verify` test for it had a `// ADDITIONAL_COMPILE_FLAGS: -Xclang
-verify-ignore-unexpected=error` directive to work around this issue.
The changes I propose enable us to drop that directive.
As I understand it, the `verify` test was misplaced so I moved it to
`test/{std -> libcxx}/atomics`.
(I ran into this while working on #121414 in which we would add another
static assertion in `__check_atomic_mandates`)
We only use `_LIBCPP_DISABLE_EXTENSION_WARNINGS` in a single place while
we use extensions all over the place. The warnings are already disabled,
since libc++'s headers are system headers, so this shouldn't be in any
way observable by users.
This set usage of operator& instead of std::addressof seems not be easy
to "abuse". Some seem easy to misuse, like basic_ostream::operator<<,
trying to do that results in compilation errors since the `widen`
function is not specialized for the hijacking character type. Hence
there are no tests.
The test would not check its absence and the code path intended for
pointer was never actually instantiated.
I added a few pointer types since there was no coverage.
---------
Co-authored-by: Louis Dionne <ldionne.2@gmail.com>
This patch documents the underlying API for implementing atomics on a
platform.
This doesn't change the operations that std::atomic is based on, but it
reorganizes the C11 / GCC implementation split to make it clearer what's
the base support layer and what's not.
Now that we've dropped support for older C++ dialects in the
synchronization library, we can use lambdas to clarify some of the code
used to implement atomic_wait.
The __atomic_base base class is only useful to conditionalize the
operations we provide inside std::atomic. It shouldn't be used directly
from other places in the library which can use std::atomic directly
instead.
Since we've granularized our includes, using std::atomic directly should
not make much of a difference compile-time wise.
This patch starts using std::atomic directly from other classes like
std::barrier and std::latch. Changing this shouldn't be an ABI break
since both classes have the same size and layout.
The benefits of this patch are isolating other parts of the code base
from implementation details of std::atomic and simplifying the mental
model for std::atomic's layers of implementation by making it clear that
__atomic_base is only an implementation detail of std::atomic.
Currently, the library-internal feature test macros are only defined if
the feature is not available, and always have the prefix
`_LIBCPP_HAS_NO_`. This patch changes that, so that they are always
defined and have the prefix `_LIBCPP_HAS_` instead. This changes the
canonical use of these macros to `#if _LIBCPP_HAS_FEATURE`, which means
that using an undefined macro (e.g. due to a missing include) is
diagnosed now. While this is rather unlikely currently, a similar change
in `<__configuration/availability.h>` caught a few bugs. This also
improves readability, since it removes the double-negation of `#ifndef
_LIBCPP_HAS_NO_FEATURE`.
The current patch only touches the macros defined in `<__config>`. If
people are happy with this approach, I'll make a follow-up PR to also
change the macros defined in `<__config_site>`.
This patch adds a large number of missing includes in the libc++ headers
and the test suite. Those were found as part of the effort to move
towards a mostly monolithic top-level std module.
Many headers include `<cstddef>` just for size_t, and pulling in
additional content (e.g. the traits used for std::byte) is unnecessary.
To solve this problem, this patch splits up `<cstddef>` into
subcomponents so that headers can include only the parts that they
actually require.
This has the added benefit of making the modules build a lot stricter
with respect to IWYU, and also providing a canonical location where we
define `std::size_t` and friends (which were previously defined in
multiple headers like `<cstddef>` and `<ctime>`).
After this patch, there's still many places in the codebase where we
include `<cstddef>` when `<__cstddef/size_t.h>` would be sufficient.
This patch focuses on removing `<cstddef>` includes from __type_traits
to make these headers non-circular with `<cstddef>`. Additional
refactorings can be tackled separately.
This patch increases the alignment requirement for std::atomic_ref
such that we can guarantee lockfree operations more often. Specifically,
we require types that are 1, 2, 4, 8, or 16 bytes in size to be aligned
to at least their size to be used with std::atomic_ref.
This is the case for most types, however a notable exception is
`long long` on x86, which is 8 bytes in length but has an alignment
of 4.
As a result of this patch, one has to be more careful about the
alignment of objects used with std::atomic_ref. Failure to provide
a properly-aligned object to std::atomic_ref is a precondition
violation and is technically UB. On the flipside, this allows us
to provide an atomic_ref that is actually lockfree more often,
which is an important QOI property.
More information in the discussion at https://github.com/llvm/llvm-project/pull/99570#issuecomment-2237668661.
Co-authored-by: Louis Dionne <ldionne.2@gmail.com>
When we initially implemented the C++20 synchronization library, we
reluctantly accepted for the implementation to be backported to C++03
upon request from the person who provided the patch. This was when we
were only starting to have experience with the issues this can create,
so we flinched. Nowadays, we have a much stricter stance about not
backporting features to previous standards.
We have recently started fixing several bugs (and near bugs) in our
implementation of the synchronization library. A recurring theme during
these reviews has been how difficult to understand the current code is,
and upon inspection it becomes clear that being able to use a few recent
C++ features (in particular lambdas) would help a great deal. The code
would still be pretty intricate, but it would be a lot easier to reason
about the flow of callbacks through things like
__thread_poll_with_backoff.
As a result, this patch drops support for the synchronization library
before C++20. This makes us more strictly conforming and opens the door
to major simplifications, in particular around atomic_wait which was
supported all the way to C++03.
This change will probably have some impact on downstream users, however
since the C++20 synchronization library was added only in LLVM 10 (~3
years ago) and it's quite a niche feature, the set of people trying to
use this part of the library before C++20 should be reasonably small.
The builtin __atomic_always_lock_free takes into account the type of the
pointer provided as the second argument. Because we were passing void*,
rather than T*, the calculation failed. This meant that
atomic_ref<T>::is_always_lock_free was only true for char & bool.
This bug exists elsewhere in the atomic library (when using GCC, we fail
to pass a pointer at all, and we fail to correctly align the atomic like
_Atomic would).
This change also attempts to start sorting out testing difficulties with
this function that caused the bug to exist by using the
__GCC_ATOMIC_(CHAR|SHORT|INT|LONG|LLONG|POINTER)_IS_LOCK_FREE predefined
macros to establish an expected value for `is_always_lock_free` and
`is_lock_free` for the respective types, as well as types with matching
sizes and compatible alignment values.
Using these compiler pre-defines we can actually validate that certain
types, like char and int, are actually always lock free like they are on
every platform in the wild.
Note that this patch was actually authored by Eric Fiselier but I picked
up the patch and GitHub won't let me set Eric as the primary author.
Co-authored-by: Eric Fiselier <eric@efcs.ca>
The GCC build has gotten to the point where it's often hard to find the
actual error in the build log. We should look into enabling these
warnings again in the future, but it looks like a lot of them are
bogous.
As time went by, a few files have become mis-formatted w.r.t.
clang-format. This was made worse by the fact that formatting was not
being enforced in extensionless headers. This commit simply brings all
of libcxx/include in-line with clang-format again.
We might have to do this from time to time as we update our clang-format
version, but frankly this is really low effort now that we've formatted
everything once.
These were required a long time ago due to `static_assert` not actually
being available in C++03. Now `static_assert` is simply mapped to
`_Static_assert` in C++03, making the additional parens unnecessary.
In essence, this header has always been related to configuration of
the library but we didn't want to put it inside <__config> due to
complexity reasons. Now that we have sub-headers in <__config>, we
can move <__availability> to it and stop including it everywhere since
we already obtain the required macros via <__config>.
When we initially implemented the C++20 synchronization library, we
reluctantly accepted for the implementation to be backported to C++03
upon request from the person who provided the patch. This was when we
were only starting to have experience with the issues this can create,
so we flinched. Nowadays, we have a much stricter stance about not
backporting features to previous standards.
We have recently started fixing several bugs (and near bugs) in our
implementation of the synchronization library. A recurring theme during
these reviews has been how difficult to understand the current code is,
and upon inspection it becomes clear that being able to use a few recent
C++ features (in particular lambdas) would help a great deal. The code
would still be pretty intricate, but it would be a lot easier to reason
about the flow of callbacks through things like
__thread_poll_with_backoff.
As a result, this patch deprecates support for the synchronization
library before C++20. In the next release, we can remove that support
entirely.
These headers have become very small by using compiler builtins, often
containing only two declarations. This merges these headers, since
there doesn't seem to be much of a benefit keeping them separate.
Specifically, `is_{,_nothrow,_trivially}{assignable,constructible}` are
kept and the `copy`, `move` and `default` versions of these type traits
are moved in to the respective headers.
The goal of this patch is to make `atomic`'s wait functions to be
reusable by `atomic_ref`.
https://github.com/llvm/llvm-project/pull/76647
First, this patch is built on top of
https://github.com/llvm/llvm-project/pull/80596 , to reduce the future
merge conflicts.
This patch made the following functions as "API"s to be used by
`atomic`, `atomic_flag`, `semaphore`, `latch`, `atomic_ref`
```
__atomic_wait
__atomic_wait_unless
__atomic_notify_one
__atomic_notify_all
```
These functions are made generic to support `atomic` type and
`atomic_ref`. There are two customisation points.
```
// How to load the value from the given type (with a memory order)
__atomic_load
```
```
// what is the contention address that the platform `wait` function is going to monitor
__atomic_contention_address
```
For `atomic_ref` (not implemented in this patch), the `load` and
`address` function will be different, because
- it does not use the "atomic abstraction layer" so the `load` operation
will be some gcc builtin
- the contention address will be the user's actual type that the
`atomic_ref` is pointing to
Originally, we used __libcpp_verbose_abort to handle assertion failures.
That function was declared from all public headers. Since we don't use
that mechanism anymore, we don't need to declare __libcpp_verbose_abort
from all public headers, and we can clean up a lot of unnecessary
includes.
This patch also moves the definition of the various assertion categories
to the <__assert> header, since we now rely on regular IWYU for these
assertion macros.
rdar://105510916
As discussed in #76647, _LIBCPP_ATOMIC_ONLY_USE_BUILTINS is a
questionable configuration option. It makes our implementation of
std::atomic even more complicated than it already is for a limited
benefit.
Indeed, the original goal of that setting was to decouple libc++ from
libraries like compiler-rt and libatomic in Freestanding mode. We didn't
have a clear understanding of goals and non-goals of Freestanding back
then, but nowadays we do have a better understanding that removing all
dependencies of libc++ in Freestanding is a non-goal. We should still be
able to depend on builtins like those defined in compiler-rt for
implementing our atomic operations in Freestanding. Freestanding means
that there is no underlying operating system, not that there is no
toolchain available.
This patch removes the configuration option. This should have a very
limited fallout since that configuration was only enabled with
-ffreestanding, and libc++ basically doesn't work out of the box on
Freestanding platforms today.
The benefits are a slightly simpler implementation of std::atomic,
getting rid of one of the ABI-incompatible representations of
std::atomic, and clearing the way for proper Freestanding support to
eventually land in the library.
Fixes#81286
This is a follow-up PR to
<https://github.com/llvm/llvm-project/pull/79265>. It aims to be a
gentle refactoring of the `__cxx_atomic_wait` function that takes a
predicate.
The key idea here is that this function's signature is changed to look
like this (`std::function` used just for clarity):
```c++
__cxx_atomic_wait_fn(Atp*, std::function<bool(Tp &)> poll, memory_order __order);
```
...where `Tp` is the corresponding `value_type` to the atomic variable
type `Atp`. The function's semantics are similar to `atomic`s `.wait()`,
but instead of having a hardcoded predicate (is the loaded value unequal
to `old`?) the predicate is specified explicitly.
The `poll` function may change its argument, and it is very important
that if it returns `false`, it leaves its current understanding of the
atomic's value in the argument. Internally, `__cxx_atomic_wait_fn`
dispatches to two waiting mechanisms, depending on the type of the
atomic variable:
1. If the atomic variable can be waited on directly (for example,
Linux's futex mechanism only supports waiting on 32 bit long variables),
the value of the atomic variable (which `poll` made its decision on) is
then given to the underlying system wait function (e.g. futex).
2. If the atomic variable can not be waited on directly, there is a
global pool of atomics that are used for this task. The ["eventcount"
pattern](<https://gist.github.com/mratsim/04a29bdd98d6295acda4d0677c4d0041>)
is employed to make this possible.
The eventcount pattern needs a "monitor" variable which is read before
the condition is checked another time. libcxx has the
`__libcpp_atomic_monitor` function for this. However, this function only
has to be called in case "2", i.e. when the eventcount is actually used.
In case "1", the futex is used directly, so the monitor must be the
value of the atomic variable that the `poll` function made its decision
on to continue blocking. Previously, `__libcpp_atomic_monitor` was
_also_ used in case "1". This was the source of the ABA style bug that
PR#79265 fixed.
However, the solution in PR#79265 has some disadvantages:
- It exposes internals such as `cxx_contention_t` or the fact that
`__libcpp_thread_poll_with_backoff` needs two functions to higher level
constructs such as `semaphore`.
- It doesn't prevent consumers calling `__cxx_atomic_wait` in an error
prone way, i.e. by providing to it a predicate that doesn't take an
argument. This makes ABA style issues more likely to appear.
Now, `__cxx_atomic_wait_fn` takes just _one_ function, which is then
transformed into the `poll` and `backoff` callables needed by
`__libcpp_thread_poll_with_backoff`.
Aside from the `__cxx_atomic_wait` changes, the only other change is the
weakening of the initial atomic load of `semaphore`'s `try_acquire` into
`memory_order_relaxed` and the CAS inside the loop is changed from
`strong` to `weak`. Both weakenings should be fine, since the CAS is
called in a loop, and the "acquire" semantics of `try_acquire` come from
the CAS, not from the initial load.
