Skip to content

Conversation

@hpoussin
Copy link
Contributor

@hpoussin hpoussin commented Jul 7, 2025

As it only depends of pointer size, use _WIN64 define to simplify conditions.

As it only depends of pointer size, use _WIN64 define to simplify conditions.
@hpoussin hpoussin requested a review from a team as a code owner July 7, 2025 20:13
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Jul 7, 2025
@llvmbot
Copy link
Member

llvmbot commented Jul 7, 2025

@llvm/pr-subscribers-libcxx

Author: Hervé Poussineau (hpoussin)

Changes

As it only depends of pointer size, use _WIN64 define to simplify conditions.

Original idea by @mstorsjo on #144272 (comment)


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

2 Files Affected:

  • (modified) libcxx/include/__cxx03/__thread/support/windows.h (+2-4)
  • (modified) libcxx/include/__thread/support/windows.h (+2-4)
diff --git a/libcxx/include/__cxx03/__thread/support/windows.h b/libcxx/include/__cxx03/__thread/support/windows.h
index 45252a57efaec..f9facbeefe4be 100644
--- a/libcxx/include/__cxx03/__thread/support/windows.h
+++ b/libcxx/include/__cxx03/__thread/support/windows.h
@@ -28,12 +28,10 @@ using __libcpp_timespec_t = ::timespec;
 typedef void* __libcpp_mutex_t;
 #define _LIBCPP_MUTEX_INITIALIZER 0
 
-#if defined(_M_IX86) || defined(__i386__) || defined(_M_ARM) || defined(__arm__)
-typedef void* __libcpp_recursive_mutex_t[6];
-#elif defined(_M_AMD64) || defined(__x86_64__) || defined(_M_ARM64) || defined(__aarch64__)
+#if defined(_WIN64)
 typedef void* __libcpp_recursive_mutex_t[5];
 #else
-#  error Unsupported architecture
+typedef void* __libcpp_recursive_mutex_t[6];
 #endif
 
 _LIBCPP_EXPORTED_FROM_ABI int __libcpp_recursive_mutex_init(__libcpp_recursive_mutex_t* __m);
diff --git a/libcxx/include/__thread/support/windows.h b/libcxx/include/__thread/support/windows.h
index 5dc4fa14f45b6..2921ed900e716 100644
--- a/libcxx/include/__thread/support/windows.h
+++ b/libcxx/include/__thread/support/windows.h
@@ -28,12 +28,10 @@ using __libcpp_timespec_t = ::timespec;
 typedef void* __libcpp_mutex_t;
 #define _LIBCPP_MUTEX_INITIALIZER 0
 
-#if defined(_M_IX86) || defined(__i386__) || defined(_M_ARM) || defined(__arm__)
-typedef void* __libcpp_recursive_mutex_t[6];
-#elif defined(_M_AMD64) || defined(__x86_64__) || defined(_M_ARM64) || defined(__aarch64__)
+#if defined(_WIN64)
 typedef void* __libcpp_recursive_mutex_t[5];
 #else
-#  error Unsupported architecture
+typedef void* __libcpp_recursive_mutex_t[6];
 #endif
 
 _LIBCPP_EXPORTED_FROM_ABI int __libcpp_recursive_mutex_init(__libcpp_recursive_mutex_t* __m);

@mstorsjo
Copy link
Member

mstorsjo commented Jul 7, 2025

This looks reasonable to me, thanks! Btw, I'd prefer if you'd edit the commit message (the PR description) to omit the user name mention there (even though I appreciate the credit reference) - see https://discourse.llvm.org/t/forbidding-username-in-commits/86997.

@hpoussin
Copy link
Contributor Author

hpoussin commented Jul 7, 2025

This looks reasonable to me, thanks! Btw, I'd prefer if you'd edit the commit message (the PR description) to omit the user name mention there (even though I appreciate the credit reference) - see https://discourse.llvm.org/t/forbidding-username-in-commits/86997.

Removed your username in PR description.
It was not present in commit message.

Copy link
Contributor

@philnik777 philnik777 left a comment

Choose a reason for hiding this comment

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

Mostly looks good. I like this approach a lot more than the old one. This doesn't mean we'll ever want to officially support a platform that hasn't been supported for like two decades, but I don't see a reason to not simplify our code. If that happens to make libc++ work on some platform I don't want to stay in the way of that.

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.

LGTM but I'd like @mstorsjo to stamp this.

@ldionne
Copy link
Member

ldionne commented Jul 9, 2025

Ah, nevermind, @mstorsjo already saw this and was OK with the overall goal. Merging.

@ldionne ldionne merged commit 1431f8f into llvm:main Jul 9, 2025
98 of 101 checks passed
@hpoussin hpoussin deleted the libcxx-3 branch July 28, 2025 17:43
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.

5 participants