Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix atomic_ref<16 bytes>::is_always_lock_free #4478

Merged
merged 14 commits into from
Mar 19, 2024
Merged
15 changes: 10 additions & 5 deletions stl/inc/atomic
Original file line number Diff line number Diff line change
Expand Up @@ -1656,7 +1656,7 @@ _INLINE_VAR constexpr bool _Is_always_lock_free = _TypeSize <= 2 * sizeof(void*)
#else // ^^^ _ATOMIC_HAS_DCAS / !_ATOMIC_HAS_DCAS vvv
template <size_t _TypeSize>
_INLINE_VAR constexpr bool _Is_always_lock_free = _TypeSize <= sizeof(void*);
#endif // _ATOMIC_HAS_DCAS
#endif // ^^^ !_ATOMIC_HAS_DCAS ^^^
#endif // ^^^ break ABI ^^^

template <class _Ty, bool _Is_lock_free = _Is_always_lock_free<sizeof(_Ty)>>
Expand Down Expand Up @@ -2179,7 +2179,7 @@ public:
return sizeof(_Ty) <= 2 * sizeof(void*);
#else // ^^^ _ATOMIC_HAS_DCAS / !_ATOMIC_HAS_DCAS vvv
return sizeof(_Ty) <= sizeof(void*) || (sizeof(_Ty) <= 2 * sizeof(void*) && __std_atomic_has_cmpxchg16b());
#endif // _ATOMIC_HAS_DCAS
#endif // ^^^ !_ATOMIC_HAS_DCAS ^^^
}
#endif // ^^^ break ABI ^^^

Expand Down Expand Up @@ -2343,11 +2343,16 @@ public:

atomic_ref& operator=(const atomic_ref&) = delete;

static constexpr bool is_always_lock_free = _Is_always_lock_free<sizeof(_Ty)>;

static constexpr bool _Is_potentially_lock_free =
sizeof(_Ty) <= 2 * sizeof(void*) && (sizeof(_Ty) & (sizeof(_Ty) - 1)) == 0;

static constexpr bool is_always_lock_free =
#if _ATOMIC_HAS_DCAS
_Is_potentially_lock_free;
#else // ^^^ _ATOMIC_HAS_DCAS / !_ATOMIC_HAS_DCAS vvv
_Is_potentially_lock_free && sizeof(_Ty) <= sizeof(void*);
#endif // ^^^ !_ATOMIC_HAS_DCAS ^^^

static constexpr size_t required_alignment = _Is_potentially_lock_free ? sizeof(_Ty) : alignof(_Ty);

_NODISCARD bool is_lock_free() const noexcept {
Expand All @@ -2359,7 +2364,7 @@ public:
} else {
return __std_atomic_has_cmpxchg16b() != 0;
}
#endif // _ATOMIC_HAS_DCAS
#endif // ^^^ !_ATOMIC_HAS_DCAS ^^^
}

void store(const _Ty _Value) const noexcept {
Expand Down
3 changes: 3 additions & 0 deletions tests/std/tests/P0019R8_atomic_ref/env.lst
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,6 @@
# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception

RUNALL_INCLUDE ..\usual_20_matrix.lst
RUNALL_CROSSLIST
* PM_CL=""
* PM_CL="/D_STD_ATOMIC_ALWAYS_USE_CMPXCHG16B=1 /DTEST_CMPXCHG16B"
32 changes: 32 additions & 0 deletions tests/std/tests/P0019R8_atomic_ref/test.cpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
// Copyright (c) Microsoft Corporation.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception

#if defined(TEST_CMPXCHG16B) && (defined(__clang__) || !defined(_M_X64))
// Skip Clang because it would require the -mcx16 compiler option.
// Skip non-x64 because _STD_ATOMIC_ALWAYS_USE_CMPXCHG16B is always 1 for ARM64, and is forbidden to be 1 for 32-bit.
int main() {}
#else // ^^^ skip test / run test vvv

#include <atomic>
#include <cassert>
#include <cstddef>
Expand Down Expand Up @@ -373,6 +379,29 @@ void test_incomplete_associated_class_all() { // COMPILE-ONLY
}
#endif // ^^^ no workaround ^^^

// GH-4472 "<atomic>: With _STD_ATOMIC_ALWAYS_USE_CMPXCHG16B defined to 1,
// atomic_ref<16 bytes> does not report is_lock_free and is_always_lock_free correctly"
void test_gh_4472() {
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
struct two_pointers_t {
void* left;
void* right;
};

alignas(std::atomic_ref<two_pointers_t>::required_alignment) two_pointers_t two_pointers;

static_assert(std::atomic_ref<two_pointers_t>::required_alignment == sizeof(two_pointers_t));

#ifdef _WIN64
static_assert(std::atomic_ref<two_pointers_t>::is_always_lock_free == _STD_ATOMIC_ALWAYS_USE_CMPXCHG16B);
#else
static_assert(std::atomic_ref<two_pointers_t>::is_always_lock_free);
#endif

// We expect tests to run on machines that support DCAS, which is required by Win8+.
std::atomic_ref<two_pointers_t> ar{two_pointers};
assert(ar.is_lock_free());
}

int main() {
test_ops<false, char>();
test_ops<false, signed char>();
Expand Down Expand Up @@ -425,4 +454,7 @@ int main() {
test_ptr_ops<long*>();

test_gh_1497();
test_gh_4472();
}

#endif // ^^^ run test ^^^