Skip to content

[libcxx] Update 128-bit-atomics feature test #83841

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

Jump to bottom
Merged
merged 2 commits into from
Mar 11, 2024
Merged

[libcxx] Update 128-bit-atomics feature test #83841

merged 2 commits into from
Mar 11, 2024

Conversation

amilendra
Copy link
Contributor

@amilendra amilendra commented Mar 4, 2024 •
edited by ldionne
Loading

The 128-bit-atomics libcxx feature is incorrectly named because tests
that are Xfailed with it is really using int[128]. Additionally,
because toolchain support for that feature is determined based on a much
smaller size (char[16]), tests would execute incorrectly without
required toolchain support.

So, rename 128-bit-atomics as 1024-bit-atomics, and use an
appropriate type to check for the presence of the feature.

@amilendra
[libcxx] Update 128-bit-atomics feature test
b65c711 
The 128-bit-atomics libcxx feature is incorrectly named because tests that are
Xfailed with it is really using int[128]. Additionally, because toolchain
support for that feature is determined based on a much smaller size (char[16]),
tests would execute incorrectly without required toolchain support.

So, rename 128-bit-atomics as has-large-atomics. Use the type used in the actual
tests (int[128)) within the feature test as well.
@amilendra amilendra requested a review from a team as a code owner March 4, 2024 13:19
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Mar 4, 2024
@llvmbot
Copy link
Member

llvmbot commented Mar 4, 2024

@llvm/pr-subscribers-libcxx

Author: None (amilendra)

Changes

The 128-bit-atomics libcxx feature is incorrectly named because tests that are Xfailed with it is really using int[128]. Additionally, because toolchain support for that feature is determined based on a much smaller size (char[16]), tests would execute incorrectly without required toolchain support.

So, rename 128-bit-atomics as has-large-atomics. Use the type used in the actual tests (int[128]) within the feature test as well.

Full diff: https://github.com/llvm/llvm-project/pull/83841.diff

18 Files Affected:

  • (modified) libcxx/test/libcxx/atomics/atomics.align/align.pass.cpp (+1-1)
  • (modified) libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_compare_exchange_strong.pass.cpp (+1-1)
  • (modified) libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_compare_exchange_strong_explicit.pass.cpp (+1-1)
  • (modified) libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_compare_exchange_weak.pass.cpp (+1-1)
  • (modified) libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_compare_exchange_weak_explicit.pass.cpp (+1-1)
  • (modified) libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_exchange.pass.cpp (+1-1)
  • (modified) libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_exchange_explicit.pass.cpp (+1-1)
  • (modified) libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_init.pass.cpp (+1-1)
  • (modified) libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_is_lock_free.pass.cpp (+1-1)
  • (modified) libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_load.pass.cpp (+1-1)
  • (modified) libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_load_explicit.pass.cpp (+1-1)
  • (modified) libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_store.pass.cpp (+1-1)
  • (modified) libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_store_explicit.pass.cpp (+1-1)
  • (modified) libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.wait/atomic_notify_all.pass.cpp (+1-1)
  • (modified) libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.wait/atomic_notify_one.pass.cpp (+1-1)
  • (modified) libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.wait/atomic_wait.pass.cpp (+1-1)
  • (modified) libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.wait/atomic_wait_explicit.pass.cpp (+1-1)
  • (modified) libcxx/utils/libcxx/test/features.py (+2-2)
diff --git a/libcxx/test/libcxx/atomics/atomics.align/align.pass.cpp b/libcxx/test/libcxx/atomics/atomics.align/align.pass.cpp
index e5cafde467603f..5f74d121b28597 100644
--- a/libcxx/test/libcxx/atomics/atomics.align/align.pass.cpp
+++ b/libcxx/test/libcxx/atomics/atomics.align/align.pass.cpp
@@ -7,7 +7,7 @@
 //===----------------------------------------------------------------------===//
 //
 // UNSUPPORTED: c++03
-// REQUIRES: has-128-bit-atomics
+// REQUIRES: has-large-atomics
 // ADDITIONAL_COMPILE_FLAGS: -Wno-psabi
 // ... since C++20 std::__atomic_base initializes, so we get a warning about an
 // ABI change for vector variants since the constructor code for that is
diff --git a/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_compare_exchange_strong.pass.cpp b/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_compare_exchange_strong.pass.cpp
index 1f0f61ed3e6ea9..18ef06894d1609 100644
--- a/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_compare_exchange_strong.pass.cpp
+++ b/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_compare_exchange_strong.pass.cpp
@@ -6,7 +6,7 @@
 //
 //===----------------------------------------------------------------------===//
 
-// XFAIL: !has-128-bit-atomics
+// XFAIL: !has-large-atomics
 
 // <atomic>
 
diff --git a/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_compare_exchange_strong_explicit.pass.cpp b/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_compare_exchange_strong_explicit.pass.cpp
index 0b6fcacb3d66d1..d238aa473ab5fb 100644
--- a/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_compare_exchange_strong_explicit.pass.cpp
+++ b/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_compare_exchange_strong_explicit.pass.cpp
@@ -6,7 +6,7 @@
 //
 //===----------------------------------------------------------------------===//
 
