benchtests: Run _Float128 tests only on architectures that support it

Message ID 20200917033952.GA77991@aloka.lostca.se
State New
Headers show
Series
  • benchtests: Run _Float128 tests only on architectures that support it
Related show

Commit Message

Arjun Shankar Sept. 17, 2020, 3:39 a.m.
From: Arjun Shankar <arjun@redhat.com>


__float128 is a non-standard name and is not available on some architectures
(like aarch64 or s390x) even though they may support the standard _Float128
type.  Other architectures (like armv7) don't support quad-precision
floating-point operations at all.

This commit adds a configure check for quad-precision FP support and runs
the corresponding benchtests only on architectures that support it.  It also
replaces references to __float128 with _Float128.
---
 Tested on armv7hl, i686, x86_64, aarch64, ppc64le, and s390x.

 benchtests/Makefile       |  6 +++++-
 benchtests/expf128-inputs |  4 ++--
 benchtests/powf128-inputs |  4 ++--
 benchtests/sinf128-inputs |  4 ++--
 configure                 | 24 ++++++++++++++++++++++++
 configure.ac              | 15 +++++++++++++++
 6 files changed, 50 insertions(+), 7 deletions(-)

-- 
2.26.2

Comments

Paul Zimmermann Sept. 17, 2020, 6:47 a.m. | #1
Dear Arjun,

some comments on your patch:

