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

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik requested changes to this revision.
philnik added a comment.
This revision now requires changes to proceed.

Thanks for working on this!
Could you add the benchmark you used to `libcxx/benchmarks` and give us the exact results you get?



================
Comment at: libcxx/include/__numeric/gcd_lcm.h:15
 #include <__config>
+#if _LIBCPP_STD_VER >= 20
+#  include <__algorithm/min.h>
----------------
These should be included unconditionally.


================
Comment at: libcxx/include/__numeric/gcd_lcm.h:65
+
+    int __az    = _VSTD::countr_zero(__m);
+    int __bz    = _VSTD::countr_zero(__n);
----------------
Same throughout


================
Comment at: libcxx/include/__numeric/gcd_lcm.h:66
+    int __az    = _VSTD::countr_zero(__m);
+    int __bz    = _VSTD::countr_zero(__n);
+    int __shift = _VSTD::min(__az, __bz);
----------------
Is there anything C++20-specific other than `countr_zero`? If no, then I think it would make sense to add a `__countr_zero` to older language modes and forward the `countr_zero` implementation.


================
Comment at: libcxx/include/__numeric/gcd_lcm.h:68
+    int __shift = _VSTD::min(__az, __bz);
+    __m >>= __az, __n >>= __bz;
+    while (__m != 0) {
----------------



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145982