Skip to content

[libcxx] Fix incorrect type in the has-1024-bit-atomics feature test #84904

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

Merged
merged 1 commit into from
Mar 12, 2024

Conversation

amilendra
Copy link
Contributor

No description provided.

@amilendra amilendra requested a review from a team as a code owner March 12, 2024 12:41
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Mar 12, 2024
@llvmbot
Copy link
Member

llvmbot commented Mar 12, 2024

@llvm/pr-subscribers-libcxx

Author: None (amilendra)

Changes

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

1 Files Affected:

  • (modified) libcxx/utils/libcxx/test/features.py (+1-1)
diff --git a/libcxx/utils/libcxx/test/features.py b/libcxx/utils/libcxx/test/features.py
index 4fd8798b794a1e..872bff372b3dba 100644
--- a/libcxx/utils/libcxx/test/features.py
+++ b/libcxx/utils/libcxx/test/features.py
@@ -176,7 +176,7 @@ def _getAndroidDeviceApi(cfg):
             cfg,
             """
             #include <atomic>
-            struct Large { int storage[1024/8]; };
+            struct Large { char storage[1024/8]; };
             std::atomic<Large> x;
             int main(int, char**) { (void)x.load(); (void)x.is_lock_free(); return 0; }
           """,

amilendra referenced this pull request Mar 12, 2024
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.
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.

Oops! Thanks for noticing.

@ldionne ldionne merged commit 42ecccf into llvm:main Mar 12, 2024
@amilendra amilendra deleted the fix-has-1024-bit-atomics-typo branch March 16, 2024 21:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants