Skip to content

Conversation

@ldionne
Copy link
Member

@ldionne ldionne commented Nov 22, 2024

This should reduce the preprocessed size of the atomic header and other headers in the synchronization library.

This should reduce the preprocessed size of the atomic header and other
headers in the synchronization library.
@ldionne ldionne requested a review from a team as a code owner November 22, 2024 08:35
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Nov 22, 2024
@llvmbot
Copy link
Member

llvmbot commented Nov 22, 2024

@llvm/pr-subscribers-libcxx

Author: Louis Dionne (ldionne)

Changes

This should reduce the preprocessed size of the atomic header and other headers in the synchronization library.


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

1 Files Affected:

  • (modified) libcxx/include/__atomic/atomic.h (+4-3)
diff --git a/libcxx/include/__atomic/atomic.h b/libcxx/include/__atomic/atomic.h
index ae0475693f22b4..d83719c8733d7e 100644
--- a/libcxx/include/__atomic/atomic.h
+++ b/libcxx/include/__atomic/atomic.h
@@ -16,7 +16,6 @@
 #include <__atomic/memory_order.h>
 #include <__config>
 #include <__cstddef/ptrdiff_t.h>
-#include <__functional/operations.h>
 #include <__memory/addressof.h>
 #include <__type_traits/enable_if.h>
 #include <__type_traits/is_floating_point.h>
@@ -376,7 +375,8 @@ struct atomic<_Tp> : __atomic_base<_Tp> {
     auto __builtin_op = [](auto __a, auto __builtin_operand, auto __order) {
       return std::__cxx_atomic_fetch_add(__a, __builtin_operand, __order);
     };
-    return __rmw_op(std::forward<_This>(__self), __operand, __m, std::plus<>{}, __builtin_op);
+    auto __plus = [](auto __a, auto __b) { return __a + __b; };
+    return __rmw_op(std::forward<_This>(__self), __operand, __m, __plus, __builtin_op);
   }
 
   template <class _This>
@@ -384,7 +384,8 @@ struct atomic<_Tp> : __atomic_base<_Tp> {
     auto __builtin_op = [](auto __a, auto __builtin_operand, auto __order) {
       return std::__cxx_atomic_fetch_sub(__a, __builtin_operand, __order);
     };
-    return __rmw_op(std::forward<_This>(__self), __operand, __m, std::minus<>{}, __builtin_op);
+    auto __minus = [](auto __a, auto __b) { return __a - __b; };
+    return __rmw_op(std::forward<_This>(__self), __operand, __m, __minus, __builtin_op);
   }
 
 public:

@philnik777
Copy link
Contributor

This makes me wonder whether we should instead split up __functional/operations.h more. We already have a few of comments splitting the header into multiple categories: arithmetic, bitwise, comparison and logical operators.

@ldionne
Copy link
Member Author

ldionne commented Nov 26, 2024

@philnik777 Did this actually result in an improvement to the header sizes? If not, probably not worth pushing in that direction.

@ldionne
Copy link
Member Author

ldionne commented Dec 3, 2024

Merging since this can't hurt. If we want to split operations.h itself, we could do that separately.

@ldionne ldionne merged commit c806042 into llvm:main Dec 3, 2024
63 checks passed
@ldionne ldionne deleted the review/reduce-atomic-header-size branch December 3, 2024 21:57
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