[libc++] Refactor bitset::{any, all}

[winner245]

This patch refactors the all() and any() methods in bitset to eliminate redundant code while preserving the performance. Code generation remains unchanged, as verified on Compiler Explorer.

[llvmbot]

@llvm/pr-subscribers-libcxx

Author: Peng Liu (winner245)

Changes

The any() and all() member functions of bitset are implemented using the same bitwise logic as std::find() with __bit_iterator optimizations. Therefore, we can simplify any() and all() to one line by directly calling std::find, and inline them within the class.

---

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

1 Files Affected:

• (modified) libcxx/include/bitset (+8-48)

diff --git a/libcxx/include/bitset b/libcxx/include/bitset
index f905b6f274e3f..8d0a5ec67a748 100644
--- a/libcxx/include/bitset
+++ b/libcxx/include/bitset
@@ -224,8 +224,12 @@ protected:
     return to_ullong(integral_constant < bool, _Size< sizeof(unsigned long long) * CHAR_BIT>());
   }
 
-  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 bool all() const _NOEXCEPT;
-  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 bool any() const _NOEXCEPT;
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 bool all() const _NOEXCEPT {
+    return std::find(__make_iter(0), __make_iter(_Size), false) == __make_iter(_Size);
+  }
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 bool any() const _NOEXCEPT {
+    return std::find(__make_iter(0), __make_iter(_Size), true) != __make_iter(_Size);
+  }
   _LIBCPP_HIDE_FROM_ABI size_t __hash_code() const _NOEXCEPT;
 
 private:
@@ -388,40 +392,6 @@ __bitset<_N_words, _Size>::to_ullong(true_type, true_type) const {
   return __r;
 }
 
-template <size_t _N_words, size_t _Size>
-_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 bool __bitset<_N_words, _Size>::all() const _NOEXCEPT {
-  // do middle whole words
-  size_t __n                  = _Size;
-  __const_storage_pointer __p = __first_;
-  for (; __n >= __bits_per_word; ++__p, __n -= __bits_per_word)
-    if (~*__p)
-      return false;
-  // do last partial word
-  if (__n > 0) {
-    __storage_type __m = ~__storage_type(0) >> (__bits_per_word - __n);
-    if (~*__p & __m)
-      return false;
-  }
-  return true;
-}
-
-template <size_t _N_words, size_t _Size>
-_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 bool __bitset<_N_words, _Size>::any() const _NOEXCEPT {
-  // do middle whole words
-  size_t __n                  = _Size;
-  __const_storage_pointer __p = __first_;
-  for (; __n >= __bits_per_word; ++__p, __n -= __bits_per_word)
-    if (*__p)
-      return true;
-  // do last partial word
-  if (__n > 0) {
-    __storage_type __m = ~__storage_type(0) >> (__bits_per_word - __n);
-    if (*__p & __m)
-      return true;
-  }
-  return false;
-}
-
 template <size_t _N_words, size_t _Size>
 inline size_t __bitset<_N_words, _Size>::__hash_code() const _NOEXCEPT {
   size_t __h = 0;
@@ -713,8 +683,8 @@ public:
   _LIBCPP_HIDE_FROM_ABI bool operator!=(const bitset& __rhs) const _NOEXCEPT;
 #  endif
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 bool test(size_t __pos) const;
-  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 bool all() const _NOEXCEPT;
-  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 bool any() const _NOEXCEPT;
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 bool all() const _NOEXCEPT { return __base::all(); }
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 bool any() const _NOEXCEPT { return __base::any(); }
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 bool none() const _NOEXCEPT { return !any(); }
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 bitset operator<<(size_t __pos) const _NOEXCEPT;
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 bitset operator>>(size_t __pos) const _NOEXCEPT;
@@ -901,16 +871,6 @@ _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 bool bitset<_Size>::test(siz
   return (*this)[__pos];
 }
 
-template <size_t _Size>
-inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 bool bitset<_Size>::all() const _NOEXCEPT {
-  return __base::all();
-}
-
-template <size_t _Size>
-inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 bool bitset<_Size>::any() const _NOEXCEPT {
-  return __base::any();
-}
-
 template <size_t _Size>
 inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 bitset<_Size>
 bitset<_Size>::operator<<(size_t __pos) const _NOEXCEPT {

[ldionne]

LGTM but can you please confirm that the codegen is the same?

[winner245]

In terms of codegen (https://godbolt.org/z/cdEbbTTnY at -O2), I found that the previous implementation generates more efficient assembly as it directly checks the underlying storage. In contrast, my proposed implementation introduces function calls to std::__find and __bit_iterator, resulting in additional stack operations. This indicates that the original implementation has advantages in terms of performance. I will explore ways to refactor it while maintaining efficiency.

[ldionne]

Requesting changes since this degrades codegen.

[winner245]

The new refactoring yields exactly the same code in terms of codegen, as verified on https://godbolt.org/z/xx8hb4sPM.

[ldionne]

Thanks for the refactoring! I have a minor suggestion but LGTM otherwise.

[ldionne]

I wouldn't block this patch on it, however it might be nice to consider a follow-up refactoring.

We have a __bit_not in valarray here: https://github.com/llvm/llvm-project/blob/main/libcxx/include/valarray#L491

We could provide a single __bit_not in https://github.com/llvm/llvm-project/blob/main/libcxx/include/__functional/operations.h#L220 and use it from both valarray and bitset.

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.