-// XFAIL: !has-128-bit-atomics
+// XFAIL: !has-large-atomics
 
 // <atomic>
 
diff --git a/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_compare_exchange_weak.pass.cpp b/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_compare_exchange_weak.pass.cpp
index 5de2f519ea4351..70d5a56ed31cbf 100644
--- a/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_compare_exchange_weak.pass.cpp
+++ b/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_compare_exchange_weak.pass.cpp
@@ -6,7 +6,7 @@
 //
 //===----------------------------------------------------------------------===//
 
-// XFAIL: !has-128-bit-atomics
+// XFAIL: !has-large-atomics
 
 // <atomic>
 
diff --git a/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_compare_exchange_weak_explicit.pass.cpp b/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_compare_exchange_weak_explicit.pass.cpp
index fc0ad8a10acd16..6d9e5dd4aca733 100644
--- a/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_compare_exchange_weak_explicit.pass.cpp
+++ b/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_compare_exchange_weak_explicit.pass.cpp
@@ -6,7 +6,7 @@
 //
 //===----------------------------------------------------------------------===//
 
-// XFAIL: !has-128-bit-atomics
+// XFAIL: !has-large-atomics
 
 // <atomic>
 
diff --git a/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_exchange.pass.cpp b/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_exchange.pass.cpp
index 31cd316e023a34..e6a94ab65b4415 100644
--- a/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_exchange.pass.cpp
+++ b/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_exchange.pass.cpp
@@ -6,7 +6,7 @@
 //
 //===----------------------------------------------------------------------===//
 
-// XFAIL: !has-128-bit-atomics
+// XFAIL: !has-large-atomics
 
 // <atomic>
 
diff --git a/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_exchange_explicit.pass.cpp b/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_exchange_explicit.pass.cpp
index 834a811c643428..43e0c70b6b445e 100644
--- a/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_exchange_explicit.pass.cpp
+++ b/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_exchange_explicit.pass.cpp
@@ -6,7 +6,7 @@
 //
 //===----------------------------------------------------------------------===//
 
-// XFAIL: !has-128-bit-atomics
+// XFAIL: !has-large-atomics
 
 // <atomic>
 
diff --git a/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_init.pass.cpp b/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_init.pass.cpp
index 4eced1d2b7f37a..c5041f52d73093 100644
--- a/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_init.pass.cpp
+++ b/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_init.pass.cpp
@@ -6,7 +6,7 @@
 //
 //===----------------------------------------------------------------------===//
 
-// XFAIL: !has-128-bit-atomics
+// XFAIL: !has-large-atomics
 // ADDITIONAL_COMPILE_FLAGS: -D_LIBCPP_DISABLE_DEPRECATION_WARNINGS
 
 // <atomic>
diff --git a/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_is_lock_free.pass.cpp b/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_is_lock_free.pass.cpp
index 1a3b8393d8f9f5..b75dd7d3b324c4 100644
--- a/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_is_lock_free.pass.cpp
+++ b/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_is_lock_free.pass.cpp
@@ -6,7 +6,7 @@
 //
 //===----------------------------------------------------------------------===//
 
-// XFAIL: !has-128-bit-atomics
+// XFAIL: !has-large-atomics
 
 // <atomic>
 
diff --git a/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_load.pass.cpp b/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_load.pass.cpp
index 5bb2bb2b614f99..80dc32363a28f7 100644
--- a/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_load.pass.cpp
+++ b/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_load.pass.cpp
@@ -6,7 +6,7 @@
 //
 //===----------------------------------------------------------------------===//
 
-// XFAIL: !has-128-bit-atomics
+// XFAIL: !has-large-atomics
 
 // <atomic>
 
diff --git a/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_load_explicit.pass.cpp b/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_load_explicit.pass.cpp
index ecb27a261eb659..2d12019f449782 100644
--- a/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_load_explicit.pass.cpp
+++ b/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_load_explicit.pass.cpp
@@ -6,7 +6,7 @@
 //
 //===----------------------------------------------------------------------===//
 
-// XFAIL: !has-128-bit-atomics
+// XFAIL: !has-large-atomics
 
 // <atomic>
 
diff --git a/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_store.pass.cpp b/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_store.pass.cpp
index 25a845e9e1f8ff..19677b4ef5c503 100644
--- a/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_store.pass.cpp
+++ b/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_store.pass.cpp
@@ -6,7 +6,7 @@
 //
 //===----------------------------------------------------------------------===//
 
-// XFAIL: !has-128-bit-atomics
+// XFAIL: !has-large-atomics
 
 // <atomic>
 
diff --git a/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_store_explicit.pass.cpp b/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_store_explicit.pass.cpp
index d22657237327f0..146ceb0b9fffb8 100644
--- a/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_store_explicit.pass.cpp
+++ b/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_store_explicit.pass.cpp
@@ -6,7 +6,7 @@
 //
 //===----------------------------------------------------------------------===//
 
