Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Mar 26, 2025
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a27513b
Remove unnecessary division and modulo operations in bitset
winner245 Dec 30, 2024
f6043c3
Add _LIBCPP_ASSERT_INTERNAL
winner245 Mar 19, 2025
9f8c637
Use _LIBCPP_ASSERT_INTERNAL
winner245 Mar 26, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion libcxx/include/__bit_reference
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,7 @@ public:
}

_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 reference operator*() const _NOEXCEPT {
_LIBCPP_ASSERT_INTERNAL(__ctz_ < __bits_per_word, "Dereferencing an invalid __bit_iterator.");
return __conditional_t<_IsConst, __bit_const_reference<_Cp>, __bit_reference<_Cp> >(
__seg_, __storage_type(1) << __ctz_);
}
Expand Down Expand Up @@ -453,7 +454,10 @@ private:
_LIBCPP_HIDE_FROM_ABI
_LIBCPP_CONSTEXPR_SINCE_CXX20 explicit __bit_iterator(__storage_pointer __s, unsigned __ctz) _NOEXCEPT
: __seg_(__s),
__ctz_(__ctz) {}
__ctz_(__ctz) {
_LIBCPP_ASSERT_INTERNAL(
__ctz_ < __bits_per_word, "__bit_iterator initialized with an invalid number of trailing zeros.");
}

friend typename _Cp::__self;

Expand Down
8 changes: 6 additions & 2 deletions libcxx/include/bitset
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -464,10 +464,14 @@ protected:
return __const_reference(&__first_, __storage_type(1) << __pos);
}
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 __iterator __make_iter(size_t __pos) _NOEXCEPT {
Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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

return __iterator(&__first_ + __pos / __bits_per_word, __pos % __bits_per_word);
// Allow the == case to accommodate the past-the-end iterator.
_LIBCPP_ASSERT_INTERNAL(__pos <= __bits_per_word, "Out of bounds access in the single-word bitset implementation.");
return __pos != __bits_per_word ? __iterator(&__first_, __pos) : __iterator(&__first_ + 1, 0);
}
_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);
// Allow the == case to accommodate the past-the-end iterator.
_LIBCPP_ASSERT_INTERNAL(__pos <= __bits_per_word, "Out of bounds access in the single-word bitset implementation.");
return __pos != __bits_per_word ? __const_iterator(&__first_, __pos) : __const_iterator(&__first_ + 1, 0);
}

_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 void operator&=(const __bitset& __v) _NOEXCEPT;
Expand Down