Re: [patch] std::experimental::gcd and std::experimental::lcd

[Jonathan Wakely]

On 02/05/15 18:27 +0200, Marc Glisse wrote:

On Sat, 2 May 2015, Jonathan Wakely wrote:

These where simple to implement (almost too simple ... I probably
got something wrong!)

I didn't remember that std::abs works for unsigned. It will need more work for performance, but that can certainly be done later (I didn't look at the code beyond checking what you meant by "simple").

std::abs seems to work fine for unsigned, the overload in <cmath> for
integral types just uses __builtin_fabs. Maybe it would be better for
gcd() to just use that directly instead of including <cmath> (as
attached, which also removes the qualification on the call to gcd
because the functions only work for integral types which have no
associated namespaces anyway).

(Apart from using common_type_t, which is easy to change, these
functions meet the simpler rules for C++11 constexpr, so moving them
out of <experimental/numeric> would probably allow <ratio> to be
greatly simplified. I don't plan on doing that myself any time soon,
but it would make sense to do it some day.)

gcd is not really the hard part in ratio. But constexpr should help, it made sense not to have it in tr1, but I don't remember why we didn't use it in the more recent changes (2011, the compiler probably already supported constexpr). Maybe the interesting functions were too hard to write as one-liners...

<ratio> was added to libstdc++ in 2008 so I think the constexpr
support was not good enough at the time.


diff --git a/libstdc++-v3/include/experimental/numeric 
b/libstdc++-v3/include/experimental/numeric
index a11516b..b284110 100644
--- a/libstdc++-v3/include/experimental/numeric
+++ b/libstdc++-v3/include/experimental/numeric
@@ -40,7 +40,6 @@
 #else
 
 #include <experimental/type_traits>
-#include <cmath>
 
 namespace std _GLIBCXX_VISIBILITY(default)
 {
@@ -52,7 +51,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
 #define __cpp_lib_experimental_gcd_lcm 201411
 
-  // Greatest common divisor
+  /// Greatest common divisor
   template<typename _Mn, typename _Nn>
     constexpr common_type_t<_Mn, _Nn>
     gcd(_Mn __m, _Nn __n)
@@ -60,12 +59,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       static_assert(is_integral<_Mn>::value, "arguments to gcd are integers");
       static_assert(is_integral<_Nn>::value, "arguments to gcd are integers");
 
-      return __m == 0 ? std::abs(__n)
-       : __n == 0 ? std::abs(__m)
-       : fundamentals_v2::gcd(__n, __m % __n);
+      return __m == 0 ? __builtin_abs(__n)
+       : __n == 0 ? __builtin_abs(__m)
+       : gcd(__n, __m % __n);
     }
 
-  // Least common multiple
+  /// Least common multiple
   template<typename _Mn, typename _Nn>
     constexpr common_type_t<_Mn, _Nn>
     lcm(_Mn __m, _Nn __n)
@@ -74,7 +73,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       static_assert(is_integral<_Nn>::value, "arguments to lcm are integers");
 
       return (__m != 0 && __n != 0)
-       ? (std::abs(__m) / fundamentals_v2::gcd(__m, __n)) * std::abs(__n)
+       ? (__builtin_abs(__m) / gcd(__m, __n)) * __builtin_abs(__n)
        : 0;
     }