[libc++] Refactor std::{fill_n, count} for vector<bool>::iterator #120305

winner245 · 2024-12-17T21:30:54Z

This PR refactors the vector<bool>::iterator overloads of std::fill_n and std::count by simplifying the conditional code structure:

if (x)
  do<true>(...);
else
  do<false>(...);

to an unconditional call:

do(..., x);

github-actions · 2024-12-17T21:34:26Z

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

llvmbot · 2025-01-10T19:45:35Z

@llvm/pr-subscribers-libcxx

Author: Peng Liu (winner245)

Changes

This PR refactors the vector<bool>::iterator overloads of std::fill_n and std::count by simplifying the conditional code structure:

if (x)
  do&lt;true&gt;(...);
else
  do&lt;false&gt;(...);

to an unconditional call:

do(..., x);

Patch is 21.32 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/120305.diff

10 Files Affected:

(modified) libcxx/include/__algorithm/count.h (+6-8)
(modified) libcxx/include/__algorithm/fill_n.h (+7-11)
(modified) libcxx/include/__algorithm/find.h (+6-8)
(modified) libcxx/include/__bit/invert_if.h (+3-3)
(modified) libcxx/include/__bit_reference (+7-7)
(modified) libcxx/include/__cxx03/__algorithm/count.h (+6-8)
(modified) libcxx/include/__cxx03/__algorithm/fill_n.h (+7-11)
(modified) libcxx/include/__cxx03/__algorithm/find.h (+6-8)
(modified) libcxx/include/__cxx03/__bit/invert_if.h (+3-3)
(modified) libcxx/include/__cxx03/__bit_reference (+7-7)

diff --git a/libcxx/include/__algorithm/count.h b/libcxx/include/__algorithm/count.h
index 6910b4f43e9934..a2115f744b3941 100644
--- a/libcxx/include/__algorithm/count.h
+++ b/libcxx/include/__algorithm/count.h
@@ -42,9 +42,9 @@ __count(_Iter __first, _Sent __last, const _Tp& __value, _Proj& __proj) {
 }
 
 // __bit_iterator implementation
-template <bool _ToCount, class _Cp, bool _IsConst>
+template <class _Cp, bool _IsConst>
 _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 typename __bit_iterator<_Cp, _IsConst>::difference_type
-__count_bool(__bit_iterator<_Cp, _IsConst> __first, typename _Cp::size_type __n) {
+__count_bool(__bit_iterator<_Cp, _IsConst> __first, typename _Cp::size_type __n, bool __to_count) {
   using _It             = __bit_iterator<_Cp, _IsConst>;
   using __storage_type  = typename _It::__storage_type;
   using difference_type = typename _It::difference_type;
@@ -56,17 +56,17 @@ __count_bool(__bit_iterator<_Cp, _IsConst> __first, typename _Cp::size_type __n)
     __storage_type __clz_f = static_cast<__storage_type>(__bits_per_word - __first.__ctz_);
     __storage_type __dn    = std::min(__clz_f, __n);
     __storage_type __m     = (~__storage_type(0) << __first.__ctz_) & (~__storage_type(0) >> (__clz_f - __dn));
-    __r                    = std::__libcpp_popcount(std::__invert_if<!_ToCount>(*__first.__seg_) & __m);
+    __r                    = std::__libcpp_popcount(std::__invert_if(*__first.__seg_, !__to_count) & __m);
     __n -= __dn;
     ++__first.__seg_;
   }
   // do middle whole words
   for (; __n >= __bits_per_word; ++__first.__seg_, __n -= __bits_per_word)
-    __r += std::__libcpp_popcount(std::__invert_if<!_ToCount>(*__first.__seg_));
+    __r += std::__libcpp_popcount(std::__invert_if(*__first.__seg_, !__to_count));
   // do last partial word
   if (__n > 0) {
     __storage_type __m = ~__storage_type(0) >> (__bits_per_word - __n);
-    __r += std::__libcpp_popcount(std::__invert_if<!_ToCount>(*__first.__seg_) & __m);
+    __r += std::__libcpp_popcount(std::__invert_if(*__first.__seg_, !__to_count) & __m);
   }
   return __r;
 }
