Skip to content

[libc++] Fix inconsistency between is_lock_free and is_always_lock_free #68109

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Oct 19, 2023
Merged

[libc++] Fix inconsistency between is_lock_free and is_always_lock_free #68109

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
84ae453
[libc++] Fix inconsistency between is_lock_free and is_always_lock_free
ldionne Oct 3, 2023
0f8605f
Merge branch 'main' into review/fix-is-lock-free
ldionne Oct 17, 2023
491a3a1
Fix CI failures in C++11/03/14
ldionne Oct 18, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion libcxx/include/__atomic/atomic_base.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ struct __atomic_base // false

_LIBCPP_HIDE_FROM_ABI
bool is_lock_free() const volatile _NOEXCEPT
{return __cxx_atomic_is_lock_free(sizeof(_Tp));}
{return __cxx_atomic_is_lock_free(sizeof(__cxx_atomic_impl<_Tp>));}
_LIBCPP_HIDE_FROM_ABI
bool is_lock_free() const _NOEXCEPT
{return static_cast<__atomic_base const volatile*>(this)->is_lock_free();}
Expand Down
1 change: 1 addition & 0 deletions libcxx/test/libcxx/atomics/atomics.align/align.pass.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ int main(int, char**) {
CHECK_ALIGNMENT(struct Empty {});
CHECK_ALIGNMENT(struct OneInt { int i; });
CHECK_ALIGNMENT(struct IntArr2 { int i[2]; });
CHECK_ALIGNMENT(struct FloatArr3 { float i[3]; });
CHECK_ALIGNMENT(struct LLIArr2 { long long int i[2]; });
CHECK_ALIGNMENT(struct LLIArr4 { long long int i[4]; });
CHECK_ALIGNMENT(struct LLIArr8 { long long int i[8]; });
Expand Down
2 changes: 1 addition & 1 deletion libcxx/test/std/atomics/atomics.lockfree/isalwayslockfree.pass.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
template <typename T>
void checkAlwaysLockFree() {
if (std::atomic<T>::is_always_lock_free) {
LIBCPP_ASSERT(sizeof(std::atomic<T>) == sizeof(T)); // technically not required, but libc++ does it that way
Copy link
Member

@EricWF EricWF Oct 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume the FloatArr3 case is what causes this?
Under what architecture? I haven't been able to reproduce the size difference in godbolt.

Copy link
Member Author

@ldionne ldionne Oct 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, exactly, FloatArr3 showcases this under arm64-apple-macos (and probably any arm64 target). I looked at the assembly and we do not use a lock for FloatArr3 on arm64 so it looks like is_lock_free was really lying.

assert(std::atomic<T>().is_lock_free());
}
}
Expand Down Expand Up @@ -79,6 +78,7 @@ void run()
CHECK_ALWAYS_LOCK_FREE(struct Empty {});
CHECK_ALWAYS_LOCK_FREE(struct OneInt { int i; });
CHECK_ALWAYS_LOCK_FREE(struct IntArr2 { int i[2]; });
CHECK_ALWAYS_LOCK_FREE(struct FloatArr3 { float i[3]; });
CHECK_ALWAYS_LOCK_FREE(struct LLIArr2 { long long int i[2]; });
CHECK_ALWAYS_LOCK_FREE(struct LLIArr4 { long long int i[4]; });
CHECK_ALWAYS_LOCK_FREE(struct LLIArr8 { long long int i[8]; });
Expand Down
10 changes: 8 additions & 2 deletions libcxx/test/std/atomics/atomics.types.generic/address.pass.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,14 @@ do_test()
typedef typename std::remove_pointer<T>::type X;
A obj(T(0));
assert(obj == T(0));
bool b0 = obj.is_lock_free();
((void)b0); // mark as unused
{
bool lockfree = obj.is_lock_free();
(void)lockfree;
#if TEST_STD_VER >= 17
if (A::is_always_lock_free)
assert(lockfree);
#endif
}
obj.store(T(0));
assert(obj == T(0));
obj.store(T(1), std::memory_order_release);
Expand Down
30 changes: 24 additions & 6 deletions libcxx/test/std/atomics/atomics.types.generic/bool.pass.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,14 @@ int main(int, char**)
{
volatile std::atomic<bool> obj(true);
assert(obj == true);
bool b0 = obj.is_lock_free();
(void)b0; // to placate scan-build
{
bool lockfree = obj.is_lock_free();
(void)lockfree;
#if TEST_STD_VER >= 17
if (std::atomic<bool>::is_always_lock_free)
assert(lockfree);
#endif
}
obj.store(false);
assert(obj == false);
obj.store(true, std::memory_order_release);
Expand Down Expand Up @@ -112,8 +118,14 @@ int main(int, char**)
{
std::atomic<bool> obj(true);
assert(obj == true);
bool b0 = obj.is_lock_free();
(void)b0; // to placate scan-build
{
bool lockfree = obj.is_lock_free();
(void)lockfree;
#if TEST_STD_VER >= 17
if (std::atomic<bool>::is_always_lock_free)
assert(lockfree);
#endif
}
obj.store(false);
assert(obj == false);
obj.store(true, std::memory_order_release);
Expand Down Expand Up @@ -163,8 +175,14 @@ int main(int, char**)
{
std::atomic_bool obj(true);
assert(obj == true);
bool b0 = obj.is_lock_free();
(void)b0; // to placate scan-build
{
bool lockfree = obj.is_lock_free();
(void)lockfree;
#if TEST_STD_VER >= 17
if (std::atomic_bool::is_always_lock_free)
assert(lockfree);
#endif
}
obj.store(false);
assert(obj == false);
obj.store(true, std::memory_order_release);
Expand Down
10 changes: 8 additions & 2 deletions libcxx/test/std/atomics/atomics.types.generic/integral.pass.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,14 @@ do_test()
{
A obj(T(0));
assert(obj == T(0));
bool b0 = obj.is_lock_free();
((void)b0); // mark as unused
{
bool lockfree = obj.is_lock_free();
(void)lockfree;
#if TEST_STD_VER >= 17
if (A::is_always_lock_free)
assert(lockfree);
#endif
}
obj.store(T(0));
assert(obj == T(0));
obj.store(T(1), std::memory_order_release);
Expand Down
4 changes: 4 additions & 0 deletions ...tomics/atomics.types.operations/atomics.types.operations.req/atomic_is_lock_free.pass.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,12 @@ struct TestFn {
void operator()() const {
typedef std::atomic<T> A;
T t = T();

A a(t);
bool b1 = std::atomic_is_lock_free(static_cast<const A*>(&a));
if (A::is_always_lock_free)
assert(b1);

volatile A va(t);
bool b2 = std::atomic_is_lock_free(static_cast<const volatile A*>(&va));
assert(b1 == b2);
Expand Down