[v3] Provide a C++ version of iseqsig
Commit Message
Changes since v2:
- For __cplusplus >= 201103L use __decltype;
- Otherwise, use __typeof only if compiler defines __GNUC__;
- Use __MATH_TG for older C++ and not GNU compiler (will fail if
support for float128 is on).
Changes since v1:
- Fixed namespace violations in function arguments;
- Removed additional argument in __iseqsig_type;
- Templates for __iseqsig_type now have a single parameter, to avoid
having two different instantiations for different ordering of the
same pair of types (e.g.: (float, double) and (double, float));
- iseqsig is now a real-function, instead of a macro.
- __MATH_EVAL_FMT2 is used whithin typeof and does not evaluate in
runtime.
- Added testcase.
-- 8< --
In C++ mode, __MATH_TG cannot be used for defining iseqsig, because
__MATH_TG relies on __builtin_types_compatible_p, which is a C-only
builtin. This is true when float128 is provided as an ABI-distinct type
from long double.
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).
This patch provides a C++ version of iseqsig, in which only the type of
__MATH_EVAL_FMT2 (__typeof or __decltype) is used as a template
parameter for __iseqsig_type. This function calls the appropriate
underlying function.
Tested for powerpc64le and x86_64.
[BZ #22377]
* math/Makefile [C++] (tests): Add test for iseqsig.
* math/math.h [C++] (iseqsig): New implementation, which does
not rely on __MATH_TG/__builtin_types_compatible_p.
* math/test-math-iseqsig.cc: New file.
* sysdeps/powerpc/powerpc64le/Makefile
(CFLAGS-test-math-iseqsig.cc): New variable.
---
math/Makefile | 2 +-
math/math.h | 72 ++++++++++++++++++++++-
math/test-math-iseqsig.cc | 111 +++++++++++++++++++++++++++++++++++
sysdeps/powerpc/powerpc64le/Makefile | 6 +-
4 files changed, 187 insertions(+), 4 deletions(-)
create mode 100644 math/test-math-iseqsig.cc
Comments
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
@@ -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))
@@ -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
new file mode 100644
@@ -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>
@@ -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