Patchwork [v4] Provide a C++ version of iseqsig

login
register
mail settings
Submitter Gabriel F. T. Gomes
Date Nov. 21, 2017, 1:35 p.m.
Message ID <20171121133529.30074-1-gabriel@inconstante.eti.br>
Download mbox | patch
Permalink /patch/24400/
State New
Headers show

Comments

Gabriel F. T. Gomes - Nov. 21, 2017, 1:35 p.m.
Changes since v3:

  - Use support_record_failure from <support/check.h>;
  - Use decltype instead of __decltype (not a namespace violation).

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
Gabriel F. T. Gomes - Nov. 22, 2017, 7:20 p.m.
On Tue, 21 Nov 2017, Gabriel F. T. Gomes wrote:

>+  static int __call(float __x, float __y) throw()
                     ^                           ^
I have locally addressed the comments about missing spaces between
function name and opening parenthesis (as noted in [1]), here and in other
parts of this patch.  Sorry for not addressing this earlier and sending
versions 3 and 4 without these changes.

[1] https://sourceware.org/ml/libc-alpha/2017-11/msg00276.html


If the rest of the patch reaches consensus, I'll commit it with these
changes.

Thanks,
Gabriel
Joseph Myers - Dec. 6, 2017, 10:31 p.m.
On Wed, 22 Nov 2017, Gabriel F. T. Gomes wrote:

> On Tue, 21 Nov 2017, Gabriel F. T. Gomes wrote:
> 
> >+  static int __call(float __x, float __y) throw()
>                      ^                           ^
> I have locally addressed the comments about missing spaces between
> function name and opening parenthesis (as noted in [1]), here and in other
> parts of this patch.  Sorry for not addressing this earlier and sending
> versions 3 and 4 without these changes.
> 
> [1] https://sourceware.org/ml/libc-alpha/2017-11/msg00276.html
> 
> 
> If the rest of the patch reaches consensus, I'll commit it with these
> changes.

The patch is OK with those fixes.

Patch

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 f6d1feed78..1280f0e84e 100644
--- a/math/math.h
+++ b/math/math.h
@@ -1182,8 +1182,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..48e8f1e799
--- /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
+
+#include <support/check.h>
+
+static void
+check (int actual, int expected, const char *actual_expr, int line)
+{
+  if (actual != expected)
+    {
+      support_record_failure ();
+      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 0;
+}
+
+#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