Re: [PATCH] Define 3-argument overloads of std::hypot for C++17 (P0030R1)
On 30/09/16 14:13 +0100, Szabolcs Nagy wrote: On 30/09/16 10:35, Szabolcs Nagy wrote: On 29/09/16 14:37, Andre Vieira (lists) wrote: On arm-none-eabi I'm seeing a failure for the long double type and inputs: { 1e-2l, 1e-4l, 1e-4l, 0.0150004999375l } The abs(frac) is higher than the toler: 1.73455e-16 vs 1e-16. Is that a reasonable difference? Should we raise toler3 to 1e-15? The last line is also too high: { 2147483647.l, 2147483647.l, 2147483647.l, 3719550785.027307813987l } Yields a frac of: 1.28198e-16 Those are the only ones that pass the 1e-16 threshold. i think the tolerance should be defined in terms of LDBL_EPSILON (or numeric_limits). n*LDBL_EPSILON tolerance would accept hypot with about n ulp error. now i see that there are huge ulp errors.. toler = 10*eps; should work for all formats, but currently there is >1000 ulp error on one of the double test cases.. so tolerance is carefully set to avoid triggering the failure there i'd set toler to 1*eps if this test case is not for testing hypot quality. Ed is already re-working the code, and probably the tests. I'd prefer to wait for his new implementation, but if the FAIL is a problem now then go ahead and adjust the tolerance.
Re: [PATCH] Define 3-argument overloads of std::hypot for C++17 (P0030R1)
On 30/09/16 10:35, Szabolcs Nagy wrote: > On 29/09/16 14:37, Andre Vieira (lists) wrote: >> >> On arm-none-eabi I'm seeing a failure for the long double type and inputs: >> { 1e-2l, 1e-4l, 1e-4l, 0.0150004999375l } >> >> The abs(frac) is higher than the toler: 1.73455e-16 vs 1e-16. Is that a >> reasonable difference? Should we raise toler3 to 1e-15? >> >> The last line is also too high: >> { 2147483647.l, 2147483647.l, 2147483647.l, 3719550785.027307813987l } >> Yields a frac of: 1.28198e-16 >> >> Those are the only ones that pass the 1e-16 threshold. >> > > i think the tolerance should be defined in > terms of LDBL_EPSILON (or numeric_limits). > > n*LDBL_EPSILON tolerance would accept hypot > with about n ulp error. > now i see that there are huge ulp errors.. toler = 10*eps; should work for all formats, but currently there is >1000 ulp error on one of the double test cases.. so tolerance is carefully set to avoid triggering the failure there i'd set toler to 1*eps if this test case is not for testing hypot quality.
Re: [PATCH] Define 3-argument overloads of std::hypot for C++17 (P0030R1)
On 29/09/16 14:37, Andre Vieira (lists) wrote: > > On arm-none-eabi I'm seeing a failure for the long double type and inputs: > { 1e-2l, 1e-4l, 1e-4l, 0.0150004999375l } > > The abs(frac) is higher than the toler: 1.73455e-16 vs 1e-16. Is that a > reasonable difference? Should we raise toler3 to 1e-15? > > The last line is also too high: > { 2147483647.l, 2147483647.l, 2147483647.l, 3719550785.027307813987l } > Yields a frac of: 1.28198e-16 > > Those are the only ones that pass the 1e-16 threshold. > i think the tolerance should be defined in terms of LDBL_EPSILON (or numeric_limits). n*LDBL_EPSILON tolerance would accept hypot with about n ulp error.
Re: [PATCH] Define 3-argument overloads of std::hypot for C++17 (P0030R1)
On 29/09/16 14:37 +0100, Andre Vieira (lists) wrote: Hi Jonathan, On 27/09/16 16:11, Jonathan Wakely wrote: The test might not be very good, but tests some small integer values and some other values where accuracy is lost for one or other of the alternative implementations mentioned above. If this FAILs for some 32-bit targets we might need to adjust the tolerances or the dg-options. On arm-none-eabi I'm seeing a failure for the long double type and inputs: { 1e-2l, 1e-4l, 1e-4l, 0.0150004999375l } The abs(frac) is higher than the toler: 1.73455e-16 vs 1e-16. Is that a reasonable difference? Should we raise toler3 to 1e-15? The last line is also too high: { 2147483647.l, 2147483647.l, 2147483647.l, 3719550785.027307813987l } Yields a frac of: 1.28198e-16 Those are the only ones that pass the 1e-16 threshold. Yes, it makes sense to raise it, although Ed's reworking the code and tests anyway.
Re: [PATCH] Define 3-argument overloads of std::hypot for C++17 (P0030R1)
Hi Jonathan, On 27/09/16 16:11, Jonathan Wakely wrote: > > The test might not be very good, but tests some small integer values > and some other values where accuracy is lost for one or other of the > alternative implementations mentioned above. If this FAILs for some > 32-bit targets we might need to adjust the tolerances or the > dg-options. On arm-none-eabi I'm seeing a failure for the long double type and inputs: { 1e-2l, 1e-4l, 1e-4l, 0.0150004999375l } The abs(frac) is higher than the toler: 1.73455e-16 vs 1e-16. Is that a reasonable difference? Should we raise toler3 to 1e-15? The last line is also too high: { 2147483647.l, 2147483647.l, 2147483647.l, 3719550785.027307813987l } Yields a frac of: 1.28198e-16 Those are the only ones that pass the 1e-16 threshold. Cheers, Andre
Re: [PATCH] Define 3-argument overloads of std::hypot for C++17 (P0030R1)
On 29/09/16 13:12 +0100, Jonathan Wakely wrote: On 29/09/16 14:02 +0200, Rainer Orth wrote: Hi Jonathan, If only there was some way the Solaris team could contact us so we could coordinate and stop adding more and more hacks to mess with each others headers. But I assume they don't have access to the www or email, because the only other explanation is too rude to say on a public list. Presumably they now provide all the declarations for C++11, so we don't need to declare them. Moving the new hypot overload outside the still this doesn't need into , but into libstdc++ configury IMNSHO. I agree. Presumably they wanted their updated headers to work with existing GCC releases, and not have to wait until we modified our configury to cooperate with their changes. But if they simply communicated with us we could plan for that kind of thing in advance, and ensure we don't keep trying to fix each others headers from a distance. _GLIBCXX_USE_C99_MATH_TR1 block should mean it's declared. That will break when Solaris adds C++17 support, so we'd better add some macro to guard the new hypot overloads, so they can be disabled again. Let's call it __CORRECT_ISO_CPP17_MATH_H_PROTO (and maybe add a coment like "Dear Solaris team, ..."). ... which assumes they do read this ;-) Indeed. Since they seem to be adding declarations to and only in the global namespace, and the 3-argument form doesn't exist in the global namespace, maybe we can assume they aren't going to add it to . Does this work? It does indeed, at least running the single testcase with runtest now passes. Thanks, I'll commit that soon. Herre's what I committed, without a CORRECT_ISO_CPP17_MATH_H_PROTO macro check (which I'd put in the wrong place anyway :-) Tested powerpc64le-linux, committed to trunk. commit e253b8464a024a062dc01f0eaad66b10e3132374 Author: Jonathan Wakely Date: Thu Sep 29 13:41:55 2016 +0100 Define C++17 std::hypot without _GLIBCXX_USE_C99_MATH_TR1 * include/c_global/cmath (hypot, __hypot3): Move C++17 overloads outside _GLIBCXX_USE_C99_MATH_TR1 condition. diff --git a/libstdc++-v3/include/c_global/cmath b/libstdc++-v3/include/c_global/cmath index fffa0e7..0e7c4ad 100644 --- a/libstdc++-v3/include/c_global/cmath +++ b/libstdc++-v3/include/c_global/cmath @@ -1455,46 +1455,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION return hypot(__type(__x), __type(__y)); } -#if __cplusplus > 201402L -#define __cpp_lib_hypot 201603 - // [c.math.hypot3], three-dimensional hypotenuse - - template -inline _Tp -__hypot3(_Tp __x, _Tp __y, _Tp __z) -{ - __x = std::abs(__x); - __y = std::abs(__y); - __z = std::abs(__z); - if (_Tp __a = __x < __y ? __y < __z ? __z : __y : __x < __z ? __z : __x) - return __a * std::sqrt((__x / __a) * (__x / __a) - + (__y / __a) * (__y / __a) - + (__z / __a) * (__z / __a)); - else - return {}; -} - - inline float - hypot(float __x, float __y, float __z) - { return std::__hypot3(__x, __y, __z); } - - inline double - hypot(double __x, double __y, double __z) - { return std::__hypot3(__x, __y, __z); } - - inline long double - hypot(long double __x, long double __y, long double __z) - { return std::__hypot3(__x, __y, __z); } - - template -typename __gnu_cxx::__promote_3<_Tp, _Up, _Vp>::__type -hypot(_Tp __x, _Up __y, _Vp __z) -{ - using __type = typename __gnu_cxx::__promote_3<_Tp, _Up, _Vp>::__type; - return std::__hypot3<__type>(__x, __y, __z); -} -#endif // C++17 - #ifndef __CORRECT_ISO_CPP11_MATH_H_PROTO constexpr int ilogb(float __x) @@ -1830,6 +1790,53 @@ _GLIBCXX_END_NAMESPACE_VERSION #endif // C++11 +#if __cplusplus > 201402L +namespace std _GLIBCXX_VISIBILITY(default) +{ +_GLIBCXX_BEGIN_NAMESPACE_VERSION + + // [c.math.hypot3], three-dimensional hypotenuse +#define __cpp_lib_hypot 201603 + + template +inline _Tp +__hypot3(_Tp __x, _Tp __y, _Tp __z) +{ + __x = std::abs(__x); + __y = std::abs(__y); + __z = std::abs(__z); + if (_Tp __a = __x < __y ? __y < __z ? __z : __y : __x < __z ? __z : __x) + return __a * std::sqrt((__x / __a) * (__x / __a) + + (__y / __a) * (__y / __a) + + (__z / __a) * (__z / __a)); + else + return {}; +} + + inline float + hypot(float __x, float __y, float __z) + { return std::__hypot3(__x, __y, __z); } + + inline double + hypot(double __x, double __y, double __z) + { return std::__hypot3(__x, __y, __z); } + + inline long double + hypot(long double __x, long double __y, long double __z) + { return std::__hypot3(__x, __y, __z); } + + template +typename __gnu_cxx::__promote_3<_Tp, _Up, _Vp>::__type +hypot(_Tp __x, _Up __y, _Vp __z) +{ + using __type = typename __gnu_cxx::__promote_3<_Tp, _Up, _Vp>::__type; + return std::__hypot3<__type>(__x, __y, __z); +} +_GLIBCXX_END_NAMESPACE_VERSION +} // namespace +#endif // C++17 + + #if _GLIBCXX_USE_STD_SPEC_
Re: [PATCH] Define 3-argument overloads of std::hypot for C++17 (P0030R1)
On 29/09/16 14:02 +0200, Rainer Orth wrote: Hi Jonathan, If only there was some way the Solaris team could contact us so we could coordinate and stop adding more and more hacks to mess with each others headers. But I assume they don't have access to the www or email, because the only other explanation is too rude to say on a public list. Presumably they now provide all the declarations for C++11, so we don't need to declare them. Moving the new hypot overload outside the still this doesn't need into , but into libstdc++ configury IMNSHO. I agree. Presumably they wanted their updated headers to work with existing GCC releases, and not have to wait until we modified our configury to cooperate with their changes. But if they simply communicated with us we could plan for that kind of thing in advance, and ensure we don't keep trying to fix each others headers from a distance. _GLIBCXX_USE_C99_MATH_TR1 block should mean it's declared. That will break when Solaris adds C++17 support, so we'd better add some macro to guard the new hypot overloads, so they can be disabled again. Let's call it __CORRECT_ISO_CPP17_MATH_H_PROTO (and maybe add a coment like "Dear Solaris team, ..."). ... which assumes they do read this ;-) Indeed. Since they seem to be adding declarations to and only in the global namespace, and the 3-argument form doesn't exist in the global namespace, maybe we can assume they aren't going to add it to . Does this work? It does indeed, at least running the single testcase with runtest now passes. Thanks, I'll commit that soon.
Re: [PATCH] Define 3-argument overloads of std::hypot for C++17 (P0030R1)
Hi Jonathan, >>If only there was some way the Solaris team could contact us so we >>could coordinate and stop adding more and more hacks to mess with each >>others headers. But I assume they don't have access to the www or >>email, because the only other explanation is too rude to say on a >>public list. > > Presumably they now provide all the declarations for C++11, so we > don't need to declare them. Moving the new hypot overload outside the still this doesn't need into , but into libstdc++ configury IMNSHO. > _GLIBCXX_USE_C99_MATH_TR1 block should mean it's declared. That will > break when Solaris adds C++17 support, so we'd better add some macro > to guard the new hypot overloads, so they can be disabled again. Let's > call it __CORRECT_ISO_CPP17_MATH_H_PROTO (and maybe add a coment like > "Dear Solaris team, ..."). ... which assumes they do read this ;-) > Does this work? It does indeed, at least running the single testcase with runtest now passes. Thanks. Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University
Re: [PATCH] Define 3-argument overloads of std::hypot for C++17 (P0030R1)
On 29/09/16 11:47 +0100, Jonathan Wakely wrote: On 29/09/16 12:39 +0200, Rainer Orth wrote: Hi Jonathan, That would suggest Solaris uses include/c_std/cmath (where I forgot to add the new overloads) rather than include/c_global/cmath ... is that right? Alternatively it's using c_global/cmath but _GLIBCXX_USE_C99_MATH_TR1 is not defined, as the new overloads are inside that block. I thought that was defined for Solaris though, which is why we have the __CORRECT_ISO_CPP11_MATH_H_PROTO there in that file. it is indeed, at least initially. What I see in preprocessed input is this: ro@galeras 208 > grep _GLIBCXX_USE_C99_MATH testsuite/normal4/hypot.ii #define _GLIBCXX_USE_C99_MATH _GLIBCXX11_USE_C99_MATH #define _GLIBCXX_USE_C99_MATH_TR1 1 #undef _GLIBCXX_USE_C99_MATH #undef _GLIBCXX_USE_C99_MATH_TR1 It turns out the #undef's are from : #if __cplusplus >= 201103L #undef _GLIBCXX_USE_C99_MATH #undef _GLIBCXX_USE_C99_MATH_TR1 #endif No idea what this nonsense is trying to accomplish! It's already in Solaris 11.3, however. Wow. If only there was some way the Solaris team could contact us so we could coordinate and stop adding more and more hacks to mess with each others headers. But I assume they don't have access to the www or email, because the only other explanation is too rude to say on a public list. Presumably they now provide all the declarations for C++11, so we don't need to declare them. Moving the new hypot overload outside the _GLIBCXX_USE_C99_MATH_TR1 block should mean it's declared. That will break when Solaris adds C++17 support, so we'd better add some macro to guard the new hypot overloads, so they can be disabled again. Let's call it __CORRECT_ISO_CPP17_MATH_H_PROTO (and maybe add a coment like "Dear Solaris team, ..."). Does this work? diff --git a/libstdc++-v3/include/c_global/cmath b/libstdc++-v3/include/c_global/cmath index fffa0e7..86ffd34 100644 --- a/libstdc++-v3/include/c_global/cmath +++ b/libstdc++-v3/include/c_global/cmath @@ -1455,46 +1455,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION return hypot(__type(__x), __type(__y)); } -#if __cplusplus > 201402L -#define __cpp_lib_hypot 201603 - // [c.math.hypot3], three-dimensional hypotenuse - - template -inline _Tp -__hypot3(_Tp __x, _Tp __y, _Tp __z) -{ - __x = std::abs(__x); - __y = std::abs(__y); - __z = std::abs(__z); - if (_Tp __a = __x < __y ? __y < __z ? __z : __y : __x < __z ? __z : __x) - return __a * std::sqrt((__x / __a) * (__x / __a) - + (__y / __a) * (__y / __a) - + (__z / __a) * (__z / __a)); - else - return {}; -} - - inline float - hypot(float __x, float __y, float __z) - { return std::__hypot3(__x, __y, __z); } - - inline double - hypot(double __x, double __y, double __z) - { return std::__hypot3(__x, __y, __z); } - - inline long double - hypot(long double __x, long double __y, long double __z) - { return std::__hypot3(__x, __y, __z); } - - template -typename __gnu_cxx::__promote_3<_Tp, _Up, _Vp>::__type -hypot(_Tp __x, _Up __y, _Vp __z) -{ - using __type = typename __gnu_cxx::__promote_3<_Tp, _Up, _Vp>::__type; - return std::__hypot3<__type>(__x, __y, __z); -} -#endif // C++17 - #ifndef __CORRECT_ISO_CPP11_MATH_H_PROTO constexpr int ilogb(float __x) @@ -1830,6 +1790,55 @@ _GLIBCXX_END_NAMESPACE_VERSION #endif // C++11 +#if __cplusplus > 201402L +namespace std _GLIBCXX_VISIBILITY(default) +{ +_GLIBCXX_BEGIN_NAMESPACE_VERSION + +#ifndef __CORRECT_ISO_CPP17_MATH_H_PROTO + // [c.math.hypot3], three-dimensional hypotenuse +#define __cpp_lib_hypot 201603 + + template +inline _Tp +__hypot3(_Tp __x, _Tp __y, _Tp __z) +{ + __x = std::abs(__x); + __y = std::abs(__y); + __z = std::abs(__z); + if (_Tp __a = __x < __y ? __y < __z ? __z : __y : __x < __z ? __z : __x) + return __a * std::sqrt((__x / __a) * (__x / __a) + + (__y / __a) * (__y / __a) + + (__z / __a) * (__z / __a)); + else + return {}; +} + + inline float + hypot(float __x, float __y, float __z) + { return std::__hypot3(__x, __y, __z); } + + inline double + hypot(double __x, double __y, double __z) + { return std::__hypot3(__x, __y, __z); } + + inline long double + hypot(long double __x, long double __y, long double __z) + { return std::__hypot3(__x, __y, __z); } + + template +typename __gnu_cxx::__promote_3<_Tp, _Up, _Vp>::__type +hypot(_Tp __x, _Up __y, _Vp __z) +{ + using __type = typename __gnu_cxx::__promote_3<_Tp, _Up, _Vp>::__type; + return std::__hypot3<__type>(__x, __y, __z); +#endif +} +_GLIBCXX_END_NAMESPACE_VERSION +} // namespace +#endif // C++17 + + #if _GLIBCXX_USE_STD_SPEC_FUNCS # include #endif
Re: [PATCH] Define 3-argument overloads of std::hypot for C++17 (P0030R1)
Hi Jonathan, >>It turns out the #undef's are from : >> >>#if __cplusplus >= 201103L >>#undef _GLIBCXX_USE_C99_MATH >>#undef _GLIBCXX_USE_C99_MATH_TR1 >>#endif >> >>No idea what this nonsense is trying to accomplish! It's already in >>Solaris 11.3, however. > > Wow. > > If only there was some way the Solaris team could contact us so we > could coordinate and stop adding more and more hacks to mess with each > others headers. But I assume they don't have access to the www or > email, because the only other explanation is too rude to say on a > public list. it very much depends who you are dealing with: some are quite good at coordinating with other/external groups, while others, well, don't get me started... I'll see what I can dig up here: my goal has been to get rid of the need for fixincludes on newer Solaris versions, but progress has been slow ;-( We'll probably have to undo this via fixincludes since at least on Solaris 11.x it's already in the wild. Maybe this can be stopped before Solaris 12 ships, though... >>> Once again I wish we had a Solaris machine in the compile farm, or it >>> was possible to install a Solaris VM and get OS updates without paying >>> Oracle for the privilege. >> >>That's easily doable: Solaris is free for development use; you get >>access to the release (11, 11.1, 11.2, 11.3, ...) versions, just not to >>patches/updates. > > Yes, but because the libc headers get changed by patches and updates, > I found it was useless to install it in a VM. Because I had outdated > headers I couldn't test how our build system interacts with a fully > updated version of Solaris. True: there have certainly issues like this in the past; that's why I'm trying to keep even older Solaris versions up to date WRT patches. >>They do have a program for free academic use of >>Solaris, including updates, these days: https://academy.oracle.com/. > > That might be useful, I'll see if I qualify. Thanks. Another option might be to get Oracle grant a similar exception to the compile farm. I'll ask around if there's any chance of this. Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University
Re: [PATCH] Define 3-argument overloads of std::hypot for C++17 (P0030R1)
On 29/09/16 12:39 +0200, Rainer Orth wrote: Hi Jonathan, That would suggest Solaris uses include/c_std/cmath (where I forgot to add the new overloads) rather than include/c_global/cmath ... is that right? Alternatively it's using c_global/cmath but _GLIBCXX_USE_C99_MATH_TR1 is not defined, as the new overloads are inside that block. I thought that was defined for Solaris though, which is why we have the __CORRECT_ISO_CPP11_MATH_H_PROTO there in that file. it is indeed, at least initially. What I see in preprocessed input is this: ro@galeras 208 > grep _GLIBCXX_USE_C99_MATH testsuite/normal4/hypot.ii #define _GLIBCXX_USE_C99_MATH _GLIBCXX11_USE_C99_MATH #define _GLIBCXX_USE_C99_MATH_TR1 1 #undef _GLIBCXX_USE_C99_MATH #undef _GLIBCXX_USE_C99_MATH_TR1 It turns out the #undef's are from : #if __cplusplus >= 201103L #undef _GLIBCXX_USE_C99_MATH #undef _GLIBCXX_USE_C99_MATH_TR1 #endif No idea what this nonsense is trying to accomplish! It's already in Solaris 11.3, however. Wow. If only there was some way the Solaris team could contact us so we could coordinate and stop adding more and more hacks to mess with each others headers. But I assume they don't have access to the www or email, because the only other explanation is too rude to say on a public list. Once again I wish we had a Solaris machine in the compile farm, or it was possible to install a Solaris VM and get OS updates without paying Oracle for the privilege. That's easily doable: Solaris is free for development use; you get access to the release (11, 11.1, 11.2, 11.3, ...) versions, just not to patches/updates. Yes, but because the libc headers get changed by patches and updates, I found it was useless to install it in a VM. Because I had outdated headers I couldn't test how our build system interacts with a fully updated version of Solaris. They do have a program for free academic use of Solaris, including updates, these days: https://academy.oracle.com/. That might be useful, I'll see if I qualify. Thanks.
Re: [PATCH] Define 3-argument overloads of std::hypot for C++17 (P0030R1)
Hi Jonathan, >>That would suggest Solaris uses include/c_std/cmath (where I forgot to >>add the new overloads) rather than include/c_global/cmath ... is that >>right? > > Alternatively it's using c_global/cmath but _GLIBCXX_USE_C99_MATH_TR1 > is not defined, as the new overloads are inside that block. I thought > that was defined for Solaris though, which is why we have the > __CORRECT_ISO_CPP11_MATH_H_PROTO there in that file. it is indeed, at least initially. What I see in preprocessed input is this: ro@galeras 208 > grep _GLIBCXX_USE_C99_MATH testsuite/normal4/hypot.ii #define _GLIBCXX_USE_C99_MATH _GLIBCXX11_USE_C99_MATH #define _GLIBCXX_USE_C99_MATH_TR1 1 #undef _GLIBCXX_USE_C99_MATH #undef _GLIBCXX_USE_C99_MATH_TR1 It turns out the #undef's are from : #if __cplusplus >= 201103L #undef _GLIBCXX_USE_C99_MATH #undef _GLIBCXX_USE_C99_MATH_TR1 #endif No idea what this nonsense is trying to accomplish! It's already in Solaris 11.3, however. > Once again I wish we had a Solaris machine in the compile farm, or it > was possible to install a Solaris VM and get OS updates without paying > Oracle for the privilege. That's easily doable: Solaris is free for development use; you get access to the release (11, 11.1, 11.2, 11.3, ...) versions, just not to patches/updates. They do have a program for free academic use of Solaris, including updates, these days: https://academy.oracle.com/. Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University
Re: [PATCH] Define 3-argument overloads of std::hypot for C++17 (P0030R1)
Hi Jonathan, > That would suggest Solaris uses include/c_std/cmath (where I forgot to > add the new overloads) rather than include/c_global/cmath ... is that > right? no, include/cmath points to the c_global version. Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University
Re: [PATCH] Define 3-argument overloads of std::hypot for C++17 (P0030R1)
On 29/09/16 10:10 +0100, Jonathan Wakely wrote: On 29/09/16 10:54 +0200, Rainer Orth wrote: Hi Jonathan, This adds the new 3D std::hypot() functions. This implementation seems to be faster than the naïve sqrt(x*x + y*y + z*z) implementation, or hypot(hypot(x, y), z), and should be a bit more accurate at very large or very small values due to reducing the arguments by the largest one. Improvements welcome though, as this is not my forte. The test might not be very good, but tests some small integer values and some other values where accuracy is lost for one or other of the alternative implementations mentioned above. If this FAILs for some 32-bit targets we might need to adjust the tolerances or the dg-options. * doc/xml/manual/status_cxx2017.xml: Update status. * include/c_global/cmath (hypot): Add three-dimensional overloads. * testsuite/26_numerics/headers/cmath/hypot.cc: New. Tested powerpc64le-linux and x86_64-linux, committed to trunk. the new test currently FAILs on Solaris 12 (both SPARC and x86): +FAIL: 26_numerics/headers/cmath/hypot.cc (test for excess errors) +WARNING: 26_numerics/headers/cmath/hypot.cc compilation failed to produce execu table Excess errors: /vol/gcc/src/hg/trunk/local/libstdc++-v3/testsuite/26_numerics/headers/cmath/hypot.cc:38: error: no matching function for call to 'hypot(double, double, double)' /vol/gcc/src/hg/trunk/local/libstdc++-v3/testsuite/26_numerics/headers/cmath/hypot.cc:38: error: no matching function for call to 'hypot(double, double, double)' /vol/gcc/src/hg/trunk/local/libstdc++-v3/testsuite/26_numerics/headers/cmath/hypot.cc:38: error: template argument 2 is invalid and many more. That would suggest Solaris uses include/c_std/cmath (where I forgot to add the new overloads) rather than include/c_global/cmath ... is that right? Alternatively it's using c_global/cmath but _GLIBCXX_USE_C99_MATH_TR1 is not defined, as the new overloads are inside that block. I thought that was defined for Solaris though, which is why we have the __CORRECT_ISO_CPP11_MATH_H_PROTO there in that file. Once again I wish we had a Solaris machine in the compile farm, or it was possible to install a Solaris VM and get OS updates without paying Oracle for the privilege.
Re: [PATCH] Define 3-argument overloads of std::hypot for C++17 (P0030R1)
On 29/09/16 10:54 +0200, Rainer Orth wrote: Hi Jonathan, This adds the new 3D std::hypot() functions. This implementation seems to be faster than the naïve sqrt(x*x + y*y + z*z) implementation, or hypot(hypot(x, y), z), and should be a bit more accurate at very large or very small values due to reducing the arguments by the largest one. Improvements welcome though, as this is not my forte. The test might not be very good, but tests some small integer values and some other values where accuracy is lost for one or other of the alternative implementations mentioned above. If this FAILs for some 32-bit targets we might need to adjust the tolerances or the dg-options. * doc/xml/manual/status_cxx2017.xml: Update status. * include/c_global/cmath (hypot): Add three-dimensional overloads. * testsuite/26_numerics/headers/cmath/hypot.cc: New. Tested powerpc64le-linux and x86_64-linux, committed to trunk. the new test currently FAILs on Solaris 12 (both SPARC and x86): +FAIL: 26_numerics/headers/cmath/hypot.cc (test for excess errors) +WARNING: 26_numerics/headers/cmath/hypot.cc compilation failed to produce execu table Excess errors: /vol/gcc/src/hg/trunk/local/libstdc++-v3/testsuite/26_numerics/headers/cmath/hypot.cc:38: error: no matching function for call to 'hypot(double, double, double)' /vol/gcc/src/hg/trunk/local/libstdc++-v3/testsuite/26_numerics/headers/cmath/hypot.cc:38: error: no matching function for call to 'hypot(double, double, double)' /vol/gcc/src/hg/trunk/local/libstdc++-v3/testsuite/26_numerics/headers/cmath/hypot.cc:38: error: template argument 2 is invalid and many more. That would suggest Solaris uses include/c_std/cmath (where I forgot to add the new overloads) rather than include/c_global/cmath ... is that right?
Re: [PATCH] Define 3-argument overloads of std::hypot for C++17 (P0030R1)
Hi Jonathan, > This adds the new 3D std::hypot() functions. This implementation seems > to be faster than the naïve sqrt(x*x + y*y + z*z) implementation, or > hypot(hypot(x, y), z), and should be a bit more accurate at very large > or very small values due to reducing the arguments by the largest one. > Improvements welcome though, as this is not my forte. > > The test might not be very good, but tests some small integer values > and some other values where accuracy is lost for one or other of the > alternative implementations mentioned above. If this FAILs for some > 32-bit targets we might need to adjust the tolerances or the > dg-options. > > * doc/xml/manual/status_cxx2017.xml: Update status. > * include/c_global/cmath (hypot): Add three-dimensional overloads. > * testsuite/26_numerics/headers/cmath/hypot.cc: New. > > Tested powerpc64le-linux and x86_64-linux, committed to trunk. the new test currently FAILs on Solaris 12 (both SPARC and x86): +FAIL: 26_numerics/headers/cmath/hypot.cc (test for excess errors) +WARNING: 26_numerics/headers/cmath/hypot.cc compilation failed to produce execu table Excess errors: /vol/gcc/src/hg/trunk/local/libstdc++-v3/testsuite/26_numerics/headers/cmath/hypot.cc:38: error: no matching function for call to 'hypot(double, double, double)' /vol/gcc/src/hg/trunk/local/libstdc++-v3/testsuite/26_numerics/headers/cmath/hypot.cc:38: error: no matching function for call to 'hypot(double, double, double)' /vol/gcc/src/hg/trunk/local/libstdc++-v3/testsuite/26_numerics/headers/cmath/hypot.cc:38: error: template argument 2 is invalid and many more. Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University
Re: [PATCH] Define 3-argument overloads of std::hypot for C++17 (P0030R1)
On 28/09/16 12:40 +, Joseph Myers wrote: On Wed, 28 Sep 2016, Jonathan Wakely wrote: On 27/09/16 23:28 +, Joseph Myers wrote: > On Tue, 27 Sep 2016, Jonathan Wakely wrote: > > > This adds the new 3D std::hypot() functions. This implementation seems > > to be faster than the naïve sqrt(x*x + y*y + z*z) implementation, or > > hypot(hypot(x, y), z), and should be a bit more accurate at very large > > or very small values due to reducing the arguments by the largest one. > > Improvements welcome though, as this is not my forte. > > Should I take it from this implementation that C++ is not concerned by > certain considerations that would arise for C: spurious underflow > exceptions from the scaling when some arguments much larger than others; > spurious "invalid" exceptions from the comparisons when any argument is > (quiet) NaN; handling of mixed (quiet) NaN and Inf arguments (where ISO C > Annex F has Inf returned, not NaN)? The entire spec from the C++ draft is: Returns: √ x^2 + y^2 + z^2 As a further issue: even if you ignore exceptions and Annex F issues, and don't have NaN arguments, if you have an Inf argument it will be the largest, and so your code will do Inf/Inf, and so return NaN, if any argument is Inf (when obviously the correct answer in that case is Inf). I've opened https://gcc.gnu.org/PR6 but I don't plan to work on it.
Re: [PATCH] Define 3-argument overloads of std::hypot for C++17 (P0030R1)
On Wed, 28 Sep 2016, Jonathan Wakely wrote: > On 27/09/16 23:28 +, Joseph Myers wrote: > > On Tue, 27 Sep 2016, Jonathan Wakely wrote: > > > > > This adds the new 3D std::hypot() functions. This implementation seems > > > to be faster than the naïve sqrt(x*x + y*y + z*z) implementation, or > > > hypot(hypot(x, y), z), and should be a bit more accurate at very large > > > or very small values due to reducing the arguments by the largest one. > > > Improvements welcome though, as this is not my forte. > > > > Should I take it from this implementation that C++ is not concerned by > > certain considerations that would arise for C: spurious underflow > > exceptions from the scaling when some arguments much larger than others; > > spurious "invalid" exceptions from the comparisons when any argument is > > (quiet) NaN; handling of mixed (quiet) NaN and Inf arguments (where ISO C > > Annex F has Inf returned, not NaN)? > > The entire spec from the C++ draft is: Returns: √ x^2 + y^2 + z^2 As a further issue: even if you ignore exceptions and Annex F issues, and don't have NaN arguments, if you have an Inf argument it will be the largest, and so your code will do Inf/Inf, and so return NaN, if any argument is Inf (when obviously the correct answer in that case is Inf). -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH] Define 3-argument overloads of std::hypot for C++17 (P0030R1)
On 28/09/16 13:31 +0200, Marc Glisse wrote: On Wed, 28 Sep 2016, Jonathan Wakely wrote: On 27/09/16 23:28 +, Joseph Myers wrote: On Tue, 27 Sep 2016, Jonathan Wakely wrote: This adds the new 3D std::hypot() functions. This implementation seems to be faster than the naïve sqrt(x*x + y*y + z*z) implementation, or hypot(hypot(x, y), z), and should be a bit more accurate at very large or very small values due to reducing the arguments by the largest one. Improvements welcome though, as this is not my forte. Should I take it from this implementation that C++ is not concerned by certain considerations that would arise for C: spurious underflow exceptions from the scaling when some arguments much larger than others; spurious "invalid" exceptions from the comparisons when any argument is (quiet) NaN; handling of mixed (quiet) NaN and Inf arguments (where ISO C Annex F has Inf returned, not NaN)? The entire spec from the C++ draft is: Returns: √ x^2 + y^2 + z^2 It doesn't mention "without undue overflow or underflow" as the C standard does for hypot(x, y). Handling those conditions would of course be nice, but I'm not capable of doing so, and I'd rather have a poor implementation than none. p0030r0 had "In addition to the two-argument versions of certain math functions in and its float and long double overloads, C++ adds three-argument versions of these functions that follow similar semantics as their two-argument counterparts." One could interpret that as an intent to follow the C semantics. On the other hand, the paper mentions speed and convenience as the main selling points. Maybe LWG could clarify the intent? I expect the answer will be that it's QoI.
Re: [PATCH] Define 3-argument overloads of std::hypot for C++17 (P0030R1)
On Wed, 28 Sep 2016, Jonathan Wakely wrote: On 27/09/16 23:28 +, Joseph Myers wrote: On Tue, 27 Sep 2016, Jonathan Wakely wrote: This adds the new 3D std::hypot() functions. This implementation seems to be faster than the naïve sqrt(x*x + y*y + z*z) implementation, or hypot(hypot(x, y), z), and should be a bit more accurate at very large or very small values due to reducing the arguments by the largest one. Improvements welcome though, as this is not my forte. Should I take it from this implementation that C++ is not concerned by certain considerations that would arise for C: spurious underflow exceptions from the scaling when some arguments much larger than others; spurious "invalid" exceptions from the comparisons when any argument is (quiet) NaN; handling of mixed (quiet) NaN and Inf arguments (where ISO C Annex F has Inf returned, not NaN)? The entire spec from the C++ draft is: Returns: √ x^2 + y^2 + z^2 It doesn't mention "without undue overflow or underflow" as the C standard does for hypot(x, y). Handling those conditions would of course be nice, but I'm not capable of doing so, and I'd rather have a poor implementation than none. p0030r0 had "In addition to the two-argument versions of certain math functions in and its float and long double overloads, C++ adds three-argument versions of these functions that follow similar semantics as their two-argument counterparts." One could interpret that as an intent to follow the C semantics. On the other hand, the paper mentions speed and convenience as the main selling points. Maybe LWG could clarify the intent? -- Marc Glisse
Re: [PATCH] Define 3-argument overloads of std::hypot for C++17 (P0030R1)
On 27/09/16 23:50 +0200, Marc Glisse wrote: On Tue, 27 Sep 2016, Jonathan Wakely wrote: This adds the new 3D std::hypot() functions. This implementation seems to be faster than the naïve sqrt(x*x + y*y + z*z) implementation, or hypot(hypot(x, y), z), and should be a bit more accurate at very large or very small values due to reducing the arguments by the largest one. Improvements welcome though, as this is not my forte. I understand the claims about accuracy, but the one about speed seems very surprising to me. Was your test specifically with denormals on not-so-recent hardware, or specifically when sqrt does a library call for errno? Otherwise, I have a hard time believing that 3 multiplications and 2 additions can be slower than 4 multiplications, 2 additions, plus a bunch of tests and divisions. I didn't test any denormals, just similar inputs to the ones in the new testcase. Looking at my crappy benchmark again it seems I'd commented out the sqrt(x*x+y*y+z*z) version in favour of a call to gsl_hypot3 ... oops! So what I committed is faster than hypot(hypot(x, y), z) but as you guessed, slower than the simple sqrt(x*x+y*y+z*z). I'm happy to defer to you in terms of what would be a better implementation, as I'm very unlikely to ever need to use this. I don't know whether we should be optimizing for speed or accuracy. But since nobody else had contributed an implementation (and this was the one C++17 feature libc++ has that we don't :-) I decided to add it.
Re: [PATCH] Define 3-argument overloads of std::hypot for C++17 (P0030R1)
On 27/09/16 23:28 +, Joseph Myers wrote: On Tue, 27 Sep 2016, Jonathan Wakely wrote: This adds the new 3D std::hypot() functions. This implementation seems to be faster than the naïve sqrt(x*x + y*y + z*z) implementation, or hypot(hypot(x, y), z), and should be a bit more accurate at very large or very small values due to reducing the arguments by the largest one. Improvements welcome though, as this is not my forte. Should I take it from this implementation that C++ is not concerned by certain considerations that would arise for C: spurious underflow exceptions from the scaling when some arguments much larger than others; spurious "invalid" exceptions from the comparisons when any argument is (quiet) NaN; handling of mixed (quiet) NaN and Inf arguments (where ISO C Annex F has Inf returned, not NaN)? The entire spec from the C++ draft is: Returns: √ x^2 + y^2 + z^2 It doesn't mention "without undue overflow or underflow" as the C standard does for hypot(x, y). Handling those conditions would of course be nice, but I'm not capable of doing so, and I'd rather have a poor implementation than none. Again, improvements are welcome. I'm not the right person to do that.
Re: [PATCH] Define 3-argument overloads of std::hypot for C++17 (P0030R1)
On Tue, 27 Sep 2016, Jonathan Wakely wrote: > This adds the new 3D std::hypot() functions. This implementation seems > to be faster than the naïve sqrt(x*x + y*y + z*z) implementation, or > hypot(hypot(x, y), z), and should be a bit more accurate at very large > or very small values due to reducing the arguments by the largest one. > Improvements welcome though, as this is not my forte. Should I take it from this implementation that C++ is not concerned by certain considerations that would arise for C: spurious underflow exceptions from the scaling when some arguments much larger than others; spurious "invalid" exceptions from the comparisons when any argument is (quiet) NaN; handling of mixed (quiet) NaN and Inf arguments (where ISO C Annex F has Inf returned, not NaN)? -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH] Define 3-argument overloads of std::hypot for C++17 (P0030R1)
On Tue, 27 Sep 2016, Jonathan Wakely wrote: This adds the new 3D std::hypot() functions. This implementation seems to be faster than the naïve sqrt(x*x + y*y + z*z) implementation, or hypot(hypot(x, y), z), and should be a bit more accurate at very large or very small values due to reducing the arguments by the largest one. Improvements welcome though, as this is not my forte. I understand the claims about accuracy, but the one about speed seems very surprising to me. Was your test specifically with denormals on not-so-recent hardware, or specifically when sqrt does a library call for errno? Otherwise, I have a hard time believing that 3 multiplications and 2 additions can be slower than 4 multiplications, 2 additions, plus a bunch of tests and divisions. -- Marc Glisse