[libcxx-commits] [PATCH] D145982: [libc++] Implement std::gcd using the binary version

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik added a comment.

Neither quick bench nor I on my local system can reproduce a 4x speedup. In fact, the naive version is significantly faster than your version. Are you sure everything works as expected?



================
Comment at: libcxx/include/__bit/countr.h:35
 
-#if _LIBCPP_STD_VER >= 20
-
 template <__libcpp_unsigned_integer _Tp>
+inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR int __countr_zero(_Tp __t) noexcept {
----------------
This doesn't work pre-C++20.


================
Comment at: libcxx/include/__bit/countr.h:36
 template <__libcpp_unsigned_integer _Tp>
-_LIBCPP_HIDE_FROM_ABI constexpr int countr_zero(_Tp __t) noexcept {
+inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR int __countr_zero(_Tp __t) noexcept {
   if (__t == 0)
----------------



================
Comment at: libcxx/include/__numeric/gcd_lcm.h:64
+    int __bz    = std::__countr_zero(__n);
+    int __shift = _VSTD::min(__az, __bz);
+    __m >>= __az;
----------------
Same for other places.


================
Comment at: libcxx/test/libcxx/transitive_includes/cxx26.csv:188-208
+ext/hash_map algorithm
+ext/hash_map cmath
+ext/hash_map cstddef
+ext/hash_map cstdint
+ext/hash_map cstring
+ext/hash_map functional
+ext/hash_map initializer_list
----------------
This shouldn't be in here anymore. Did you maybe use an older version to generate this?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145982/new/

https://reviews.llvm.org/D145982