Message ID | 20171114132152.22775-1-gabriel@inconstante.eti.br |
---|---|
State | New |
Headers | show |
Series | [v3] Provide a C++ version of iseqsig | expand |
On 11/14/2017 02:21 PM, Gabriel F. T. Gomes wrote: > diff --git a/math/math.h b/math/math.h > index 326fd8ebe1..acc909d37f 100644 > --- a/math/math.h > +++ b/math/math.h > @@ -1152,8 +1152,76 @@ iszero (__T __val) > > /* Return X == Y but raising "invalid" and setting errno if X or Y is > a NaN. */ > -# define iseqsig(x, y) \ > - __MATH_TG (__MATH_EVAL_FMT2 (x, y), __iseqsig, ((x), (y))) > +# if !defined __cplusplus || (__cplusplus < 201103L && !defined __GNUC__) > +# define iseqsig(x, y) \ > + __MATH_TG (__MATH_EVAL_FMT2 (x, y), __iseqsig, ((x), (y))) > +# else > +/* In C++ mode, __MATH_TG cannot be used, because it relies on > + __builtin_types_compatible_p, which is a C-only builtin. Moreover, > + the comparison macros from ISO C take two floating-point arguments, > + which need not have the same type. Choosing what underlying function > + to call requires evaluating the formats of the arguments, then > + selecting which is wider. The macro __MATH_EVAL_FMT2 provides this > + information, however, only the type of the macro expansion is > + relevant (actually evaluating the expression would be incorrect). > + Thus, the type is used as a template parameter for __iseqsig_type, > + which calls the appropriate underlying function. */ > +template<typename _T1, typename _T2> > +inline int > +iseqsig(_T1 __x, _T2 __y) throw() > +{ > +# if __cplusplus >= 201103L > + typedef __decltype (__MATH_EVAL_FMT2 (__x, __y)) _T3; > +# else > + typedef __typeof (__MATH_EVAL_FMT2 (__x, __y)) _T3; > +# endif > + return __iseqsig_type<_T3>::__call(__x, __y); > +} > + > +} /* extern "C++" */ > +# endif /* __cplusplus */ Would these two expressions have the same types, assuming _T1 and _T2 are the template parameters from the iseqsig definition? __MATH_EVAL_FMT2 (__x, __y) __MATH_EVAL_FMT2 (_T1 (), _T2 ()) I believe the second expression would be safe to evaluate, so it could be used to select a suitable inline function. This would then work with any C++ version. But the current approach is okay as well. > diff --git a/math/test-math-iseqsig.cc b/math/test-math-iseqsig.cc > new file mode 100644 > index 0000000000..0316340638 > --- /dev/null > +++ b/math/test-math-iseqsig.cc > +static bool errors; Maybe use support_record_failure from <support/check.h> instead? > +static void > +check (int actual, int expected, const char *actual_expr, int line) > +{ > + if (actual != expected) > + { > + errors = true; > + printf ("%s:%d: error: %s\n", __FILE__, line, actual_expr); > + printf ("%s:%d: expected: %d\n", __FILE__, line, expected); > + printf ("%s:%d: actual: %d\n", __FILE__, line, actual); > + } > +} > + > +#define CHECK(actual, expected) \ > + check ((actual), (expected), #actual, __LINE__) I think my memory protection key patches contain a more general implementation of this called TEST_COMPARE. Adhemerval requested just a minor documentation change, so I could commit that separately if you want to use it. Thanks, Florian
On Tue, 14 Nov 2017, Florian Weimer wrote: > Would these two expressions have the same types, assuming _T1 and _T2 are the > template parameters from the iseqsig definition? > > __MATH_EVAL_FMT2 (__x, __y) > __MATH_EVAL_FMT2 (_T1 (), _T2 ()) Yes, they should always have the same type.
On Tue, 14 Nov 2017, Florian Weimer wrote: >On 11/14/2017 02:21 PM, Gabriel F. T. Gomes wrote: > >> +template<typename _T1, typename _T2> >> +inline int >> +iseqsig(_T1 __x, _T2 __y) throw() >> +{ >> +# if __cplusplus >= 201103L >> + typedef __decltype (__MATH_EVAL_FMT2 (__x, __y)) _T3; >> +# else >> + typedef __typeof (__MATH_EVAL_FMT2 (__x, __y)) _T3; >> +# endif >> + return __iseqsig_type<_T3>::__call(__x, __y); >> +} > >Would these two expressions have the same types, assuming _T1 and _T2 >are the template parameters from the iseqsig definition? > > __MATH_EVAL_FMT2 (__x, __y) > __MATH_EVAL_FMT2 (_T1 (), _T2 ()) > >I believe the second expression would be safe to evaluate, so it could >be used to select a suitable inline function. This would then work with >any C++ version. Do you mean that I should go back to the first version of the patch (that worked with function overloading and had a third parameter)? For instance: template <typename _T1, typename _T2> inline int __iseqsig_type (float, _T1 x, _T2 y) { return __iseqsigf (x, y); } /* ... Likewise for other types. */ template <typename _T1, typename _T2> inline int iseqsig (_T1 __x, _T2 __y) { _T1 __w = _T1 (); _T2 __z = _T2 (); return __iseqsig_type (__MATH_EVAL_FMT2 (__w, __z), __x, __y); } I am asking it, because on the second and third versions of this patch, the selection of the underlying function is through a template parameter, so I think we need to use typeof or decltype (at least I think that the template parameter requires it). >Maybe use support_record_failure from <support/check.h> instead? I already adapted locally to use this, but I'll wait for your answer to my question above, before sending a new version. >> +static void >> +check (int actual, int expected, const char *actual_expr, int line) >> +{ >> + if (actual != expected) >> + { >> + errors = true; >> + printf ("%s:%d: error: %s\n", __FILE__, line, actual_expr); >> + printf ("%s:%d: expected: %d\n", __FILE__, line, expected); >> + printf ("%s:%d: actual: %d\n", __FILE__, line, actual); >> + } >> +} >> + >> +#define CHECK(actual, expected) \ >> + check ((actual), (expected), #actual, __LINE__) > >I think my memory protection key patches contain a more general >implementation of this called TEST_COMPARE. Adhemerval requested just a >minor documentation change, so I could commit that separately if you >want to use it. I can wait for your changes to go in in their own timing, then I'd gladly write a clean up to use them (here and in other similar tests).
On 11/17/2017 06:25 PM, Gabriel F. T. Gomes wrote: >> Would these two expressions have the same types, assuming _T1 and _T2 >> are the template parameters from the iseqsig definition? >> >> __MATH_EVAL_FMT2 (__x, __y) >> __MATH_EVAL_FMT2 (_T1 (), _T2 ()) >> >> I believe the second expression would be safe to evaluate, so it could >> be used to select a suitable inline function. This would then work with >> any C++ version. > Do you mean that I should go back to the first version of the patch (that > worked with function overloading and had a third parameter)? I do not have a strong preference here. I'm fine with the use of decltype/__typeof as well. I just wanted to point out this other solution in case we need it at some point. Thanks, Florian
diff --git a/math/Makefile b/math/Makefile index b2bd3d3bcc..de4106b0ff 100644 --- a/math/Makefile +++ b/math/Makefile @@ -215,7 +215,7 @@ tests-static += atest-exp atest-sincos atest-exp2 ifneq (,$(CXX)) tests += test-math-isinff test-math-iszero test-math-issignaling \ - test-math-iscanonical test-math-cxx11 + test-math-iscanonical test-math-cxx11 test-math-iseqsig endif ifneq (no,$(PERL)) diff --git a/math/math.h b/math/math.h index 326fd8ebe1..acc909d37f 100644 --- a/math/math.h +++ b/math/math.h @@ -1152,8 +1152,76 @@ iszero (__T __val) /* Return X == Y but raising "invalid" and setting errno if X or Y is a NaN. */ -# define iseqsig(x, y) \ - __MATH_TG (__MATH_EVAL_FMT2 (x, y), __iseqsig, ((x), (y))) +# if !defined __cplusplus || (__cplusplus < 201103L && !defined __GNUC__) +# define iseqsig(x, y) \ + __MATH_TG (__MATH_EVAL_FMT2 (x, y), __iseqsig, ((x), (y))) +# else +/* In C++ mode, __MATH_TG cannot be used, because it relies on + __builtin_types_compatible_p, which is a C-only builtin. Moreover, + the comparison macros from ISO C take two floating-point arguments, + which need not have the same type. Choosing what underlying function + to call requires evaluating the formats of the arguments, then + selecting which is wider. The macro __MATH_EVAL_FMT2 provides this + information, however, only the type of the macro expansion is + relevant (actually evaluating the expression would be incorrect). + Thus, the type is used as a template parameter for __iseqsig_type, + which calls the appropriate underlying function. */ +extern "C++" { +template<typename> struct __iseqsig_type; + +template<> struct __iseqsig_type<float> +{ + static int __call(float __x, float __y) throw() + { + return __iseqsigf (__x, __y); + } +}; + +template<> struct __iseqsig_type<double> +{ + static int __call(double __x, double __y) throw() + { + return __iseqsig (__x, __y); + } +}; + +template<> struct __iseqsig_type<long double> +{ + static int __call(double __x, double __y) throw() + { +# ifndef __NO_LONG_DOUBLE_MATH + return __iseqsigl (__x, __y); +# else + return __iseqsig (__x, __y); +# endif + } +}; + +# if __HAVE_DISTINCT_FLOAT128 +template<> struct __iseqsig_type<_Float128> +{ + static int __call(_Float128 __x, _Float128 __y) throw() + { + return __iseqsigf128 (__x, __y); + } +}; +# endif + +template<typename _T1, typename _T2> +inline int +iseqsig(_T1 __x, _T2 __y) throw() +{ +# if __cplusplus >= 201103L + typedef __decltype (__MATH_EVAL_FMT2 (__x, __y)) _T3; +# else + typedef __typeof (__MATH_EVAL_FMT2 (__x, __y)) _T3; +# endif + return __iseqsig_type<_T3>::__call(__x, __y); +} + +} /* extern "C++" */ +# endif /* __cplusplus */ + #endif __END_DECLS diff --git a/math/test-math-iseqsig.cc b/math/test-math-iseqsig.cc new file mode 100644 index 0000000000..0316340638 --- /dev/null +++ b/math/test-math-iseqsig.cc @@ -0,0 +1,111 @@ +/* Test for the C++ implementation of iseqsig. + Copyright (C) 2017 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C 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 + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <http://www.gnu.org/licenses/>. */ + +#define _GNU_SOURCE 1 +#include <math.h> +#include <stdio.h> + +#include <limits> + +/* There is no NaN for _Float128 in std::numeric_limits. + Include ieee754_float128.h and use the bitfields in the union + ieee854_float128.ieee_nan to build a NaN. */ +#if __HAVE_DISTINCT_FLOAT128 +# include <ieee754_float128.h> +#endif + +static bool errors; + +static void +check (int actual, int expected, const char *actual_expr, int line) +{ + if (actual != expected) + { + errors = true; + printf ("%s:%d: error: %s\n", __FILE__, line, actual_expr); + printf ("%s:%d: expected: %d\n", __FILE__, line, expected); + printf ("%s:%d: actual: %d\n", __FILE__, line, actual); + } +} + +#define CHECK(actual, expected) \ + check ((actual), (expected), #actual, __LINE__) + +template <class T1, class T2> +static void +check_type () +{ + T1 t1 = 0; + T2 t2 = 0; + CHECK (iseqsig (t1, t2), 1); + + t2 = 1; + CHECK (iseqsig (t1, t2), 0); + + if (std::numeric_limits<T1>::has_quiet_NaN + && std::numeric_limits<T2>::has_quiet_NaN) + { + CHECK (iseqsig (std::numeric_limits<T1>::quiet_NaN (), t2), 0); + CHECK (iseqsig (t1, std::numeric_limits<T2>::quiet_NaN ()), 0); + CHECK (iseqsig (std::numeric_limits<T1>::quiet_NaN (), + std::numeric_limits<T2>::quiet_NaN ()), 0); + } +} + +#if __HAVE_DISTINCT_FLOAT128 +static void +check_float128 () +{ + ieee854_float128 q1, q2, q3_nan; + + q1.d = 0; + q2.d = 1; + q3_nan.ieee_nan.negative = 0; + q3_nan.ieee_nan.exponent = 0x7FFF; + q3_nan.ieee_nan.quiet_nan = 1; + q3_nan.ieee_nan.mantissa0 = 0x0000; + q3_nan.ieee_nan.mantissa1 = 0x00000000; + q3_nan.ieee_nan.mantissa2 = 0x00000000; + q3_nan.ieee_nan.mantissa3 = 0x00000000; + + CHECK (iseqsig (q1.d, q1.d), 1); + CHECK (iseqsig (q1.d, q2.d), 0); + CHECK (iseqsig (q1.d, q3_nan.d), 0); + CHECK (iseqsig (q3_nan.d, q3_nan.d), 0); +} +#endif + +static int +do_test (void) +{ + check_type<float, float> (); + check_type<float, double> (); + check_type<float, long double> (); + check_type<double, float> (); + check_type<double, double> (); + check_type<double, long double> (); + check_type<long double, float> (); + check_type<long double, double> (); + check_type<long double, long double> (); +#if __HAVE_DISTINCT_FLOAT128 + check_float128 (); +#endif + return errors; +} + +#include <support/test-driver.c> diff --git a/sysdeps/powerpc/powerpc64le/Makefile b/sysdeps/powerpc/powerpc64le/Makefile index f554a791b7..9b8bd138cd 100644 --- a/sysdeps/powerpc/powerpc64le/Makefile +++ b/sysdeps/powerpc/powerpc64le/Makefile @@ -17,9 +17,13 @@ $(foreach suf,$(all-object-suffixes),$(objpfx)test-float128%$(suf)): CFLAGS += - $(foreach suf,$(all-object-suffixes),$(objpfx)test-ifloat128%$(suf)): CFLAGS += -mfloat128 CFLAGS-libm-test-support-float128.c += -mfloat128 CFLAGS-test-math-iscanonical.cc += -mfloat128 +CFLAGS-test-math-iseqsig.cc += -mfloat128 CFLAGS-test-math-issignaling.cc += -mfloat128 CFLAGS-test-math-iszero.cc += -mfloat128 -$(objpfx)test-float128% $(objpfx)test-ifloat128% $(objpfx)test-math-iszero: \ +$(foreach test, \ + test-float128% test-ifloat128% test-math-iscanonical \ + test-math-iseqsig test-math-issignaling test-math-iszero, \ + $(objpfx)$(test)): \ gnulib-tests += $(f128-loader-link) endif