* the changes to benchtests/Makefile and benchtests/*f128-inputs look ok to me
* for configure.ac I'm not fluent enough in the autotools
* I wonder why you also patch configure, isn't this file automatically
  generated?
* finally "find . -type f -exec grep \-l __float128 {} \;" finds several other
  instances of __float128 (sysdeps/x86_64/fpu/math-tests-snan.h for example).
  Should some of those be changed to _Float128 too?

Best regards,
Paul
Florian Weimer via Libc-alpha Sept. 17, 2020, 7:21 a.m. | #2
* Paul Zimmermann:

> * I wonder why you also patch configure, isn't this file automatically

>   generated?


It's checked into the repository, so it shows up in the diff.

> * finally "find . -type f -exec grep \-l __float128 {} \;" finds several other

>   instances of __float128 (sysdeps/x86_64/fpu/math-tests-snan.h for example).

>   Should some of those be changed to _Float128 too?


I think it's mostly comments or architecture-specific code.  For x86, we
know that __float128 exists, so that's not actually a problem.

Thanks,
Florian
-- 
Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill
Florian Weimer via Libc-alpha Sept. 17, 2020, 8:51 a.m. | #3
* Arjun Shankar:

> diff --git a/configure.ac b/configure.ac

> index 93e68fb696..7ee79162be 100644

> --- a/configure.ac

> +++ b/configure.ac

> @@ -1821,6 +1821,21 @@ libc_cv_pie_default=$libc_cv_cc_pie_default

>  AC_SUBST(libc_cv_cc_pie_default)

>  AC_SUBST(libc_cv_pie_default)

>  

> +AC_CACHE_CHECK([whether quad-precision floating-point is supported],

> +libc_cv_have_qp_float, [libc_cv_have_qp_float=yes

> +cat > conftest.c <<EOF

> +#include <math.h>

> +#if __HAVE_FLOAT128

> +# error quad-precision floating-point is supported

> +#endif

> +EOF

> +if eval "${CC-cc} -S conftest.c 2>&AS_MESSAGE_LOG_FD 1>&AS_MESSAGE_LOG_FD"; then

> +  libc_cv_have_qp_float=no

> +fi

> +rm -f conftest.*])

> +AC_SUBST(libc_cv_have_qp_float)

> +LIBC_CONFIG_VAR([have-qp-float], [$libc_cv_have_qp_float])


This configure test does not run against <math.h> from the glibc tree,
so its result is not correct.

Please try adding the benchtests via a fragment in
sysdeps/ieee754/float128/Makefile.  I think this won't need additional
configure checks.

Thanks,
Florian
-- 
Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill
Joseph Myers Sept. 17, 2020, 4:21 p.m. | #4
On Thu, 17 Sep 2020, Florian Weimer via Libc-alpha wrote:

> Please try adding the benchtests via a fragment in

> sysdeps/ieee754/float128/Makefile.  I think this won't need additional

> configure checks.


If you want these benchmarks only if _Float128 has a different format from 
long double, you can use the existing makefile variable $(float128-fcts).  
If you want to run them also when _Float128 has the same format as long 
double, use $(float128-alias-fcts) as well.  See math/Makefile for example 
usage.

-- 
Joseph S. Myers
joseph@codesourcery.com
Florian Weimer via Libc-alpha Sept. 17, 2020, 5:12 p.m. | #5
* Joseph Myers:

> On Thu, 17 Sep 2020, Florian Weimer via Libc-alpha wrote:

>

>> Please try adding the benchtests via a fragment in

>> sysdeps/ieee754/float128/Makefile.  I think this won't need additional

>> configure checks.

>

> If you want these benchmarks only if _Float128 has a different format from 

> long double, you can use the existing makefile variable $(float128-fcts).  

> If you want to run them also when _Float128 has the same format as long 

> double, use $(float128-alias-fcts) as well.  See math/Makefile for example 

> usage.


In this case, the ifeq Makefile conditional should be added to
benchtests/Makefile, right?

Thanks,
Florian
-- 
Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill
Joseph Myers Sept. 17, 2020, 8:48 p.m. | #6
On Thu, 17 Sep 2020, Florian Weimer via Libc-alpha wrote:

> * Joseph Myers:

> 

> > On Thu, 17 Sep 2020, Florian Weimer via Libc-alpha wrote:

> >

> >> Please try adding the benchtests via a fragment in

> >> sysdeps/ieee754/float128/Makefile.  I think this won't need additional

> >> configure checks.

> >

> > If you want these benchmarks only if _Float128 has a different format from 

> > long double, you can use the existing makefile variable $(float128-fcts).  

> > If you want to run them also when _Float128 has the same format as long 

> > double, use $(float128-alias-fcts) as well.  See math/Makefile for example 

> > usage.

> 

> In this case, the ifeq Makefile conditional should be added to

> benchtests/Makefile, right?


ifeq, or any of the other mechanisms used such as $(var-$(float128-fcts)).

-- 
Joseph S. Myers
joseph@codesourcery.com

Patch

diff --git a/benchtests/Makefile b/benchtests/Makefile
index 3095076055..6590a95439 100644
--- a/benchtests/Makefile
+++ b/benchtests/Makefile
@@ -25,7 +25,11 @@  bench-math := acos acosh asin asinh atan atanh cos cosh exp exp2 log log2 \
 	      modf pow rint sin sincos sinh sqrt tan tanh fmin fmax fminf \
 	      fmaxf powf trunc truncf roundeven roundevenf expf exp2f logf \
 	      log2f sincosf sinf cosf isnan isinf isfinite hypot logb logbf \
-	      exp10f expf128 powf128 sinf128
+	      exp10f
+
+ifeq (yes,$(have-qp-float))
+bench-math += expf128 powf128 sinf128
+endif
 
 bench-pthread := pthread_once thread_create
 
diff --git a/benchtests/expf128-inputs b/benchtests/expf128-inputs
index 5b36f8672a..5dc0f4e49b 100644
--- a/benchtests/expf128-inputs
+++ b/benchtests/expf128-inputs
@@ -1,6 +1,6 @@ 
 ## includes: math.h
-## args: __float128
-## ret: __float128
+## args: _Float128
+## ret: _Float128
 # Random inputs in [-10,10]
 ## name: workload-random.wrf
 0x4.e6f9d6da10d9a422942a89cdfa1p+0L
diff --git a/benchtests/powf128-inputs b/benchtests/powf128-inputs
index 7cbabafff6..6826266ad5 100644
--- a/benchtests/powf128-inputs
+++ b/benchtests/powf128-inputs
@@ -1,6 +1,6 @@ 
 ## includes: math.h
-## args: __float128:__float128
-## ret: __float128
+## args: _Float128:_Float128
+## ret: _Float128
 # Random inputs in [-10,10] such that x and y are not both negative
 ## name: workload-random.wrf
 0x8.130b31ed5288656428a29cead83p+0L, -0x6.e7ead09b7877a118813b50cfb3c8p+0L
diff --git a/benchtests/sinf128-inputs b/benchtests/sinf128-inputs
index 9aaf312413..01b7533680 100644
--- a/benchtests/sinf128-inputs
+++ b/benchtests/sinf128-inputs
@@ -1,6 +1,6 @@ 
 ## includes: math.h
-## args: __float128
-## ret: __float128
+## args: _Float128
+## ret: _Float128
 # Random inputs in [-10,10]
 ## name: workload-random.wrf
 0x4.e6f9d6da10d9a422942a89cdfa1p+0
diff --git a/configure b/configure
index 4795e721e5..a2a8636ca6 100755
--- a/configure
+++ b/configure
@@ -595,6 +595,7 @@  DEFINES
 static_nss
 profile
 libc_cv_multidir
+libc_cv_have_qp_float
 libc_cv_pie_default
 libc_cv_cc_pie_default
 libc_cv_pic_default
@@ -6805,6 +6806,29 @@  libc_cv_pie_default=$libc_cv_cc_pie_default
 
 
 
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking whether quad-precision floating-point is supported" >&5
+$as_echo_n "checking whether quad-precision floating-point is supported... " >&6; }
+if ${libc_cv_have_qp_float+:} false; then :
+  $as_echo_n "(cached) " >&6
+else
+  libc_cv_have_qp_float=yes
+cat > conftest.c <<EOF
+#include <math.h>
+#if __HAVE_FLOAT128
+# error quad-precision floating-point is supported
+#endif
+EOF
+if eval "${CC-cc} -S conftest.c 2>&5 1>&5"; then
+  libc_cv_have_qp_float=no
+fi
+rm -f conftest.*
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $libc_cv_have_qp_float" >&5
+$as_echo "$libc_cv_have_qp_float" >&6; }
+
+config_vars="$config_vars
+have-qp-float = $libc_cv_have_qp_float"
+
 # Set the `multidir' variable by grabbing the variable from the compiler.
 # We do it once and save the result in a generated makefile.
 libc_cv_multidir=`${CC-cc} $CFLAGS $CPPFLAGS -print-multi-directory`
diff --git a/configure.ac b/configure.ac
index 93e68fb696..7ee79162be 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1821,6 +1821,21 @@  libc_cv_pie_default=$libc_cv_cc_pie_default
 AC_SUBST(libc_cv_cc_pie_default)
 AC_SUBST(libc_cv_pie_default)
 
+AC_CACHE_CHECK([whether quad-precision floating-point is supported],
+libc_cv_have_qp_float, [libc_cv_have_qp_float=yes
+cat > conftest.c <<EOF
+#include <math.h>
+#if __HAVE_FLOAT128
+# error quad-precision floating-point is supported
+#endif
+EOF
+if eval "${CC-cc} -S conftest.c 2>&AS_MESSAGE_LOG_FD 1>&AS_MESSAGE_LOG_FD"; then
+  libc_cv_have_qp_float=no
+fi
+rm -f conftest.*])
+AC_SUBST(libc_cv_have_qp_float)
+LIBC_CONFIG_VAR([have-qp-float], [$libc_cv_have_qp_float])
+
 # Set the `multidir' variable by grabbing the variable from the compiler.
 # We do it once and save the result in a generated makefile.
 libc_cv_multidir=`${CC-cc} $CFLAGS $CPPFLAGS -print-multi-directory`