Skip to content

Replace uses of __libcpp_ctz by __countr_zero #132639

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.

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

Closed
wants to merge 1 commit into from

Conversation

PaulXiCao
Copy link
Contributor

Closes #131179.

@PaulXiCao PaulXiCao requested a review from a team as a code owner March 23, 2025 22:07
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Mar 23, 2025
@llvmbot
Copy link
Member

llvmbot commented Mar 23, 2025

@llvm/pr-subscribers-libcxx

Author: None (PaulXiCao)

Changes

Closes #131179.


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

3 Files Affected:

  • (modified) libcxx/include/__algorithm/sort.h (+2-2)
  • (modified) libcxx/include/__cxx03/__algorithm/find.h (+3-3)
  • (modified) libcxx/include/__cxx03/__algorithm/sort.h (+2-2)
diff --git a/libcxx/include/__algorithm/sort.h b/libcxx/include/__algorithm/sort.h
index 8dd0721f2c65f..015d6531b1d87 100644
--- a/libcxx/include/__algorithm/sort.h
+++ b/libcxx/include/__algorithm/sort.h
@@ -359,9 +359,9 @@ inline _LIBCPP_HIDE_FROM_ABI void __swap_bitmap_pos(
   // Swap one pair on each iteration as long as both bitsets have at least one
   // element for swapping.
   while (__left_bitset != 0 && __right_bitset != 0) {
-    difference_type __tz_left  = __libcpp_ctz(__left_bitset);
+    difference_type __tz_left  = __countr_zero(__left_bitset);
     __left_bitset              = __libcpp_blsr(__left_bitset);
-    difference_type __tz_right = __libcpp_ctz(__right_bitset);
+    difference_type __tz_right = __countr_zero(__right_bitset);
     __right_bitset             = __libcpp_blsr(__right_bitset);
     _Ops::iter_swap(__first + __tz_left, __last - __tz_right);
   }
diff --git a/libcxx/include/__cxx03/__algorithm/find.h b/libcxx/include/__cxx03/__algorithm/find.h
index ff5ac9b8b1bd0..29486e9438711 100644
--- a/libcxx/include/__cxx03/__algorithm/find.h
+++ b/libcxx/include/__cxx03/__algorithm/find.h
@@ -108,7 +108,7 @@ __find_bool(__bit_iterator<_Cp, _IsConst> __first, typename _Cp::size_type __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;
     if (__b)
-      return _It(__first.__seg_, static_cast<unsigned>(std::__libcpp_ctz(__b)));
+      return _It(__first.__seg_, static_cast<unsigned>(std::__countr_zero(__b)));
     if (__n == __dn)
       return __first + __n;
     __n -= __dn;
@@ -118,14 +118,14 @@ __find_bool(__bit_iterator<_Cp, _IsConst> __first, typename _Cp::size_type __n)
   for (; __n >= __bits_per_word; ++__first.__seg_, __n -= __bits_per_word) {
     __storage_type __b = std::__invert_if<!_ToFind>(*__first.__seg_);
     if (__b)
-      return _It(__first.__seg_, static_cast<unsigned>(std::__libcpp_ctz(__b)));
+      return _It(__first.__seg_, static_cast<unsigned>(std::__countr_zero(__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;
     if (__b)
-      return _It(__first.__seg_, static_cast<unsigned>(std::__libcpp_ctz(__b)));
+      return _It(__first.__seg_, static_cast<unsigned>(std::__countr_zero(__b)));
   }
   return _It(__first.__seg_, static_cast<unsigned>(__n));
 }
diff --git a/libcxx/include/__cxx03/__algorithm/sort.h b/libcxx/include/__cxx03/__algorithm/sort.h
index 009ebf717bbd8..47472afbbe562 100644
--- a/libcxx/include/__cxx03/__algorithm/sort.h
+++ b/libcxx/include/__cxx03/__algorithm/sort.h
@@ -399,9 +399,9 @@ inline _LIBCPP_HIDE_FROM_ABI void __swap_bitmap_pos(
   // Swap one pair on each iteration as long as both bitsets have at least one
   // element for swapping.
   while (__left_bitset != 0 && __right_bitset != 0) {
-    difference_type __tz_left  = __libcpp_ctz(__left_bitset);
+    difference_type __tz_left  = __countr_zero(__left_bitset);
     __left_bitset              = __libcpp_blsr(__left_bitset);
-    difference_type __tz_right = __libcpp_ctz(__right_bitset);
+    difference_type __tz_right = __countr_zero(__right_bitset);
     __right_bitset             = __libcpp_blsr(__right_bitset);
     _Ops::iter_swap(__first + __tz_left, __last - __tz_right);
   }

@winner245
Copy link
Contributor

Hi @PaulXiCao, thanks for your efforts in addressing this issue! Just to provide some context—this issue was opened a week ago as a follow-up to my previous work and was assigned to me shortly thereafter. I’ve been actively working on related patches, but since this change depends on dropping Clang 18 support, I hadn’t submitted a patch yet.

While I truly appreciate your enthusiasm and contribution, I think it would be helpful in the future to check with the assignee before working on a recently assigned issue, especially if it’s a follow-up to their prior work. This can help avoid duplicated effort and ensure smoother collaboration. Thanks again!

@mordante
Copy link
Member

Hi @PaulXiCao, thanks for your efforts in addressing this issue! Just to provide some context—this issue was opened a week ago as a follow-up to my previous work and was assigned to me shortly thereafter. I’ve been actively working on related patches, but since this change depends on dropping Clang 18 support, I hadn’t submitted a patch yet.

While I truly appreciate your enthusiasm and contribution, I think it would be helpful in the future to check with the assignee before working on a recently assigned issue, especially if it’s a follow-up to their prior work. This can help avoid duplicated effort and ensure smoother collaboration. Thanks again!

FYI @winner245 removing Clang-18 support is blocked by some CI runners, the maintainers for these runners are working on upgrading the clang version of their runners.

@winner245
Copy link
Contributor

Hi @mordante, thank you for the update on the current status of removing Clang 18 support, which is currently blocked by some CI runners. I understand that the changes in this patch will need to be put on hold until Clang 18 support is officially dropped (@PaulXiCao).

@mordante
Copy link
Member

Hi @mordante, thank you for the update on the current status of removing Clang 18 support, which is currently blocked by some CI runners. I understand that the changes in this patch will need to be put on hold until Clang 18 support is officially dropped (@PaulXiCao).

Looking at the status of the CI this works in Clang-18; the ARM runners are still on Clang-18.

@winner245
Copy link
Contributor

Hi @mordante, thank you for the update on the current status of removing Clang 18 support, which is currently blocked by some CI runners. I understand that the changes in this patch will need to be put on hold until Clang 18 support is officially dropped (@PaulXiCao).

Looking at the status of the CI this works in Clang-18; the ARM runners are still on Clang-18.

The primary goal of #131179 (based on my discussion with @ldionne in #122641 (comment) ) was to eliminate all usages of __libcpp_ctz in favor of the built-in implementation available since Clang 19. Ultimately, this allows us to remove functions like __libcpp_ctz, but only when we officially drop support for Clang 18.

That being said, the current patch doesn't fully address issue #131179. Most of the changes are applied to frozen header files, which I believe should stay unchanged unless there is a strong reason. That's why I didn't make these changes in my earlier patch.

@mordante
Copy link
Member

Hi @mordante, thank you for the update on the current status of removing Clang 18 support, which is currently blocked by some CI runners. I understand that the changes in this patch will need to be put on hold until Clang 18 support is officially dropped (@PaulXiCao).

Looking at the status of the CI this works in Clang-18; the ARM runners are still on Clang-18.

The primary goal of #131179 (based on my discussion with @ldionne in #122641 (comment) ) was to eliminate all usages of __libcpp_ctz in favor of the built-in implementation available since Clang 19. Ultimately, this allows us to remove functions like __libcpp_ctz, but only when we officially drop support for Clang 18.

I didn't look into the patch or which Clang version supports the built-in. If you say that's Clang-19 this patch indeed needs to be post-poned until we can drop Clang-18 support. I mainly wanted to update you on the status of Clang-18 support.

@ldionne
Copy link
Member

ldionne commented Mar 25, 2025

Closing per @winner245 's comment -- we already have a patch ready to go once Clang 18 is dropped, and this patch as-is doesn't implement all that we wanted from #131179.

@ldionne ldionne closed this Mar 25, 2025
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.

[libc++] Replace uses of __libcpp_ctz by __countr_zero
5 participants