-// XFAIL: !has-128-bit-atomics
+// XFAIL: !has-large-atomics
 
 // <atomic>
 
diff --git a/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.wait/atomic_notify_all.pass.cpp b/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.wait/atomic_notify_all.pass.cpp
index 93ed607d413b27..aa188e1d133463 100644
--- a/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.wait/atomic_notify_all.pass.cpp
+++ b/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.wait/atomic_notify_all.pass.cpp
@@ -8,7 +8,7 @@
 //
 // UNSUPPORTED: no-threads
 // XFAIL: c++03
-// XFAIL: !has-128-bit-atomics
+// XFAIL: !has-large-atomics
 
 // XFAIL: availability-synchronization_library-missing
 
diff --git a/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.wait/atomic_notify_one.pass.cpp b/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.wait/atomic_notify_one.pass.cpp
index ad48ef1441f475..92d4e142b13580 100644
--- a/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.wait/atomic_notify_one.pass.cpp
+++ b/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.wait/atomic_notify_one.pass.cpp
@@ -8,7 +8,7 @@
 //
 // UNSUPPORTED: no-threads
 // XFAIL: c++03
-// XFAIL: !has-128-bit-atomics
+// XFAIL: !has-large-atomics
 
 // XFAIL: availability-synchronization_library-missing
 
diff --git a/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.wait/atomic_wait.pass.cpp b/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.wait/atomic_wait.pass.cpp
index 449e50fa12b5f4..79db80a1e38e3e 100644
--- a/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.wait/atomic_wait.pass.cpp
+++ b/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.wait/atomic_wait.pass.cpp
@@ -8,7 +8,7 @@
 //
 // UNSUPPORTED: no-threads
 // XFAIL: c++03
-// XFAIL: !has-128-bit-atomics
+// XFAIL: !has-large-atomics
 
 // XFAIL: availability-synchronization_library-missing
 
diff --git a/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.wait/atomic_wait_explicit.pass.cpp b/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.wait/atomic_wait_explicit.pass.cpp
index a6ee4fc6327975..c2668dea59fcc9 100644
--- a/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.wait/atomic_wait_explicit.pass.cpp
+++ b/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.wait/atomic_wait_explicit.pass.cpp
@@ -8,7 +8,7 @@
 //
 // UNSUPPORTED: no-threads
 // XFAIL: c++03
-// XFAIL: !has-128-bit-atomics
+// XFAIL: !has-large-atomics
 
 // XFAIL: availability-synchronization_library-missing
 
diff --git a/libcxx/utils/libcxx/test/features.py b/libcxx/utils/libcxx/test/features.py
index 6ef40755c59d97..d2dfb747a2b2c3 100644
--- a/libcxx/utils/libcxx/test/features.py
+++ b/libcxx/utils/libcxx/test/features.py
@@ -195,12 +195,12 @@ def _getAndroidDeviceApi(cfg):
         ),
     ),
     Feature(
-        name="has-128-bit-atomics",
+        name="has-large-atomics",
         when=lambda cfg: sourceBuilds(
             cfg,
             """
             #include <atomic>
-            struct Large { char storage[128/8]; };
+            struct Large { int storage[128]; };
             std::atomic<Large> x;
             int main(int, char**) { (void)x.load(); (void)x.is_lock_free(); return 0; }
           """,

ldionne
ldionne requested changes Mar 7, 2024
View reviewed changes
Copy link
Member

@ldionne ldionne left a comment

Choose a reason for hiding this comment

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

Thanks for noticing this! We used to call it large-atomics and we moved to specific sizes instead because that's what we needed to convey in the test suite.

Instead, I would simply rename it to has-1024-bit-atomics and use char[1024/8] in the test. WDYT?

@amilendra
Copy link
Contributor Author

Thanks for noticing this! We used to call it large-atomics and we moved to specific sizes instead because that's what we needed to convey in the test suite.

Instead, I would simply rename it to has-1024-bit-atomics and use char[1024/8] in the test. WDYT?

Makes sense. Done.

@amilendra amilendra requested a review from ldionne March 11, 2024 10:38
@ldionne
Copy link
Member

ldionne commented Mar 11, 2024

Thanks! I will merge this. However, can you please disable the "hide my email" feature in Github so we have a regular email address associated to this commit?

@amilendra
Copy link
Contributor Author

Thanks! I will merge this. However, can you please disable the "hide my email" feature in Github so we have a regular email address associated to this commit?

Done. I think my work email address should be visible now.

@ldionne ldionne merged commit 6aef8df into llvm:main Mar 11, 2024
@ldionne
Copy link
Member

ldionne commented Mar 11, 2024

Thank you!

@amilendra amilendra deleted the rename-128-bit-atomics branch March 11, 2024 20:19
@StephanTLavavej StephanTLavavej mentioned this pull request May 31, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ldionne ldionne ldionne approved these changes

Assignees
No one assigned
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@amilendra @llvmbot @ldionne