[PATCHv2,3/7] ldbl-128ibm-compat: Add tests for IBM long double functions

Message ID 20200217220609.26623-1-murphyp@linux.vnet.ibm.com
State Superseded
Delegated to: Joseph Myers
Headers

Commit Message

Paul E. Murphy Feb. 17, 2020, 10:06 p.m. UTC
  From: Rajalakshmi Srinivasaraghavan <raji@linux.vnet.ibm.com>

My brain misfired last friday.  Such things are not always unheard of on
such weekdays.  The reason for the awkward macro usage is to work around
old compiler quirks.  ppc64le will need to bump the minimum compiler
version to 7.3 from 6.2 to support building with the new ldouble format.

The reason for the awkward __LONG_DOUBLE_USES_FLOAT128 test is that GCC 7
compilers can (and do via later patchsets) get invoked with
"-mabi=ieeelongdouble" and without "-mfloat128".  GCC 7 treats this
as not having _Float128 and thereby __HAVE_DISTINCT_FLOAT128 == 0.

I have added a comment noting the above. Likewise, the copyright year is
updated in test-ibm.h.  Likewise, fix up the co-authorship comments.

---8<---

This patch creates test-ibm128* tests from the long double function tests.
In order to explicitly test IBM long double functions -mabi=ibmlongdouble is
added to CFLAGS.

Likewise, update the test headers to correct choose ULPs when redirects
are enabled.

Co-authored-by: Tulio Magno Quites Machado Filho  <tuliom@linux.ibm.com>
Co-authored-by: Paul E. Murphy  <murphyp@linux.vnet.ibm.com>

---
 math/Makefile                                 |  7 ++-
 math/test-float128.h                          |  6 ++-
 math/test-float64x.h                          |  6 ++-
 math/test-ibm128.h                            | 19 +++++++
 math/test-ldouble.h                           |  6 +++
 sysdeps/ieee754/ldbl-128ibm-compat/Makeconfig |  3 ++
 sysdeps/powerpc/powerpc64/le/Makefile         | 50 ++++++++++++++++---
 7 files changed, 88 insertions(+), 9 deletions(-)
 create mode 100644 math/test-ibm128.h
 create mode 100644 sysdeps/ieee754/ldbl-128ibm-compat/Makeconfig
  

Comments

Joseph Myers Feb. 17, 2020, 10:24 p.m. UTC | #1
On Mon, 17 Feb 2020, Paul E. Murphy wrote:

> The reason for the awkward __LONG_DOUBLE_USES_FLOAT128 test is that GCC 7
> compilers can (and do via later patchsets) get invoked with
> "-mabi=ieeelongdouble" and without "-mfloat128".  GCC 7 treats this
> as not having _Float128 and thereby __HAVE_DISTINCT_FLOAT128 == 0.

Why are you building without -mfloat128?

Normally, code built with GCC 7 can expect that, if long double has the 
binary128 format, _Float128 is also available.  Having powerpc64le (only) 
break that invariant seems a bad idea.  I think you should make sure that 
__HAVE_FLOAT128 and __HAVE_DISTINCT_FLOAT128 both are defined to 1 (that 
you compile with options causing them to be defined to 1) when building 
any parts of glibc that use binary128, rather than working around them 
being defined to 0.

> -#if FLT128_MANT_DIG == LDBL_MANT_DIG
> +/* __LONG_DOUBLE_USES_FLOAT128 is used instead of __HAVE_DISTINCT_FLOAT128
> +   because GCC 7 compilers can support IEEE 128 long double without
> +   outwardly supporting _Float128.  When minimum GCC is raised to 8, this
> +   check can be replaced.  */

Any place where something can be simplified when the minimum GCC version 
is increased should have a conditional using __GNUC_PREREQ, not just a 
free-form comment; that's the only way we can effectively find all such 
places when increasing the minimum GCC version.

> +/* _Float128 unconditionally redirects to lgamma.  Ensure the ULPs do too.  */
> +# define gamma lgamma

Rather than a hack like this, I think you should do one of the following:

* Just all the float128 ulps for gamma to the relevant libm-test-ulps 
file.

* Properly implement ulps sharing in the libm-test machinery for all cases 
where tests of more than one function use the same table of tests, and 
update all libm-test-ulps files accordingly.
  

Patch

