[libc++] Remove unnecessary division and modulo operations in bitset

[winner245]

The PR removes the unnecessary division and modulo operations in the one-word specialization __bitset<1, _Size>. The reason is that for the one-word specialization, we have __pos < __bits_per_word (as __bitset<1, _Size> is an implementation detail only used by the public bitset). So __pos / __bits_per_word == 0 and __pos / __pos % __bits_per_word == __pos.

[llvmbot]

@llvm/pr-subscribers-libcxx

Author: Peng Liu (winner245)

Changes

The PR removes the unnecessary division and modulo operations in the one-word specialization __bitset<1, _Size>. The reason is that for the one-word specialization, we have __pos < __bits_per_word (as __bitset<1, _Size> is an implementation detail only used by the public bitset). So __pos / __bits_per_word == 0 and __pos / __pos % __bits_per_word == __pos.

---

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

1 Files Affected:

• (modified) libcxx/include/bitset (+2-2)

diff --git a/libcxx/include/bitset b/libcxx/include/bitset
index a20842985b3d5..31e960cc8797b 100644
--- a/libcxx/include/bitset
+++ b/libcxx/include/bitset
@@ -465,10 +465,10 @@ protected:
     return __const_reference(&__first_, __storage_type(1) << __pos);
   }
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 __iterator __make_iter(size_t __pos) _NOEXCEPT {
-    return __iterator(&__first_ + __pos / __bits_per_word, __pos % __bits_per_word);
+    return __iterator(&__first_, __pos);
   }
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 __const_iterator __make_iter(size_t __pos) const _NOEXCEPT {
-    return __const_iterator(&__first_ + __pos / __bits_per_word, __pos % __bits_per_word);
+    return __const_iterator(&__first_, __pos);
   }
 
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 void operator&=(const __bitset& __v) _NOEXCEPT;

[ldionne]

LGTM with suggestions applied, thanks for noticing this!

[ldionne]

I am tempted to see whether we can introduce __begin() and __end() functions instead, and get rid of __make_iter entirely. I'd imagine something like

iterator __begin() { return /* __make_iter(0); */ }
iterator __end() { return /* __make_iter(_Size); */ }

// and then here's an example usage
// OLD:
std::copy_backward(__base::__make_iter(0), __base::__make_iter(_Size - __pos), __base::__make_iter(_Size));

// NEW:
std::copy_backward(__begin(), __end() - __pos, __end());

Now the main question IMO is whether __end() - __pos produces equivalent code. I think we'll end up calling

llvm-project/libcxx/include/__bit_reference

Line 368 in 52f941a

_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 __bit_iterator& operator+=(difference_type __n) {

, which might not produce efficient code. I still think we should investigate that since that looks like the way we'd want to write our code at a high level.

[ldionne]

I filed that as #133111, let's do it in a follow-up