[Patch, committed] Fortran: call of overloaded ‘abs(long long int&)’ is ambiguous [PR109492]

2023-04-13 Thread Harald Anlauf via Gcc-patches
Dear all,

I've committed the attached patch which fixes a portability issue
when bootstrapping on Solaris.  Discussed and confirmed in the PR
by Jonathan for Solaris and regtested by me on x86_64-pc-linux-gnu.

https://gcc.gnu.org/g:43816633afd275a9057232a6ebfdc19e441f09ec

(Unfortunately the commit message contains Unicode characters
that I got by using copy&paste of the error message.  I wonder
if "git gcc-verify" could have warned me ...)

Thanks,
Harald

From 43816633afd275a9057232a6ebfdc19e441f09ec Mon Sep 17 00:00:00 2001
From: Harald Anlauf 
Date: Thu, 13 Apr 2023 22:42:23 +0200
Subject: [PATCH] =?UTF-8?q?Fortran:=20call=20of=20overloaded=20=E2=80=98ab?=
 =?UTF-8?q?s(long=20long=20int&)=E2=80=99=20is=20ambiguous=20[PR109492]?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

gcc/fortran/ChangeLog:

	PR fortran/109492
	* trans-expr.cc (gfc_conv_power_op): Use absu_hwi and
	unsigned HOST_WIDE_INT for portability.
---
 gcc/fortran/trans-expr.cc | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc
index 79367fa2ae0..09cdd9263c4 100644
--- a/gcc/fortran/trans-expr.cc
+++ b/gcc/fortran/trans-expr.cc
@@ -3400,11 +3400,12 @@ gfc_conv_power_op (gfc_se * se, gfc_expr * expr)
   && TREE_CODE (TREE_TYPE (rse.expr)) == INTEGER_TYPE)
 {
   wi::tree_to_wide_ref wlhs = wi::to_wide (lse.expr);
-  HOST_WIDE_INT v, w;
+  HOST_WIDE_INT v;
+  unsigned HOST_WIDE_INT w;
   int kind, ikind, bit_size;

   v = wlhs.to_shwi ();
-  w = abs (v);
+  w = absu_hwi (v);

   kind = expr->value.op.op1->ts.kind;
   ikind = gfc_validate_kind (BT_INTEGER, kind, false);
--
2.35.3



Re: abs(long long)

2012-10-03 Thread Marc Glisse

On Wed, 3 Oct 2012, Gabriel Dos Reis wrote:


On Tue, Oct 2, 2012 at 1:53 PM, Marc Glisse  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.

--
Marc Glisse


Re: abs(long long)

2012-10-03 Thread Gabriel Dos Reis
On Tue, Oct 2, 2012 at 1:53 PM, Marc Glisse  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;
>


Re: abs(long long)

2012-10-02 Thread Marc Glisse

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  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.


--
Marc GlisseIndex: 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;


Re: abs(long long)

2012-10-02 Thread Gabriel Dos Reis
On Tue, Oct 2, 2012 at 8:07 AM, Marc Glisse  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.
What you find there is the result of years of several iterations
(including something similar to your earlier patch) all having issues
in one way of another until we settled on the builtin functions approach.
I have no appetite to go back to those days full of headache.

-- Gaby


Re: abs(long long)

2012-10-02 Thread Gabriel Dos Reis
On Tue, Oct 2, 2012 at 9:34 AM, Daniel Krügler
 wrote:
> 2012/10/2 Marc Glisse :
>> Here I am talking of a library issue: the wording that says that there are
>> sufficient overloads such that integer types call the double version of math
>> functions. It is fairly obvious that it doesn't apply to abs(long) for
>> instance which has an explicit overload. For short or unsigned, I still read
>> it as saying that it converts to double...
>
> This really looks like a problem of the Standard Library specification
> to me and
> a corresponding issue should be submitted. In fact the wording can be
> interpreted
> that mixing  with  would imply two different versions of
> std::abs(int) because of different required return types. I will
> prepare a corresponding
> submission to the LWG.

This was already an issue I reported to LWG when C++98 came out.
Now that you hold  wrtite access to the issue document, you can
make sure it won't slip through the crack this time :-p

-- Gaby


Re: abs(long long)

2012-10-02 Thread Gabriel Dos Reis
On Tue, Oct 2, 2012 at 8:07 AM, Marc Glisse  wrote:

>>> The library installed by the system was compiled with g++, and is then
>>> used
>>> with clang++. If we can avoid installing 2 config.h files to make that
>>> work...
>>
>>
>> Two things:
>>  1. that is clearly a clang problem.  I don't think it is libstdc++'s job
>>  tp try to solve clang's misguided configuration and installation.
>
>
> Translated: libstdc++ should only ever be used with the very version of g++
> that was used to compile it. clang++, icpc, sunCC, etc should never try to
> use a libstdc++ compiled with another compiler.

Obviously, I cannot require you to exercise common sense
and keep in check non-sensical strech.

libstdc++ was and is developed for GCc/g++.  If you are have
a 3rd party compiler that you would like to use with g++/libstdc++, you
should
   (a) either convince your 3rd party compiler supplier
 to understand the library you already have (libstdc++), or
   (b) supply yourself the glue between libstdc++ and your compiler.
Many compilers have done that in the past; I don't see anything
special with clang++

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

>
> I am not saying libstdc++ should go to great lengths to support other
> compilers, but when it is actually easier to support them than not to...
> (testing a macro is easier than a configure test)
>
>
>>  2. I am not sure you understand what I wrote: you can leave the
>>  use of the current macro the way it is if you appropriately
>>  define it in terms of what you want to change it to.
>
>
> I was complaining about the configure-time nature of the macro. If it is
> defined at each compiler run based on __SIZEOF_INT128__, I am happy.

I am saying to can arrange to supply the appropriate definition
without having to change the uses.

>
>>> More precisely, does that mean you want __builtin_llabs instead of
>>> ::llabs?
>>> I thought the compiler knew they were the same.
>>
>>
>> Yes. Another reason is that it simplifies the implementation AND if
>> people want want to do something with the intrinsics' fallback
>> libstdc++ will gracefully deliver that.
>
>
> I don't see how that simplifies the implementation, it is several characters
> longer than ::llabs, and we still need to handle llabs.

You are on the wrong track if you are taking the number of characters
used in the implemetation.


> 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)?
>
> Note that I am happy to let you take over this PR if you like.
>
> --
> Marc Glisse