diff --git a/math/Makefile b/math/Makefile
index 1d203e7ad5..84a8b94c74 100644
--- a/math/Makefile
+++ b/math/Makefile
@@ -145,10 +145,15 @@  type-float128-yes := float128
 # _Float64x may be supported, only as an alias type.
 type-float64x-yes := float64x
 
+# IBM long double support in additional to IEEE 128 long double support
+type-ibm128-suffix := l
+type-ibm128-yes := ibm128
+
 types = $(types-basic) $(type-float128-$(float128-fcts))
 test-types = $(test-types-basic) $(type-float128-$(float128-fcts)) \
 	     float32 float64 $(type-float128-$(float128-alias-fcts)) \
-	     float32x $(type-float64x-$(float64x-alias-fcts))
+	     float32x $(type-float64x-$(float64x-alias-fcts)) \
+	     $(type-ibm128-$(ibm128-fcts))
 
 # Pairs of types for which narrowing functions should be tested (this
 # variable has more entries than libm-narrow-types because it includes
diff --git a/math/test-float128.h b/math/test-float128.h
index 8f9eec14aa..4cf18fe8d1 100644
--- a/math/test-float128.h
+++ b/math/test-float128.h
@@ -28,7 +28,11 @@ 
 #define CFLOAT __CFLOAT128
 #define BUILD_COMPLEX(real, imag) (CMPLXF128 ((real), (imag)))
 #define PREFIX FLT128
-#if FLT128_MANT_DIG == LDBL_MANT_DIG
+/* __LONG_DOUBLE_USES_FLOAT128 is used instead of __HAVE_DISTINCT_FLOAT128
+   because GCC 7 compilers can support IEEE 128 long double without
+   outwardly supporting _Float128.  When minimum GCC is raised to 8, this
+   check can be replaced.  */
+#if __LONG_DOUBLE_USES_FLOAT128 == 0 && FLT128_MANT_DIG == LDBL_MANT_DIG
 # define TYPE_STR "ldouble"
 # define ULP_IDX ULP_LDBL
 # define ULP_I_IDX ULP_I_LDBL
diff --git a/math/test-float64x.h b/math/test-float64x.h
index 9543238209..1e9916c2d8 100644
--- a/math/test-float64x.h
+++ b/math/test-float64x.h
@@ -28,7 +28,11 @@ 
 #define CFLOAT __CFLOAT64X
 #define BUILD_COMPLEX(real, imag) (CMPLXF64X ((real), (imag)))
 #define PREFIX FLT64X
-#if FLT64X_MANT_DIG == LDBL_MANT_DIG
+/* __LONG_DOUBLE_USES_FLOAT128 is used instead of __HAVE_DISTINCT_FLOAT128
+   because GCC 7 compilers can support IEEE 128 long double without
+   outwardly supporting _Float128.  When minimum GCC is raised to 8, this
+   check can be replaced.  */
+#if __LONG_DOUBLE_USES_FLOAT128 == 0 && FLT64X_MANT_DIG == LDBL_MANT_DIG
 # define TYPE_STR "ldouble"
 # define ULP_IDX ULP_LDBL
 # define ULP_I_IDX ULP_I_LDBL
diff --git a/math/test-ibm128.h b/math/test-ibm128.h
new file mode 100644
index 0000000000..fce6ef1376
--- /dev/null
+++ b/math/test-ibm128.h
@@ -0,0 +1,19 @@ 
+/* Common definitions for libm tests for ibm long double.
+   Copyright (C) 2020 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/>.  */
+
+#include "test-ldouble.h"
diff --git a/math/test-ldouble.h b/math/test-ldouble.h
index 17c526bd71..1dc1c86c07 100644
--- a/math/test-ldouble.h
+++ b/math/test-ldouble.h
@@ -27,6 +27,12 @@ 
 # define TYPE_STR "double"
 # define ULP_IDX ULP_DBL
 # define ULP_I_IDX ULP_I_DBL
+#elif __LONG_DOUBLE_USES_FLOAT128 == 1
+# define TYPE_STR "float128"
+# define ULP_IDX ULP_FLT128
+# define ULP_I_IDX ULP_I_FLT128
+/* _Float128 unconditionally redirects to lgamma.  Ensure the ULPs do too.  */
+# define gamma lgamma
 #else
 # define TYPE_STR "ldouble"
 # define ULP_IDX ULP_LDBL