@@ -74,9 +74,7 @@ __count_bool(__bit_iterator<_Cp, _IsConst> __first, typename _Cp::size_type __n)
 template <class, class _Cp, bool _IsConst, class _Tp, class _Proj, __enable_if_t<__is_identity<_Proj>::value, int> = 0>
 _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 __iter_diff_t<__bit_iterator<_Cp, _IsConst> >
 __count(__bit_iterator<_Cp, _IsConst> __first, __bit_iterator<_Cp, _IsConst> __last, const _Tp& __value, _Proj&) {
-  if (__value)
-    return std::__count_bool<true>(__first, static_cast<typename _Cp::size_type>(__last - __first));
-  return std::__count_bool<false>(__first, static_cast<typename _Cp::size_type>(__last - __first));
+  return std::__count_bool(__first, static_cast<typename _Cp::size_type>(__last - __first), static_cast<bool>(__value));
 }
 
 template <class _InputIterator, class _Tp>
diff --git a/libcxx/include/__algorithm/fill_n.h b/libcxx/include/__algorithm/fill_n.h
index 5069a72783f348..f3af58f4d9664d 100644
--- a/libcxx/include/__algorithm/fill_n.h
+++ b/libcxx/include/__algorithm/fill_n.h
@@ -30,9 +30,9 @@ template <class _OutputIterator, class _Size, class _Tp>
 inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _OutputIterator
 __fill_n(_OutputIterator __first, _Size __n, const _Tp& __value);
 
-template <bool _FillVal, class _Cp>
+template <class _Cp>
 _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI void