Re: abs(long long)

2012-10-02 Thread Daniel Krügler
2012/10/2 Marc Glisse :
> Here I am talking of a library issue: the wording that says that there are
> sufficient overloads such that integer types call the double version of math
> functions. It is fairly obvious that it doesn't apply to abs(long) for
> instance which has an explicit overload. For short or unsigned, I still read
> it as saying that it converts to double...

This really looks like a problem of the Standard Library specification
to me and
a corresponding issue should be submitted. In fact the wording can be
interpreted
that mixing  with  would imply two different versions of
std::abs(int) because of different required return types. I will
prepare a corresponding
submission to the LWG.

- Daniel


Re: abs(long long)

2012-10-02 Thread Marc Glisse

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


I understand that it is originally a library issue, but I don't think
it makes sense to resolve it in isolation of that core issue.


They seem mostly orthogonal to me, since the library only uses an informal 
language describing the desired outcome and not the actual overloads 
necessary to achieve it, whereas the core issue is about determining 
priorities for a non-ambiguous overload resolution (if we are talking 
about the same, where Jens Maurer has a proposal).



The library installed by the system was compiled with g++, and is then used
with clang++. If we can avoid installing 2 config.h files to make that
work...


Two things:
 1. that is clearly a clang problem.  I don't think it is libstdc++'s job
 tp try to solve clang's misguided configuration and installation.


Translated: libstdc++ should only ever be used with the very version of 
g++ that was used to compile it. clang++, icpc, sunCC, etc should never 
try to use a libstdc++ compiled with another compiler.


I am not saying libstdc++ should go to great lengths to support other 
compilers, but when it is actually easier to support them than not to...

(testing a macro is easier than a configure test)


 2. I am not sure you understand what I wrote: you can leave the
 use of the current macro the way it is if you appropriately
 define it in terms of what you want to change it to.


I was complaining about the configure-time nature of the macro. If it is 
defined at each compiler run based on __SIZEOF_INT128__, I am happy.



More precisely, does that mean you want __builtin_llabs instead of ::llabs?
I thought the compiler knew they were the same.


Yes. Another reason is that it simplifies the implementation AND if
people want want to do something with the intrinsics' fallback
libstdc++ will gracefully deliver that.


I don't see how that simplifies the implementation, it is several 
characters longer than ::llabs, and we still need to handle llabs. 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)?


Note that I am happy to let you take over this PR if you like.

--
Marc Glisse


Re: abs(long long)

2012-10-02 Thread Gabriel Dos Reis
On Tue, Oct 2, 2012 at 4:21 AM, Marc Glisse  wrote:
> On Tue, 2 Oct 2012, Gabriel Dos Reis wrote:
>
>> On Tue, Oct 2, 2012 at 3:57 AM, Marc Glisse  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?
>
>
> Here I am talking of a library issue: the wording that says that there are
> sufficient overloads such that integer types call the double version of math
> functions. It is fairly obvious that it doesn't apply to abs(long) for
> instance which has an explicit overload. For short or unsigned, I still read
> it as saying that it converts to double...

I understand that it is originally a library issue, but I don't think
it makes sense to resolve it in isolation of that core issue.


>
>
>>> * 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?
>
>
> The library installed by the system was compiled with g++, and is then used
> with clang++. If we can avoid installing 2 config.h files to make that
> work...

Two things:
  1. that is clearly a clang problem.  I don't think it is libstdc++'s job
  tp try to solve clang's misguided configuration and installation.

  2. I am not sure you understand what I wrote: you can leave the
  use of the current macro the way it is if you appropriately
  define it in terms of what you want to change it to.



>
>
>>> * 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.
>
>
> More precisely, does that mean you want __builtin_llabs instead of ::llabs?
> I thought the compiler knew they were the same.

Yes. Another reason is that it simplifies the implementation AND if
people want want to do something with the intrinsics' fallback
libstdc++ will gracefully deliver that.

-- Gaby


Re: abs(long long)

2012-10-02 Thread Marc Glisse

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


On Tue, Oct 2, 2012 at 3:57 AM, Marc Glisse  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?


Here I am talking of a library issue: the wording that says that there are 
sufficient overloads such that integer types call the double version of 
math functions. It is fairly obvious that it doesn't apply to abs(long) 
for instance which has an explicit overload. For short or unsigned, I 
still read it as saying that it converts to double...



* 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?


The library installed by the system was compiled with g++, and is then 
used with clang++. If we can avoid installing 2 config.h files to make 
that work...



* 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.


More precisely, does that mean you want __builtin_llabs instead of 
::llabs? I thought the compiler knew they were the same.


--
Marc Glisse


Re: abs(long long)

2012-10-02 Thread Gabriel Dos Reis
On Tue, Oct 2, 2012 at 3:57 AM, Marc Glisse  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  
>
> 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/header

abs(long long)

2012-10-02 Thread Marc Glisse

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


2012-10-02  Marc Glisse  

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 GlisseIndex: 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 

abs(long long)

2012-10-02 Thread Marc Glisse

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


2012-10-02  Marc Glisse  

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 GlisseIndex: 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.or