Patchwork abs(long long)

login
register
mail settings
Submitter Marc Glisse
Date Oct. 2, 2012, 6:53 p.m.
Message ID <alpine.DEB.2.02.1210022015230.3794@laptop-mg.saclay.inria.fr>
Download mbox | patch
Permalink /patch/188615/
State New
Headers show

Comments

Marc Glisse - Oct. 2, 2012, 6:53 p.m.
On Tue, 2 Oct 2012, Gabriel Dos Reis wrote:

> Whining on this list about libstdc++ internal macros and your dislike
> of them is not going to produce anything today or tomorrow.

Other compilers using libstdc++ was just an extra argument. Even if g++ 
was the only compiler on earth, I would still consider a compile-time test 
superior to a configure test. The macro __SIZEOF_INT128__ was invented 
precisely for this purpose. Yes, that's just more whining ;-)


On Tue, 2 Oct 2012, Gabriel Dos Reis wrote:

> On Tue, Oct 2, 2012 at 8:07 AM, Marc Glisse <marc.glisse@inria.fr> wrote:
>
>> Or do you mean:
>> always call __builtin_llabs (whether we have an llabs or not), and let the
>> compiler replace it with either (x<0)?-x:x or a library call (I assume it
>> never does that unless it has seen a corresponding declaration)?
>
> See what we did in c/cmath and c_global/cmath.

Note that llabs is quite different from asin. __builtin_llabs generates an 
ABS_EXPR, which will later be expanded either to a special instruction or 
to a condition. It never generates a call to llabs (I am not sure exactly 
if Paolo's instructions to use llabs meant he wanted an actual library 
call). __builtin_asin on the other hand is never expanded inline (except 
maybe for special constant input like 0) and expands to a call to the 
library function asin.

Would the attached patch be better, assuming it passes testing? For lldiv, 
there is no builtin (for good reason).

         * include/c_std/cstdlib (abs(long long)): Define with
 	__builtin_llabs when we have long long.
         (abs(__int128)): Define when we have __int128.
         (div(long long, long long)): Use lldiv.
Gabriel Dos Reis - Oct. 3, 2012, 5:29 p.m.
On Tue, Oct 2, 2012 at 1:53 PM, Marc Glisse <marc.glisse@inria.fr> wrote:

>> See what we did in c/cmath and c_global/cmath.
>
>
> Note that llabs is quite different from asin.

Is asin the function you took out of c/cmath?

>  __builtin_llabs generates an
> ABS_EXPR, which will later be expanded either to a special instruction or to
> a condition. It never generates a call to llabs (I am not sure exactly if
> Paolo's instructions to use llabs meant he wanted an actual library call).

distinction without difference.  Again see c/cmath.


> __builtin_asin on the other hand is never expanded inline (except maybe for
> special constant input like 0) and expands to a call to the library function
> asin.
>
> Would the attached patch be better, assuming it passes testing? For lldiv,
> there is no builtin (for good reason).
>
>         * include/c_std/cstdlib (abs(long long)): Define with
>         __builtin_llabs when we have long long.
>
>         (abs(__int128)): Define when we have __int128.

This change is OK

>         (div(long long, long long)): Use lldiv.

not this one.

>
>
> --
> Marc Glisse
> Index: cstdlib
> ===================================================================
> --- cstdlib     (revision 191941)
> +++ cstdlib     (working copy)
> @@ -128,21 +128,31 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>    using ::strtod;
>    using ::strtol;
>    using ::strtoul;
>    using ::system;
>  #ifdef _GLIBCXX_USE_WCHAR_T
>    using ::wcstombs;
>    using ::wctomb;
>  #endif // _GLIBCXX_USE_WCHAR_T
>
>    inline long
> -  abs(long __i) { return labs(__i); }
> +  abs(long __i) { return __builtin_labs(__i); }
> +
> +#ifdef _GLIBCXX_USE_LONG_LONG
> +  inline long long
> +  abs(long long __x) { return __builtin_llabs (__x); }
> +#endif
> +
> +#if !defined(__STRICT_ANSI__) && defined(_GLIBCXX_USE_INT128)
> +  inline __int128
> +  abs(__int128 __x) { return __x >= 0 ? __x : -__x; }
> +#endif
>
>    inline ldiv_t
>    div(long __i, long __j) { return ldiv(__i, __j); }
>
>  _GLIBCXX_END_NAMESPACE_VERSION
>  } // namespace
>
>  #if _GLIBCXX_USE_C99
>
>  #undef _Exit
> @@ -161,29 +171,26 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>  #if !_GLIBCXX_USE_C99_LONG_LONG_DYNAMIC
>    using ::lldiv_t;
>  #endif
>  #if _GLIBCXX_USE_C99_CHECK || _GLIBCXX_USE_C99_DYNAMIC
>    extern "C" void (_Exit)(int) throw () _GLIBCXX_NORETURN;
>  #endif
>  #if !_GLIBCXX_USE_C99_DYNAMIC
>    using ::_Exit;
>  #endif
>
> -  inline long long
> -  abs(long long __x) { return __x >= 0 ? __x : -__x; }
> -
>  #if !_GLIBCXX_USE_C99_LONG_LONG_DYNAMIC
>    using ::llabs;
>
>    inline lldiv_t
>    div(long long __n, long long __d)
> -  { lldiv_t __q; __q.quot = __n / __d; __q.rem = __n % __d; return __q; }
> +  { return ::lldiv (__n, __d); }
>
>    using ::lldiv;
>  #endif
>
>  #if _GLIBCXX_USE_C99_LONG_LONG_CHECK || _GLIBCXX_USE_C99_LONG_LONG_DYNAMIC
>    extern "C" long long int (atoll)(const char *) throw ();
>    extern "C" long long int
>      (strtoll)(const char * __restrict, char ** __restrict, int) throw ();
>    extern "C" unsigned long long int
>      (strtoull)(const char * __restrict, char ** __restrict, int) throw ();
> @@ -198,21 +205,20 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>
>  _GLIBCXX_END_NAMESPACE_VERSION
>  } // namespace __gnu_cxx
>
>  namespace std
>  {
>  #if !_GLIBCXX_USE_C99_LONG_LONG_DYNAMIC
>    using ::__gnu_cxx::lldiv_t;
>  #endif
>    using ::__gnu_cxx::_Exit;
> -  using ::__gnu_cxx::abs;
>  #if !_GLIBCXX_USE_C99_LONG_LONG_DYNAMIC
>    using ::__gnu_cxx::llabs;
>    using ::__gnu_cxx::div;
>    using ::__gnu_cxx::lldiv;
>  #endif
>    using ::__gnu_cxx::atoll;
>    using ::__gnu_cxx::strtof;
>    using ::__gnu_cxx::strtoll;
>    using ::__gnu_cxx::strtoull;
>    using ::__gnu_cxx::strtold;
>
Marc Glisse - Oct. 3, 2012, 8 p.m.
On Wed, 3 Oct 2012, Gabriel Dos Reis wrote:

> On Tue, Oct 2, 2012 at 1:53 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
>
>>         * include/c_std/cstdlib (abs(long long)): Define with
>>         __builtin_llabs when we have long long.
>>
>>         (abs(__int128)): Define when we have __int128.
>
> This change is OK

Thanks, I'll commit that part only.

>>         (div(long long, long long)): Use lldiv.
>
> not this one.

Ok. Note that glibc's implementation does more than just / and %. Possible 
reasons are:
1) glibc does useless work
2) libstdc++ has a bug
3) there are platforms supported by glibc but not by libstdc++