-__fill_n_bool(__bit_iterator<_Cp, false> __first, typename _Cp::size_type __n) {
+__fill_n_bool(__bit_iterator<_Cp, false> __first, typename _Cp::size_type __n, bool __to_fill) {
   using _It            = __bit_iterator<_Cp, false>;
   using __storage_type = typename _It::__storage_type;
 
@@ -42,7 +42,7 @@ __fill_n_bool(__bit_iterator<_Cp, false> __first, typename _Cp::size_type __n) {
     __storage_type __clz_f = static_cast<__storage_type>(__bits_per_word - __first.__ctz_);
     __storage_type __dn    = std::min(__clz_f, __n);
     __storage_type __m     = (~__storage_type(0) << __first.__ctz_) & (~__storage_type(0) >> (__clz_f - __dn));
-    if (_FillVal)
+    if (__to_fill)
       *__first.__seg_ |= __m;
     else
       *__first.__seg_ &= ~__m;
@@ -51,13 +51,13 @@ __fill_n_bool(__bit_iterator<_Cp, false> __first, typename _Cp::size_type __n) {
   }
   // do middle whole words
   __storage_type __nw = __n / __bits_per_word;
-  std::__fill_n(std::__to_address(__first.__seg_), __nw, _FillVal ? static_cast<__storage_type>(-1) : 0);
+  std::__fill_n(std::__to_address(__first.__seg_), __nw, __to_fill ? static_cast<__storage_type>(-1) : 0);
   __n -= __nw * __bits_per_word;
   // do last partial word
   if (__n > 0) {
     __first.__seg_ += __nw;
     __storage_type __m = ~__storage_type(0) >> (__bits_per_word - __n);
-    if (_FillVal)
+    if (__to_fill)
       *__first.__seg_ |= __m;
     else
       *__first.__seg_ &= ~__m;
@@ -67,12 +67,8 @@ __fill_n_bool(__bit_iterator<_Cp, false> __first, typename _Cp::size_type __n) {
 template <class _Cp, class _Size>
 inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 __bit_iterator<_Cp, false>
 __fill_n(__bit_iterator<_Cp, false> __first, _Size __n, const bool& __value) {
-  if (__n > 0) {
-    if (__value)
-      std::__fill_n_bool<true>(__first, __n);
-    else
-      std::__fill_n_bool<false>(__first, __n);
-  }
+  if (__n > 0)
+    std::__fill_n_bool(__first, __n, __value);
   return __first + __n;
 }
 
diff --git a/libcxx/include/__algorithm/find.h b/libcxx/include/__algorithm/find.h
index a05d50718595ee..b16ca8fcb8fd52 100644
--- a/libcxx/include/__algorithm/find.h
+++ b/libcxx/include/__algorithm/find.h
@@ -95,9 +95,9 @@ __find(_Tp* __first, _Tp* __last, const _Up& __value, _Proj& __proj) {
 }
 
 // __bit_iterator implementation
-template <bool _ToFind, class _Cp, bool _IsConst>
+template <class _Cp, bool _IsConst>
 _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI __bit_iterator<_Cp, _IsConst>
-__find_bool(__bit_iterator<_Cp, _IsConst> __first, typename _Cp::size_type __n) {
+__find_bool(__bit_iterator<_Cp, _IsConst> __first, typename _Cp::size_type __n, bool __to_find) {
   using _It            = __bit_iterator<_Cp, _IsConst>;
   using __storage_type = typename _It::__storage_type;
 
@@ -107,7 +107,7 @@ __find_bool(__bit_iterator<_Cp, _IsConst> __first, typename _Cp::size_type __n)
     __storage_type __clz_f = static_cast<__storage_type>(__bits_per_word - __first.__ctz_);
     __storage_type __dn    = std::min(__clz_f, __n);
     __storage_type __m     = (~__storage_type(0) << __first.__ctz_) & (~__storage_type(0) >> (__clz_f - __dn));
-    __storage_type __b     = std::__invert_if<!_ToFind>(*__first.__seg_) & __m;
+    __storage_type __b     = std::__invert_if(*__first.__seg_, !__to_find) & __m;
     if (__b)
       return _It(__first.__seg_, static_cast<unsigned>(std::__libcpp_ctz(__b)));
     if (__n == __dn)
@@ -117,14 +117,14 @@ __find_bool(__bit_iterator<_Cp, _IsConst> __first, typename _Cp::size_type __n)
   }
   // do middle whole words
   for (; __n >= __bits_per_word; ++__first.__seg_, __n -= __bits_per_word) {
-    __storage_type __b = std::__invert_if<!_ToFind>(*__first.__seg_);
+    __storage_type __b = std::__invert_if(*__first.__seg_, !__to_find);
     if (__b)
       return _It(__first.__seg_, static_cast<unsigned>(std::__libcpp_ctz(__b)));
   }
   // do last partial word
   if (__n > 0) {
     __storage_type __m = ~__storage_type(0) >> (__bits_per_word - __n);
-    __storage_type __b = std::__invert_if<!_ToFind>(*__first.__seg_) & __m;
+    __storage_type __b = std::__invert_if(*__first.__seg_, !__to_find) & __m;
     if (__b)
       return _It(__first.__seg_, static_cast<unsigned>(std::__libcpp_ctz(__b)));
   }
@@ -134,9 +134,7 @@ __find_bool(__bit_iterator<_Cp, _IsConst> __first, typename _Cp::size_type __n)
 template <class _Cp, bool _IsConst, class _Tp, class _Proj, __enable_if_t<__is_identity<_Proj>::value, int> = 0>
 inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 __bit_iterator<_Cp, _IsConst>
 __find(__bit_iterator<_Cp, _IsConst> __first, __bit_iterator<_Cp, _IsConst> __last, const _Tp& __value, _Proj&) {
-  if (static_cast<bool>(__value))
-    return std::__find_bool<true>(__first, static_cast<typename _Cp::size_type>(__last - __first));
-  return std::__find_bool<false>(__first, static_cast<typename _Cp::size_type>(__last - __first));
+  return std::__find_bool(__first, static_cast<typename _Cp::size_type>(__last - __first), static_cast<bool>(__value));
 }
 
 // segmented iterator implementation
diff --git a/libcxx/include/__bit/invert_if.h b/libcxx/include/__bit/invert_if.h
index f7606ede26da00..59be02445cea8a 100644
--- a/libcxx/include/__bit/invert_if.h
+++ b/libcxx/include/__bit/invert_if.h
@@ -18,9 +18,9 @@
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
-template <bool _Invert, class _Tp>
-_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 _Tp __invert_if(_Tp __v) {
-  if (_Invert)
+template <class _Tp>
+_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 _Tp __invert_if(_Tp __v, bool __invert) {
+  if (__invert)
     return ~__v;
   return __v;
 }
diff --git a/libcxx/include/__bit_reference b/libcxx/include/__bit_reference
index 9fa24c98d493fd..0cc6892dba0627 100644
--- a/libcxx/include/__bit_reference
+++ b/libcxx/include/__bit_reference
@@ -964,9 +964,9 @@ private:
   template <class _Dp>
   friend struct __bit_array;
 
-  template <bool _FillVal, class _Dp>
+  template <class _Dp>
   _LIBCPP_CONSTEXPR_SINCE_CXX20 friend void
-  __fill_n_bool(__bit_iterator<_Dp, false> __first, typename _Dp::size_type __n);
+  __fill_n_bool(__bit_iterator<_Dp, false> __first, typename _Dp::size_type __n, bool __to_fill);
 
   template <class _Dp, bool _IC>
   _LIBCPP_CONSTEXPR_SINCE_CXX20 friend __bit_iterator<_Dp, false> __copy_aligned(
@@ -1007,12 +1007,12 @@ private:
   template <class _Dp, bool _IC1, bool _IC2>
   _LIBCPP_CONSTEXPR_SINCE_CXX20 friend bool
       equal(__bit_iterator<_Dp, _IC1>, __bit_iterator<_Dp, _IC1>, __bit_iterator<_Dp, _IC2>);
-  template <bool _ToFind, class _Dp, bool _IC>
+  template <class _Dp, bool _IC>
   _LIBCPP_CONSTEXPR_SINCE_CXX20 friend __bit_iterator<_Dp, _IC>
-      __find_bool(__bit_iterator<_Dp, _IC>, typename _Dp::size_type);
-  template <bool _ToCount, class _Dp, bool _IC>
-  friend typename __bit_iterator<_Dp, _IC>::difference_type _LIBCPP_HIDE_FROM_ABI
-  _LIBCPP_CONSTEXPR_SINCE_CXX20 __count_bool(__bit_iterator<_Dp, _IC>, typename _Dp::size_type);
+  __find_bool(__bit_iterator<_Dp, _IC>, typename _Dp::size_type, bool __to_find);
+  template <class _Dp, bool _IC>
+  friend typename __bit_iterator<_Dp, _IC>::difference_type _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20
+  __count_bool(__bit_iterator<_Dp, _IC>, typename _Dp::size_type, bool __to_count);
 };
 
 _LIBCPP_END_NAMESPACE_STD
diff --git a/libcxx/include/__cxx03/__algorithm/count.h b/libcxx/include/__cxx03/__algorithm/count.h
index 7c1fc3e5798980..e7675923d905b4 100644
--- a/libcxx/include/__cxx03/__algorithm/count.h
+++ b/libcxx/include/__cxx03/__algorithm/count.h
@@ -41,9 +41,9 @@ __count(_Iter __first, _Sent __last, const _Tp& __value, _Proj& __proj) {
 }
 
 // __bit_iterator implementation
-template <bool _ToCount, class _Cp, bool _IsConst>
+template <class _Cp, bool _IsConst>
 _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 typename __bit_iterator<_Cp, _IsConst>::difference_type
-__count_bool(__bit_iterator<_Cp, _IsConst> __first, typename _Cp::size_type __n) {
+__count_bool(__bit_iterator<_Cp, _IsConst> __first, typename _Cp::size_type __n, bool __to_count) {
   using _It             = __bit_iterator<_Cp, _IsConst>;
   using __storage_type  = typename _It::__storage_type;
   using difference_type = typename _It::difference_type;
@@ -55,17 +55,17 @@ __count_bool(__bit_iterator<_Cp, _IsConst> __first, typename _Cp::size_type __n)
     __storage_type __clz_f = static_cast<__storage_type>(__bits_per_word - __first.__ctz_);
     __storage_type __dn    = std::min(__clz_f, __n);
     __storage_type __m     = (~__storage_type(0) << __first.__ctz_) & (~__storage_type(0) >> (__clz_f - __dn));
-    __r                    = std::__libcpp_popcount(std::__invert_if<!_ToCount>(*__first.__seg_) & __m);
+    __r                    = std::__libcpp_popcount(std::__invert_if(*__first.__seg_, !__to_count) & __m);
     __n -= __dn;
     ++__first.__seg_;
   }
   // do middle whole words
   for (; __n >= __bits_per_word; ++__first.__seg_, __n -= __bits_per_word)
-    __r += std::__libcpp_popcount(std::__invert_if<!_ToCount>(*__first.__seg_));
+    __r += std::__libcpp_popcount(std::__invert_if(*__first.__seg_, !__to_count));
   // do last partial word
   if (__n > 0) {
     __storage_type __m = ~__storage_type(0) >> (__bits_per_word - __n);
-    __r += std::__libcpp_popcount(std::__invert_if<!_ToCount>(*__first.__seg_) & __m);
+    __r += std::__libcpp_popcount(std::__invert_if(*__first.__seg_, !__to_count) & __m);
   }
   return __r;
 }
@@ -73,9 +73,7 @@ __count_bool(__bit_iterator<_Cp, _IsConst> __first, typename _Cp::size_type __n)
 template <class, class _Cp, bool _IsConst, class _Tp, class _Proj, __enable_if_t<__is_identity<_Proj>::value, int> = 0>
 _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 __iter_diff_t<__bit_iterator<_Cp, _IsConst> >
 __count(__bit_iterator<_Cp, _IsConst> __first, __bit_iterator<_Cp, _IsConst> __last, const _Tp& __value, _Proj&) {
-  if (__value)
-    return std::__count_bool<true>(__first, static_cast<typename _Cp::size_type>(__last - __first));
-  return std::__count_bool<false>(__first, static_cast<typename _Cp::size_type>(__last - __first));
+  return std::__count_bool(__first, static_cast<typename _Cp::size_type>(__last - __first), static_cast<bool>(__value));
 }
 
 template <class _InputIterator, class _Tp>
diff --git a/libcxx/include/__cxx03/__algorithm/fill_n.h b/libcxx/include/__cxx03/__algorithm/fill_n.h
index 99b712c7b0360c..ac96e53cfa24ce 100644
--- a/libcxx/include/__cxx03/__algorithm/fill_n.h
+++ b/libcxx/include/__cxx03/__algorithm/fill_n.h
@@ -31,9 +31,9 @@ template <class _OutputIterator, class _Size, class _Tp>
 inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _OutputIterator
 __fill_n(_OutputIterator __first, _Size __n, const _Tp& __value);
 
-template <bool _FillVal, class _Cp>
+template <class _Cp>
 _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI void
-__fill_n_bool(__bit_iterator<_Cp, false> __first, typename _Cp::size_type __n) {
+__fill_n_bool(__bit_iterator<_Cp, false> __first, typename _Cp::size_type __n, bool __to_fill) {
   using _It            = __bit_iterator<_Cp, false>;
   using __storage_type = typename _It::__storage_type;
 
@@ -43,7 +43,7 @@ __fill_n_bool(__bit_iterator<_Cp, false> __first, typename _Cp::size_type __n) {
     __storage_type __clz_f = static_cast<__storage_type>(__bits_per_word - __first.__ctz_);
     __storage_type __dn    = std::min(__clz_f, __n);
     __storage_type __m     = (~__storage_type(0) << __first.__ctz_) & (~__storage_type(0) >> (__clz_f - __dn));
-    if (_FillVal)
+    if (__to_fill)
       *__first.__seg_ |= __m;
     else
       *__first.__seg_ &= ~__m;
@@ -52,13 +52,13 @@ __fill_n_bool(__bit_iterator<_Cp, false> __first, typename _Cp::size_type __n) {
   }
   // do middle whole words
   __storage_type __nw = __n / __bits_per_word;
-  std::__fill_n(std::__to_address(__first.__seg_), __nw, _FillVal ? static_cast<__storage_type>(-1) : 0);
+  std::__fill_n(std::__to_address(__first.__seg_), __nw, __to_fill ? static_cast<__storage_type>(-1) : 0);
   __n -= __nw * __bits_per_word;
   // do last partial word
   if (__n > 0) {
     __first.__seg_ += __nw;
     __storage_type __m = ~__storage_type(0) >> (__bits_per_word - __n);
-    if (_FillVal)
+    if (__to_fill)
       *__first.__seg_ |= __m;
     else
       *__first.__seg_ &= ~__m;
@@ -68,12 +68,8 @@ __fill_n_bool(__bit_iterator<_Cp, false> __first, typename _Cp::size_type __n) {
 template <class _Cp, class _Size>
 inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 __bit_iterator<_Cp, false>
 __fill_n(__bit_iterator<_Cp, false> __first, _Size __n, const bool& __value) {
-  if (__n > 0) {
-    if (__value)
-      std::__fill_n_bool<true>(__first, __n);
-    else
-      std::__fill_n_bool<false>(__first, __n);
-  }
+  if (__n > 0)
+    std::__fill_n_bool(__first, __n, __value);
   return __first + __n;
 }
 
diff --git a/libcxx/include/__cxx03/__algorithm/find.h b/libcxx/include/__cxx03/__algorithm/find.h
index ff5ac9b8b1bd0a..23263da2dd1e84 100644
--- a/libcxx/include/__cxx03/__algorithm/find.h
+++ b/libcxx/include/__cxx03/__algorithm/find.h
@@ -94,9 +94,9 @@ __find(_Tp* __first, _Tp* __last, const _Up& __value, _Proj& __proj) {
 }
 
 // __bit_iterator implementation
-template <bool _ToFind, class _Cp, bool _IsConst>
+template <class _Cp, bool _IsConst>
 _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI __bit_iterator<_Cp, _IsConst>
-__find_bool(__bit_iterator<_Cp, _IsConst> __first, typename _Cp::size_type __n) {
+__find_bool(__bit_iterator<_Cp, _IsConst> __first, typename _Cp::size_type __n, bool __to_find) {
   using _It            = __bit_iterator<_Cp, _IsConst>;
   using __storage_type = typename _It::__storage_type;
 
@@ -106,7 +106,7 @@ __find_bool(__bit_iterator<_Cp, _IsConst> __first, typename _Cp::size_type __n)
     __storage_type __clz_f = static_cast<__storage_type>(__bits_per_word - __first.__ctz_);
     __storage_type __dn    = std::min(__clz_f, __n);
     __storage_type __m     = (~__storage_type(0) << __first.__ctz_) & (~__storage_type(0) >> (__clz_f - __dn));
-    __storage_type __b     = std::__invert_if<!_ToFind>(*__first.__seg_) & __m;
+    __storage_type __b     = std::__invert_if(*__first.__seg_, !__to_find) & __m;
     if (__b)
       return _It(__first.__seg_, static_cast<unsigned>(std::__libcpp_ctz(__b)));
     if (__n == __dn)
@@ -116,14 +116,14 @@ __find_bool(__bit_iterator<_Cp, _IsConst> __first, typename _Cp::size_type __n)
   }
   // do middle whole words
   for (; __n >= __bits_per_word; ++__first.__seg_, __n -= __bits_per_word) {
-    __storage_type __b = std::__invert_if<!_ToFind>(*__first.__seg_);
+    __storage_type __b = std::__invert_if(*__first.__seg_, !__to_find);
     if (__b)
       return _It(__first.__seg_, static_cast<unsigned>(std::__libcpp_ctz(__b)));
   }
   // do last partial word
   if (__n > 0) {
     __storage_type __m = ~__storage_type(0) >> (__bits_per_word - __n);
-    __storage_type __b = std::__invert_if<!_ToFind>(*__first.__seg_) & __m;
+    __storage_type __b = std::__invert_if(*__first.__seg_, !__to_find) & __m;
     if (__b)
       return _It(__first.__seg_, static_cast<unsigned>(std::__libcpp_ctz(__b)));
   }
@@ -133,9 +133,7 @@ __find_bool(__bit_iterator<_Cp, _IsConst> __first, typename _Cp::size_type __n)
 template <class _Cp, bool _IsConst, class _Tp, class _Proj, __enable_if_t<__is_identity<_Proj>::value, int> = 0>
 inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 __bit_iterator<_Cp, _IsConst>
 __find(__bit_iterator<_Cp, _IsConst> __first, __bit_iterator<_Cp, _IsConst> __last, const _Tp& __value, _Proj&) {
-  if (static_cast<bool>(__value))
-    return std::__find_bool<true>(__first, static_cast<typename _Cp::size_type>(__last - __first));
-  return std::__find_bool<false>(__first, static_cast<typename _Cp::size_type>(__last - __first));
+  return std::__find_bool(__first, static_cast<typename _Cp::size_type>(__last - __first), static_cast<bool>(__value));
 }
 
 // segmented iterator implementation
diff --git a/libcxx/include/__cxx03/__bit/invert_if.h b/libcxx/include/__cxx03/__bit/invert_if.h
index b111d702ea7550..46e16728ca494e 100644
--- a/libcxx/include/__cxx03/__bit/invert_if.h
+++ b/libcxx/include/__cxx03/__bit/invert_if.h
@@ -18,9 +18,9 @@
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
-template <bool _Invert, class _Tp>
-_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 _Tp __invert_if(_Tp __v) {
-  if (_Invert)
+template <class _Tp>
+_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 _Tp __invert_if(_Tp __v, bool __invert) {
+  if (__invert)
     return ~__v;
   return __v;
 }
diff --git a/libcxx/include/__cxx03/__bit_reference b/libcxx/include/__cxx03/__bit_reference
index bf86f9a76e24a1..a0d7fbf4a1e561 100644
--- a/libcxx/include/__cxx03/__bit_reference
+++ b/libcxx/include/__cxx03/__bit_reference
@@ -966,9 +966,9 @@ private:
   template <class _Dp>
   friend struct __bit_array;
 
-  template <bool _FillVal, class _Dp>
+  template <class _Dp>
   _LIB...
[truncated]

ldionne

I am pretty sure I remember there was a performance reason why @philnik777 refactored the value into a template parameter when he last looked at this. I'd like him to chime in.

winner245 · 2025-01-28T16:09:06Z

I am pretty sure I remember there was a performance reason why @philnik777 refactored the value into a template parameter when he last looked at this. I'd like him to chime in.

@philnik777, Would you share a bit of your insights on the performance reasons @ldionne mentioned? It would be really helpful to understand your concerns on this.

In the meantime, I've double-checked the current implementation. While it uses the bool parameter as a NTTP, the if condition checks on the bool values are all performed at runtime. I think we may improve the current implementation in the following two directions:

Performance: If we are concerned about the performance impact due to runtime if, we should leverage the compile-time benefits of NTTP by using constexpr if (C++17 and later) or templates with SFINAE (C++14 and earlier). Using NTTP while still performing runtime if still incurs unnecessary overhead.
Clarity: If we can tolerate the minor performance impact (as is currently implemented), it is more straightforward to use a regular function parameter instead of NTTP. This allows us to simplify the implementation a bit by removing unnecessary if/else statements, as this refactoring does.

Which approach do you find more useful?

philnik777 · 2025-01-28T22:16:43Z

Sorry, I thought I already commented. I don't remember the details, but I'm quite certain this has a non-trivial performance impact due to a significantly larger number of branches and clang probably not being able to vectorize the aligned case (haven't checked though). It also makes constant folding less likely, since the non-aligned code paths are much more complicated than the aligned ones and inlining being mostly done on function boundaries.

In the meantime, I've double-checked the current implementation. While it uses the bool parameter as a NTTP, the if condition checks on the bool values are all performed at runtime. I think we may improve the current implementation in the following two directions:

Performance: If we are concerned about the performance impact due to runtime if, we should leverage the compile-time benefits of NTTP by using constexpr if (C++17 and later) or templates with SFINAE (C++14 and earlier). Using NTTP while still performing runtime if still incurs unnecessary overhead.

They are technically runtime checks, but the compiler will constant fold it even at -O0. There is no real benefit to your suggested changes. They may ecen be worse, since sfinae is quite expensive.

winner245 changed the title ~~Refactor std::fill and std::count for bit iterator~~ [libc++] Refactor std::fill and std::count for bit iterator Dec 17, 2024

winner245 changed the title ~~[libc++] Refactor std::fill and std::count for bit iterator~~ [libc++] Refactor std::fill_n and std::count for bit iterator Dec 17, 2024

winner245 force-pushed the simplify-fill_n branch 9 times, most recently from 662184d to f150f78 Compare December 20, 2024 04:13

Simplify std::fill and std::count for bit iterator

6e257ee

winner245 force-pushed the simplify-fill_n branch from f150f78 to 6e257ee Compare December 20, 2024 11:53

winner245 changed the title ~~[libc++] Refactor std::fill_n and std::count for bit iterator~~ [libc++] Refactor std::{fill_n, count} for vector<bool>::iterator Dec 24, 2024

winner245 marked this pull request as ready for review January 10, 2025 19:45

winner245 requested a review from a team as a code owner January 10, 2025 19:45

llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Jan 10, 2025

ldionne requested changes Jan 20, 2025

View reviewed changes

ldionne requested a review from philnik777 January 20, 2025 17:32

winner245 closed this Mar 12, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[libc++] Refactor std::{fill_n, count} for vector<bool>::iterator #120305

[libc++] Refactor std::{fill_n, count} for vector<bool>::iterator #120305

Uh oh!

winner245 commented Dec 17, 2024 •

edited

Loading

Uh oh!

github-actions bot commented Dec 17, 2024 •

edited

Loading

Uh oh!

llvmbot commented Jan 10, 2025

Uh oh!

ldionne left a comment

Uh oh!

winner245 commented Jan 28, 2025

Uh oh!

philnik777 commented Jan 28, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

[libc++] Refactor std::{fill_n, count} for vector<bool>::iterator #120305

[libc++] Refactor std::{fill_n, count} for vector<bool>::iterator #120305

Uh oh!

Conversation

winner245 commented Dec 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Dec 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Jan 10, 2025

Uh oh!

ldionne left a comment

Choose a reason for hiding this comment

Uh oh!

winner245 commented Jan 28, 2025

Uh oh!

philnik777 commented Jan 28, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

winner245 commented Dec 17, 2024 •

edited

Loading

github-actions bot commented Dec 17, 2024 •

edited

Loading