Re: abs(long long)

[Gabriel Dos Reis <gdr at integrable-solutions dot net>]

On Tue, Oct 2, 2012 at 3:57 AM, Marc Glisse <marc.glisse@inria.fr> wrote:
> (Forgot libstdc++...)
>
>
> Hello,
>
> here is the patch from PR54686. Several notes:
>
> * I'll have to ask experts if std::abs(unsigned) (yes, a weird thing to do,
> but still) is meant to return a double...

don't we have a core issue about preferring unsigned -> long or long long?

> * I still don't like the configure-time _GLIBCXX_USE_INT128, I think it
> should use defined(__SIZEOF_INT128__), which would help other compilers.

Why would that be a problem with the appropriate #define?

> * newlib has llabs, according to the doc. It would be good to know what
> newlib is missing for libstdc++ to detect it as C99-ready.
>
> I tested a previous version (without __STRICT_ANSI__) on x86_64-linux-gnu
> and Oleg Endo did a basic check on sh/newlib. I'll do a last check after the
> review (no point if the patch needs changing again).

In general, I think I have a bias toward using compiler intrinsics,
for which the
compiler already has lot of knowledge about.


>
> 2012-10-02  Marc Glisse  <marc.glisse@inria.fr>
>
>         PR libstdc++/54686
>         * include/c_std/cstdlib (abs(long long)): Define fallback whenever
>         we have long long but possibly not llabs.
>         (abs(long long)): Use llabs when available.
>         (abs(__int128)): Define when we have __int128.
>         (div(long long, long long)): Use lldiv.
>         * testsuite/26_numerics/headers/cstdlib/54686.c: New file.
>
> --
> Marc Glisse
>
> Index: include/c_std/cstdlib
> ===================================================================
> --- include/c_std/cstdlib       (revision 191941)
> +++ include/c_std/cstdlib       (working copy)
> @@ -130,20 +130,32 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>    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); }
>
> +#if defined (_GLIBCXX_USE_LONG_LONG) \
> +    && (!_GLIBCXX_USE_C99 || _GLIBCXX_USE_C99_LONG_LONG_DYNAMIC)
> +  // Fallback version if we don't have llabs but still allow long long.
> +  inline long long
> +  abs(long long __x) { return __x >= 0 ? __x : -__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
>  #undef llabs
> @@ -161,29 +173,29 @@ _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
>
> +#if !_GLIBCXX_USE_C99_LONG_LONG_DYNAMIC
>    inline long long
> -  abs(long long __x) { return __x >= 0 ? __x : -__x; }
> +  abs(long long __x) { return ::llabs (__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,22 +210,22 @@ _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::abs;
>    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;
>  } // namespace std
> Index: testsuite/26_numerics/headers/cstdlib/54686.c
> ===================================================================
> --- testsuite/26_numerics/headers/cstdlib/54686.c       (revision 0)
> +++ testsuite/26_numerics/headers/cstdlib/54686.c       (revision 0)
> @@ -0,0 +1,32 @@
> +// { dg-do compile }
> +// { dg-options "-std=c++11" }
> +
> +// Copyright (C) 2012 Free Software Foundation, Inc.
> +//
> +// This file is part of the GNU ISO C++ Library.  This library is free
> +// software; you can redistribute it and/or modify it under the
> +// terms of the GNU General Public License as published by the
> +// Free Software Foundation; either version 3, or (at your option)
> +// any later version.
> +//
> +// This library is distributed in the hope that it will be useful,
> +// but WITHOUT ANY WARRANTY; without even the implied warranty of
> +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +// GNU General Public License for more details.
> +//
> +// You should have received a copy of the GNU General Public License along
> +// with this library; see the file COPYING3.  If not see
> +// <http://www.gnu.org/licenses/>.
> +
> +#include <cmath>
> +#include <cstdlib>
> +#include <type_traits>
> +#include <utility>
> +
> +#ifdef _GLIBCXX_USE_LONG_LONG
> +void test01()
> +{
> +  static_assert (std::is_same<decltype (std::abs (std::declval<long long>
> ())),
> +                             long long>::value, "Missing abs(long long)");
> +}
> +#endif
>
> Property changes on: testsuite/26_numerics/headers/cstdlib/54686.c
> ___________________________________________________________________
> Added: svn:keywords
>    + Author Date Id Revision URL
> Added: svn:eol-style
>    + native
>
>