I choose to believe it is option 3.

Patch

Index: cstdlib

===================================================================
--- cstdlib	(revision 191941)

+++ cstdlib	(working copy)

@@ -128,21 +128,31 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION

   using ::strtod;
   using ::strtol;
   using ::strtoul;
   using ::system;
 #ifdef _GLIBCXX_USE_WCHAR_T
   using ::wcstombs;
   using ::wctomb;
 #endif // _GLIBCXX_USE_WCHAR_T
 
   inline long
-  abs(long __i) { return labs(__i); }

+  abs(long __i) { return __builtin_labs(__i); }

+

+#ifdef _GLIBCXX_USE_LONG_LONG

+  inline long long

+  abs(long long __x) { return __builtin_llabs (__x); }

+#endif

+

+#if !defined(__STRICT_ANSI__) && defined(_GLIBCXX_USE_INT128)

+  inline __int128

+  abs(__int128 __x) { return __x >= 0 ? __x : -__x; }

+#endif

 
   inline ldiv_t
   div(long __i, long __j) { return ldiv(__i, __j); }
 
 _GLIBCXX_END_NAMESPACE_VERSION
 } // namespace
 
 #if _GLIBCXX_USE_C99
 
 #undef _Exit
@@ -161,29 +171,26 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION

 #if !_GLIBCXX_USE_C99_LONG_LONG_DYNAMIC
   using ::lldiv_t;
 #endif
 #if _GLIBCXX_USE_C99_CHECK || _GLIBCXX_USE_C99_DYNAMIC
   extern "C" void (_Exit)(int) throw () _GLIBCXX_NORETURN;
 #endif
 #if !_GLIBCXX_USE_C99_DYNAMIC
   using ::_Exit;
 #endif
 
-  inline long long

-  abs(long long __x) { return __x >= 0 ? __x : -__x; }

-

 #if !_GLIBCXX_USE_C99_LONG_LONG_DYNAMIC
   using ::llabs;
 
   inline lldiv_t
   div(long long __n, long long __d)
-  { lldiv_t __q; __q.quot = __n / __d; __q.rem = __n % __d; return __q; }

+  { return ::lldiv (__n, __d); }

 
   using ::lldiv;
 #endif
 
 #if _GLIBCXX_USE_C99_LONG_LONG_CHECK || _GLIBCXX_USE_C99_LONG_LONG_DYNAMIC
   extern "C" long long int (atoll)(const char *) throw ();
   extern "C" long long int
     (strtoll)(const char * __restrict, char ** __restrict, int) throw ();
   extern "C" unsigned long long int
     (strtoull)(const char * __restrict, char ** __restrict, int) throw ();
@@ -198,21 +205,20 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION

 
 _GLIBCXX_END_NAMESPACE_VERSION
 } // namespace __gnu_cxx
 
 namespace std
 {
 #if !_GLIBCXX_USE_C99_LONG_LONG_DYNAMIC
   using ::__gnu_cxx::lldiv_t;
 #endif
   using ::__gnu_cxx::_Exit;
-  using ::__gnu_cxx::abs;

 #if !_GLIBCXX_USE_C99_LONG_LONG_DYNAMIC
   using ::__gnu_cxx::llabs;
   using ::__gnu_cxx::div;
   using ::__gnu_cxx::lldiv;
 #endif
   using ::__gnu_cxx::atoll;
   using ::__gnu_cxx::strtof;
   using ::__gnu_cxx::strtoll;
   using ::__gnu_cxx::strtoull;
   using ::__gnu_cxx::strtold;