diff --git a/sysdeps/ieee754/ldbl-128ibm-compat/Makeconfig b/sysdeps/ieee754/ldbl-128ibm-compat/Makeconfig
new file mode 100644
index 0000000000..997f632319
--- /dev/null
+++ b/sysdeps/ieee754/ldbl-128ibm-compat/Makeconfig
@@ -0,0 +1,3 @@ 
+# Include this earlier so it can be used earlier in Makefiles,
+# and sysdep/ makefiles.
+ibm128-fcts = yes
diff --git a/sysdeps/powerpc/powerpc64/le/Makefile b/sysdeps/powerpc/powerpc64/le/Makefile
index 0ce3868c3c..1594f6b179 100644
--- a/sysdeps/powerpc/powerpc64/le/Makefile
+++ b/sysdeps/powerpc/powerpc64/le/Makefile
@@ -6,6 +6,13 @@ 
 # linked executables, forcing to link the loader after libgcc link.
 f128-loader-link = -Wl,--as-needed $(elf-objpfx)ld.so -Wl,--no-as-needed
 
+# Bootstrapping code for enabling IEEE 128.  This can be removed and
+# any indirections simplified once IEEE 128 long double is enabled.
+type-ldouble-CFLAGS =
+ifeq ($(ibm128-fcts),yes)
+type-ldouble-CFLAGS += -mabi=ibmlongdouble
+endif
+
 ifeq ($(subdir),math)
 # sqrtf128 requires emulation before POWER9.
 CPPFLAGS += -I../soft-fp
@@ -28,13 +35,34 @@  CFLAGS-test-math-iscanonical.cc += -mfloat128
 CFLAGS-test-math-iseqsig.cc += -mfloat128
 CFLAGS-test-math-issignaling.cc += -mfloat128
 CFLAGS-test-math-iszero.cc += -mfloat128
-$(foreach test, \
-	  test-float128% test-ifloat128% test-float64x% test-ifloat64x% \
-	  $(foreach pair,$(f128-pairs),test-$(pair)%) \
-	  test-math-iscanonical test-math-iseqsig test-math-issignaling \
-	  test-math-iszero, \
-	  $(objpfx)$(test)): \
+$(foreach test,\
+	  basic-test \
+	  bug-nextafter \
+	  bug-nexttoward \
+	  test-fenv-clear \
+	  test-iszero-excess-precision \
+	  test-math-iscanonical \
+	  test-math-iseqsig \
+	  test-math-issignaling \
+	  test-math-iszero \
+	  test-misc \
+	  test-nan-overflow \
+	  test-nan-payload \
+	  test-snan \
+	  test-tgmath \
+	  test-tgmath2 \
+	  tst-CMPLX2 \
+	  test-%-ldbl-128ibm \
+	  test-ldouble% test-ildouble% \
+	  test-float128% test-ifloat128% \
+	  test-float64x% test-ifloat64x% \
+	  ,$(objpfx)$(test)): \
   gnulib-tests += $(f128-loader-link)
+
+$(foreach suf,$(all-object-suffixes),\
+         $(objpfx)libm-test-%ibm128$(suf) \
+         $(objpfx)test-iibm128%$(suf) $(objpfx)test-ibm128%$(suf)): \
+  CFLAGS += $(type-ldouble-CFLAGS)
 endif
 
 # Append flags to string <-> _Float128 routines.
@@ -82,3 +110,13 @@  CFLAGS-printf_fp.c = -mfloat128
 CFLAGS-printf_fphex.c = -mfloat128
 CFLAGS-printf_size.c = -mfloat128
 endif
+
+$(foreach suf,$(all-object-suffixes),nldbl-%$(suf)): \
+  CFLAGS += $(type-ldouble-CFLAGS)
+
+sysdep-CFLAGS += -mabi=ieeelongdouble -Wno-psabi
+$(foreach suf,$(all-object-suffixes),\
+         $(objpfx)libm-test-%ibm128$(suf) \
+         $(objpfx)test-iibm128%$(suf) $(objpfx)test-ibm128%$(suf)): \
+  sysdep-CFLAGS := $(filter-out -mabi=ieeelongdouble \
+    -Wno-psabi, $(sysdep-CFLAGS))