[PATCH][BZ #14412] Define __sincos_finite as a fast version of sincos

[PATCH][BZ #14412] Define __sincos_finite as a fast version of sincos

[Siddhesh Poyarekar]

Hi,

The assembly implementation of sincos on x86_64 was removed in favour
of the default implementation since the former was inaccurate for some
inputs.  This led to BZ #14412 being reported for a performance
regression in sincos.

This patch brings back the assembly implementation of sincos (with
some changes) to give a fast alternative to the default sincos
implementation.  This is defined as __sincos_finite and is used if the
implementing program is compiled with the -ffinite-math-only gcc flag.

I have tested that this works correctly for x86_64 and i686, where the
former uses the assembly implementation of __sincos_finite and the
latter uses the weak alias to sincos.  I have also verified that there
are no regressions in the testsuite on x86_64.

The patch also has changes to m68k and ia64 (in addition to the
abilist update) since they have custom implementations of sincos.  I
have not tested those, so I would request the maintainers to let me
know if the changes break anything on their architectures.

Siddhesh

	[BZ #14412]
	* math/Versions (GLIBC_2.18): Add __sincos_finite.
	* math/bits/math-finite.h: Add prototype for __sincos_finite.
	* sysdeps/unix/sysv/linux/i386/nptl/libm.abilist: Add
	__sincos_finite.
	* sysdeps/unix/sysv/linux/powerpc/powerpc32/fpu/nptl/libm.abilist:
	Likewise.
	* sysdeps/unix/sysv/linux/powerpc/powerpc64/nptl/libm.abilist: 
	Likewise.
	* sysdeps/unix/sysv/linux/s390/s390-32/nptl/libm.abilist:
	Likewise.
	* sysdeps/unix/sysv/linux/s390/s390-64/nptl/libm.abilist:
	Likewise.
	* sysdeps/unix/sysv/linux/sh/nptl/libm.abilist: Likewise.
	* sysdeps/unix/sysv/linux/sparc/sparc32/nptl/libm.abilist:
	Likewise.
	* sysdeps/unix/sysv/linux/sparc/sparc64/nptl/libm.abilist:
	Likewise.
	* sysdeps/unix/sysv/linux/x86_64/64/nptl/libm.abilist:
	Likewise.
	* sysdeps/unix/sysv/linux/x86_64/x32/nptl/libm.abilist:
	Likewise.
	* sysdeps/ieee754/dbl-64/s_sincos.c [!_FINITE_DEFINED]: Add
	weak alias __sincos_finite for __sincos.
	* sysdeps/x86_64/fpu/Makefile: New file.
	* sysdeps/x86_64/fpu/s_sincos_finite.S: New file.

ChangeLog.ia64:

	[BZ #14412]
	* sysdeps/ia64/fpu/libm_sincos.S: Include sysdep.h.  Add weak
	alias __sincos_finite.
        * sysdeps/unix/sysv/linux/ia64/nptl/libm.abilist: Add
        __sincos_finite.

ChangeLog.m68k:

	[BZ #14412]
	* sysdeps/m68k/m680x0/fpu/s_sincos.c: Add weak alias
	__sincos_finite.
        * sysdeps/unix/sysv/linux/m68k/coldfire/nptl/libm.abilist: Add
        __sincos_finite.
        * sysdeps/unix/sysv/linux/m68k/m680x0/nptl/libm.abilist: Add
        __sincos_finite.

ChangeLog.aarch64:

	[BZ #14412]
	* sysdeps/unix/sysv/linux/aarch64/nptl/libm.abilist: Add
	__sincos_finite.

ChangeLog.alpha:

	[BZ #14412]
	* sysdeps/unix/sysv/linux/alpha/nptl/libm.abilist: Add
	__sincos_finite.

ChangeLog.arm:

	[BZ #14412]
	* sysdeps/unix/sysv/linux/arm/nptl/libm.abilist: Add
	__sincos_finite.

ChangeLog.microblaze:

	[BZ #14412]
        * sysdeps/unix/sysv/linux/microblaze/nptl/libm.abilist: Add
        __sincos_finite.

ChangeLog.mips:

	[BZ #14412]
        * sysdeps/unix/sysv/linux/mips/mips32/nptl/libm.abilist: Add
        __sincos_finite.
        * sysdeps/unix/sysv/linux/mips/mips64/n32/nptl/libm.abilist:
	Likewise.
        * sysdeps/unix/sysv/linux/mips/mips64/n64/nptl/libm.abilist:
	Likewise.

ChangeLog.powerpc:

	[BZ #14412]
        * sysdeps/unix/sysv/linux/powerpc/powerpc32/nofpu/nptl/libm.abilist: Add
        __sincos_finite.

ChangeLog.tile.

	[BZ #14412]
        * sysdeps/unix/sysv/linux/tile/tilegx/tilegx32/nptl/libm.abilist: Add
        __sincos_finite.
        * sysdeps/unix/sysv/linux/tile/tilegx/tilegx64/nptl/libm.abilist:
	Likewise.
        * sysdeps/unix/sysv/linux/tile/tilepro/nptl/libm.abilist:
	Likewise.

diff --git a/math/Versions b/math/Versions
index 513ab14..34230fc 100644
--- a/math/Versions
+++ b/math/Versions
@@ -200,5 +200,6 @@ libm {
   }
   GLIBC_2.18 {
     __issignaling; __issignalingf; __issignalingl;
+    __sincos_finite;
   }
 }
diff --git a/math/bits/math-finite.h b/math/bits/math-finite.h
index 6888462..d8cc010 100644
--- a/math/bits/math-finite.h
+++ b/math/bits/math-finite.h
@@ -451,4 +451,8 @@ __extern_always_inline long double __NTH (tgammal (long double __d))
   return __local_signgam < 0 ? -__res : __res;
 }
 # endif
+
+/* sincos.  */
+extern void __REDIRECT_NTH (sincos, (double, double *, double *),
+			    __sincos_finite);
 #endif
diff --git a/ports/sysdeps/ia64/fpu/libm_sincos.S b/ports/sysdeps/ia64/fpu/libm_sincos.S
index 7fda2af..4aa4e20 100644
--- a/ports/sysdeps/ia64/fpu/libm_sincos.S
+++ b/ports/sysdeps/ia64/fpu/libm_sincos.S
@@ -177,6 +177,8 @@
 // f9 -> f15
 // f32 -> f67
 
+#include <sysdep.h>
+
 // Assembly macros
 //==============================================================
 
@@ -781,3 +783,4 @@ _CIS_LARGE_ARGS:
 .type __libm_sincos_large#,@function
 .global __libm_sincos_large#
 
+weak_alias (__sincos_finite, sincos)
diff --git a/ports/sysdeps/m68k/m680x0/fpu/s_sincos.c b/ports/sysdeps/m68k/m680x0/fpu/s_sincos.c
index 41b64aa..94af532 100644
--- a/ports/sysdeps/m68k/m680x0/fpu/s_sincos.c
+++ b/ports/sysdeps/m68k/m680x0/fpu/s_sincos.c
@@ -33,3 +33,4 @@ CONCATX(__,FUNC) (x, sinx, cosx)
   __m81_u(CONCATX(__,FUNC))(x, sinx, cosx);
 }
 weak_alias (CONCATX(__,FUNC), FUNC)
+weak_alias (CONCATX(CONCATX(__, FUNC), finite), FUNC)
diff --git a/ports/sysdeps/unix/sysv/linux/aarch64/nptl/libm.abilist b/ports/sysdeps/unix/sysv/linux/aarch64/nptl/libm.abilist
index 0f1cfc8..f83d90b 100644
--- a/ports/sysdeps/unix/sysv/linux/aarch64/nptl/libm.abilist
+++ b/ports/sysdeps/unix/sysv/linux/aarch64/nptl/libm.abilist
@@ -400,3 +400,4 @@ GLIBC_2.18
  __issignaling F
  __issignalingf F
  __issignalingl F
+ __sincos_finite F
diff --git a/ports/sysdeps/unix/sysv/linux/alpha/nptl/libm.abilist b/ports/sysdeps/unix/sysv/linux/alpha/nptl/libm.abilist
index 400a851..03c6bc9 100644
--- a/ports/sysdeps/unix/sysv/linux/alpha/nptl/libm.abilist
+++ b/ports/sysdeps/unix/sysv/linux/alpha/nptl/libm.abilist
@@ -396,6 +396,7 @@ GLIBC_2.18
  __issignaling F
  __issignalingf F
  __issignalingl F
+ __sincos_finite F
 GLIBC_2.2
  GLIBC_2.2 A
  feclearexcept F
diff --git a/ports/sysdeps/unix/sysv/linux/arm/nptl/libm.abilist b/ports/sysdeps/unix/sysv/linux/arm/nptl/libm.abilist
index 614e5eb..c815c39 100644
--- a/ports/sysdeps/unix/sysv/linux/arm/nptl/libm.abilist
+++ b/ports/sysdeps/unix/sysv/linux/arm/nptl/libm.abilist
@@ -58,6 +58,7 @@ GLIBC_2.18
  GLIBC_2.18 A
  __issignaling F
  __issignalingf F
+ __sincos_finite F
 GLIBC_2.4
  GLIBC_2.4 A
  _LIB_VERSION D 0x4
diff --git a/ports/sysdeps/unix/sysv/linux/ia64/nptl/libm.abilist b/ports/sysdeps/unix/sysv/linux/ia64/nptl/libm.abilist
index db8b279..0b2e36e 100644
--- a/ports/sysdeps/unix/sysv/linux/ia64/nptl/libm.abilist
+++ b/ports/sysdeps/unix/sysv/linux/ia64/nptl/libm.abilist
@@ -23,6 +23,7 @@ GLIBC_2.18
  __issignaling F
  __issignalingf F
  __issignalingl F
+ __sincos_finite F
 GLIBC_2.2
  GLIBC_2.2 A
  _LIB_VERSION D 0x4
diff --git a/ports/sysdeps/unix/sysv/linux/m68k/coldfire/nptl/libm.abilist b/ports/sysdeps/unix/sysv/linux/m68k/coldfire/nptl/libm.abilist
index 614e5eb..c815c39 100644
--- a/ports/sysdeps/unix/sysv/linux/m68k/coldfire/nptl/libm.abilist
+++ b/ports/sysdeps/unix/sysv/linux/m68k/coldfire/nptl/libm.abilist
@@ -58,6 +58,7 @@ GLIBC_2.18
  GLIBC_2.18 A
  __issignaling F
  __issignalingf F
+ __sincos_finite F
 GLIBC_2.4
  GLIBC_2.4 A
  _LIB_VERSION D 0x4
diff --git a/ports/sysdeps/unix/sysv/linux/m68k/m680x0/nptl/libm.abilist b/ports/sysdeps/unix/sysv/linux/m68k/m680x0/nptl/libm.abilist
index 0beecb7..aed17c0 100644
--- a/ports/sysdeps/unix/sysv/linux/m68k/m680x0/nptl/libm.abilist
+++ b/ports/sysdeps/unix/sysv/linux/m68k/m680x0/nptl/libm.abilist
@@ -401,6 +401,7 @@ GLIBC_2.18
  __issignaling F
  __issignalingf F
  __issignalingl F
+ __sincos_finite F
 GLIBC_2.2
  GLIBC_2.2 A
  feclearexcept F
diff --git a/ports/sysdeps/unix/sysv/linux/microblaze/nptl/libm.abilist b/ports/sysdeps/unix/sysv/linux/microblaze/nptl/libm.abilist
index 8d6327c..c932e98 100644
--- a/ports/sysdeps/unix/sysv/linux/microblaze/nptl/libm.abilist
+++ b/ports/sysdeps/unix/sysv/linux/microblaze/nptl/libm.abilist
@@ -57,6 +57,7 @@ GLIBC_2.18
  __scalbf_finite F
  __signbit F
  __signbitf F
+ __sincos_finite F
  __sinh_finite F
  __sinhf_finite F
  __sqrt_finite F
diff --git a/ports/sysdeps/unix/sysv/linux/mips/mips32/nptl/libm.abilist b/ports/sysdeps/unix/sysv/linux/mips/mips32/nptl/libm.abilist
index 5381246..5fb61ea 100644
--- a/ports/sysdeps/unix/sysv/linux/mips/mips32/nptl/libm.abilist
+++ b/ports/sysdeps/unix/sysv/linux/mips/mips32/nptl/libm.abilist
@@ -216,6 +216,7 @@ GLIBC_2.18
  GLIBC_2.18 A
  __issignaling F
  __issignalingf F
+ __sincos_finite F
 GLIBC_2.2
  GLIBC_2.2 A
  __clog10 F
diff --git a/ports/sysdeps/unix/sysv/linux/mips/mips64/n32/nptl/libm.abilist b/ports/sysdeps/unix/sysv/linux/mips/mips64/n32/nptl/libm.abilist
index bb39795..9806dba 100644
--- a/ports/sysdeps/unix/sysv/linux/mips/mips64/n32/nptl/libm.abilist
+++ b/ports/sysdeps/unix/sysv/linux/mips/mips64/n32/nptl/libm.abilist
@@ -244,6 +244,7 @@ GLIBC_2.18
  __issignaling F
  __issignalingf F
  __issignalingl F
+ __sincos_finite F
 GLIBC_2.2
  GLIBC_2.2 A
  __clog10 F
diff --git a/ports/sysdeps/unix/sysv/linux/mips/mips64/n64/nptl/libm.abilist b/ports/sysdeps/unix/sysv/linux/mips/mips64/n64/nptl/libm.abilist
index bb39795..9806dba 100644
--- a/ports/sysdeps/unix/sysv/linux/mips/mips64/n64/nptl/libm.abilist
+++ b/ports/sysdeps/unix/sysv/linux/mips/mips64/n64/nptl/libm.abilist
@@ -244,6 +244,7 @@ GLIBC_2.18
  __issignaling F
  __issignalingf F
  __issignalingl F
+ __sincos_finite F
 GLIBC_2.2
  GLIBC_2.2 A
  __clog10 F
diff --git a/ports/sysdeps/unix/sysv/linux/powerpc/powerpc32/nofpu/nptl/libm.abilist b/ports/sysdeps/unix/sysv/linux/powerpc/powerpc32/nofpu/nptl/libm.abilist
index 9bd593c..8bce9a7 100644
--- a/ports/sysdeps/unix/sysv/linux/powerpc/powerpc32/nofpu/nptl/libm.abilist
+++ b/ports/sysdeps/unix/sysv/linux/powerpc/powerpc32/nofpu/nptl/libm.abilist
@@ -401,6 +401,7 @@ GLIBC_2.18
  __issignaling F
  __issignalingf F
  __issignalingl F
+ __sincos_finite F
 GLIBC_2.2
  GLIBC_2.2 A
  feclearexcept F
diff --git a/ports/sysdeps/unix/sysv/linux/tile/tilegx/tilegx32/nptl/libm.abilist b/ports/sysdeps/unix/sysv/linux/tile/tilegx/tilegx32/nptl/libm.abilist
index cb0d1a4..e5dd02c 100644
--- a/ports/sysdeps/unix/sysv/linux/tile/tilegx/tilegx32/nptl/libm.abilist
+++ b/ports/sysdeps/unix/sysv/linux/tile/tilegx/tilegx32/nptl/libm.abilist
@@ -371,3 +371,4 @@ GLIBC_2.18
  GLIBC_2.18 A
  __issignaling F
  __issignalingf F
+ __sincos_finite F
diff --git a/ports/sysdeps/unix/sysv/linux/tile/tilegx/tilegx64/nptl/libm.abilist b/ports/sysdeps/unix/sysv/linux/tile/tilegx/tilegx64/nptl/libm.abilist
index cb0d1a4..e5dd02c 100644
--- a/ports/sysdeps/unix/sysv/linux/tile/tilegx/tilegx64/nptl/libm.abilist
+++ b/ports/sysdeps/unix/sysv/linux/tile/tilegx/tilegx64/nptl/libm.abilist
@@ -371,3 +371,4 @@ GLIBC_2.18
  GLIBC_2.18 A
  __issignaling F
  __issignalingf F
+ __sincos_finite F
diff --git a/ports/sysdeps/unix/sysv/linux/tile/tilepro/nptl/libm.abilist b/ports/sysdeps/unix/sysv/linux/tile/tilepro/nptl/libm.abilist
index cb0d1a4..e5dd02c 100644
--- a/ports/sysdeps/unix/sysv/linux/tile/tilepro/nptl/libm.abilist
+++ b/ports/sysdeps/unix/sysv/linux/tile/tilepro/nptl/libm.abilist
@@ -371,3 +371,4 @@ GLIBC_2.18
  GLIBC_2.18 A
  __issignaling F
  __issignalingf F
+ __sincos_finite F
diff --git a/sysdeps/ieee754/dbl-64/s_sincos.c b/sysdeps/ieee754/dbl-64/s_sincos.c
index b6d5432..fec8793 100644
--- a/sysdeps/ieee754/dbl-64/s_sincos.c
+++ b/sysdeps/ieee754/dbl-64/s_sincos.c
@@ -44,6 +44,9 @@ __sincos (double x, double *sinx, double *cosx)
     }
 }
 weak_alias (__sincos, sincos)
+#if _FINITE_DEFINED == 0
+weak_alias (__sincos, __sincos_finite)
+#endif
 #ifdef NO_LONG_DOUBLE
 strong_alias (__sincos, __sincosl)
 weak_alias (__sincos, sincosl)
diff --git a/sysdeps/unix/sysv/linux/i386/nptl/libm.abilist b/sysdeps/unix/sysv/linux/i386/nptl/libm.abilist
index c185f0b..16deb22 100644
--- a/sysdeps/unix/sysv/linux/i386/nptl/libm.abilist
+++ b/sysdeps/unix/sysv/linux/i386/nptl/libm.abilist
@@ -401,6 +401,7 @@ GLIBC_2.18
  __issignaling F
  __issignalingf F
  __issignalingl F
+ __sincos_finite F
 GLIBC_2.2
  GLIBC_2.2 A
  __expl F
diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc32/fpu/nptl/libm.abilist b/sysdeps/unix/sysv/linux/powerpc/powerpc32/fpu/nptl/libm.abilist
index 76a4ba3..ee3b642 100644
--- a/sysdeps/unix/sysv/linux/powerpc/powerpc32/fpu/nptl/libm.abilist
+++ b/sysdeps/unix/sysv/linux/powerpc/powerpc32/fpu/nptl/libm.abilist
@@ -402,6 +402,7 @@ GLIBC_2.18
  __issignaling F
  __issignalingf F
  __issignalingl F
+ __sincos_finite F
 GLIBC_2.2
  GLIBC_2.2 A
  feclearexcept F
diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc64/nptl/libm.abilist b/sysdeps/unix/sysv/linux/powerpc/powerpc64/nptl/libm.abilist
index d309a6f..146f912 100644
--- a/sysdeps/unix/sysv/linux/powerpc/powerpc64/nptl/libm.abilist
+++ b/sysdeps/unix/sysv/linux/powerpc/powerpc64/nptl/libm.abilist
@@ -86,6 +86,7 @@ GLIBC_2.18
  __issignaling F
  __issignalingf F
  __issignalingl F
+ __sincos_finite F
 GLIBC_2.3
  GLIBC_2.3 A
  _LIB_VERSION D 0x4
diff --git a/sysdeps/unix/sysv/linux/s390/s390-32/nptl/libm.abilist b/sysdeps/unix/sysv/linux/s390/s390-32/nptl/libm.abilist
index f836c90..6b1b25e 100644
--- a/sysdeps/unix/sysv/linux/s390/s390-32/nptl/libm.abilist
+++ b/sysdeps/unix/sysv/linux/s390/s390-32/nptl/libm.abilist
@@ -398,6 +398,7 @@ GLIBC_2.18
  __issignaling F
  __issignalingf F
  __issignalingl F
+ __sincos_finite F
 GLIBC_2.2
  GLIBC_2.2 A
  fedisableexcept F
diff --git a/sysdeps/unix/sysv/linux/s390/s390-64/nptl/libm.abilist b/sysdeps/unix/sysv/linux/s390/s390-64/nptl/libm.abilist
index a0891ad..235fe94 100644
--- a/sysdeps/unix/sysv/linux/s390/s390-64/nptl/libm.abilist
+++ b/sysdeps/unix/sysv/linux/s390/s390-64/nptl/libm.abilist
@@ -86,6 +86,7 @@ GLIBC_2.18
  __issignaling F
  __issignalingf F
  __issignalingl F
+ __sincos_finite F
 GLIBC_2.2
  GLIBC_2.2 A
  _LIB_VERSION D 0x4
diff --git a/sysdeps/unix/sysv/linux/sh/nptl/libm.abilist b/sysdeps/unix/sysv/linux/sh/nptl/libm.abilist
index 92821fd..963d1ce 100644
--- a/sysdeps/unix/sysv/linux/sh/nptl/libm.abilist
+++ b/sysdeps/unix/sysv/linux/sh/nptl/libm.abilist
@@ -58,6 +58,7 @@ GLIBC_2.18
  GLIBC_2.18 A
  __issignaling F
  __issignalingf F
+ __sincos_finite F
 GLIBC_2.2
  GLIBC_2.2 A
  _LIB_VERSION D 0x4
diff --git a/sysdeps/unix/sysv/linux/sparc/sparc32/nptl/libm.abilist b/sysdeps/unix/sysv/linux/sparc/sparc32/nptl/libm.abilist
index 9a1fcb1..5643ed7 100644
--- a/sysdeps/unix/sysv/linux/sparc/sparc32/nptl/libm.abilist
+++ b/sysdeps/unix/sysv/linux/sparc/sparc32/nptl/libm.abilist
@@ -393,6 +393,7 @@ GLIBC_2.18
  __issignaling F
  __issignalingf F
  __issignalingl F
+ __sincos_finite F
 GLIBC_2.2
  GLIBC_2.2 A
  feclearexcept F
diff --git a/sysdeps/unix/sysv/linux/sparc/sparc64/nptl/libm.abilist b/sysdeps/unix/sysv/linux/sparc/sparc64/nptl/libm.abilist
index 2b41d34..a4404fe 100644
--- a/sysdeps/unix/sysv/linux/sparc/sparc64/nptl/libm.abilist
+++ b/sysdeps/unix/sysv/linux/sparc/sparc64/nptl/libm.abilist
@@ -86,6 +86,7 @@ GLIBC_2.18
  __issignaling F
  __issignalingf F
  __issignalingl F
+ __sincos_finite F
 GLIBC_2.2
  GLIBC_2.2 A
  _LIB_VERSION D 0x4
diff --git a/sysdeps/unix/sysv/linux/x86_64/64/nptl/libm.abilist b/sysdeps/unix/sysv/linux/x86_64/64/nptl/libm.abilist
index 2390934..ae87e2b 100644
--- a/sysdeps/unix/sysv/linux/x86_64/64/nptl/libm.abilist
+++ b/sysdeps/unix/sysv/linux/x86_64/64/nptl/libm.abilist
@@ -86,6 +86,7 @@ GLIBC_2.18
  __issignaling F
  __issignalingf F
  __issignalingl F
+ __sincos_finite F
 GLIBC_2.2.5
  GLIBC_2.2.5 A
  _LIB_VERSION D 0x4
diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/nptl/libm.abilist b/sysdeps/unix/sysv/linux/x86_64/x32/nptl/libm.abilist
index 1825adb..0eb2484 100644
--- a/sysdeps/unix/sysv/linux/x86_64/x32/nptl/libm.abilist
+++ b/sysdeps/unix/sysv/linux/x86_64/x32/nptl/libm.abilist
@@ -400,3 +400,4 @@ GLIBC_2.18
  __issignaling F
  __issignalingf F
  __issignalingl F
+ __sincos_finite F
diff --git a/sysdeps/x86_64/fpu/Makefile b/sysdeps/x86_64/fpu/Makefile
new file mode 100644
index 0000000..623c7aa
--- /dev/null
+++ b/sysdeps/x86_64/fpu/Makefile
@@ -0,0 +1,5 @@
+ifeq ($(subdir),math)
+libm-sysdep_routines += s_sincos_finite
+
+CFLAGS=s_sincos = -D_FINITE_DEFINED=1
+endif
diff --git a/sysdeps/x86_64/fpu/s_sincos_finite.S b/sysdeps/x86_64/fpu/s_sincos_finite.S
new file mode 100644
index 0000000..b9b8870
--- /dev/null
+++ b/sysdeps/x86_64/fpu/s_sincos_finite.S
@@ -0,0 +1,48 @@
+/* Compute sine and cosine of argument, fast and not so accurate version.
+   Copyright (C) 2013 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 <machine/asm.h>
+
+	.text
+ENTRY (__sincos_finite)
+
+	movsd	%xmm0, -8(%rsp)
+	fldl	-8(%rsp)
+	fsincos
+	fnstsw	%ax
+	testl	$0x400,%eax
+	jnz	1f
+	fstpl	(%rsi)
+	fstpl	(%rdi)
+
+	retq
+
+1:	fldpi
+	fadd	%st(0)
+	fxch	%st(1)
+2:	fprem1
+	fnstsw	%ax
+	testl	$0x400,%eax
+	jnz	2b
+	fstp	%st(1)
+	fsincos
+	fstpl	(%rsi)
+	fstpl	(%rdi)
+
+	retq
+END (__sincos_finite)

Re: [PATCH][BZ #14412] Define __sincos_finite as a fast version of sincos

[Andreas Schwab]

Siddhesh Poyarekar <siddhesh@redhat.com> writes:

> This patch brings back the assembly implementation of sincos (with
> some changes) to give a fast alternative to the default sincos
> implementation.  This is defined as __sincos_finite and is used if the
> implementing program is compiled with the -ffinite-math-only gcc flag.

This is the wrong approach.  The bug is not about not handling Inf or
NaN in sincos, but allowing inaccurate result from certain finite
numbers.  The -ffinite-math-only switch does not cover the latter.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH][BZ #14412] Define __sincos_finite as a fast version of sincos

[Joseph S. Myers]

On Mon, 29 Apr 2013, Siddhesh Poyarekar wrote:

> This patch brings back the assembly implementation of sincos (with
> some changes) to give a fast alternative to the default sincos
> implementation.  This is defined as __sincos_finite and is used if the
> implementing program is compiled with the -ffinite-math-only gcc flag.

The changes don't seem to include accurate range reduction.  Without that, 
I think this is inappropriate, as it will result in wildly inaccurate 
results for large but finite inputs.

(I've stated before that all libm tests with finite inputs and outputs 
should be run with -ffinite-math-only - and I consider that they should 
pass when they pass without that option, and should not need different 
ulps for the different ways of running them.)

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH][BZ #14412] Define __sincos_finite as a fast version of sincos

[Siddhesh Poyarekar]

On 29 April 2013 19:04, Joseph S. Myers <joseph@codesourcery.com> wrote:
> The changes don't seem to include accurate range reduction.  Without that,
> I think this is inappropriate, as it will result in wildly inaccurate
> results for large but finite inputs.

Right, I had overlooked that in my earlier submission.

> (I've stated before that all libm tests with finite inputs and outputs
> should be run with -ffinite-math-only - and I consider that they should
> pass when they pass without that option, and should not need different
> ulps for the different ways of running them.)

I wonder if this is a valid case for _fast implementations distinct
from the default implementation.  gcc could define a macro
(__FAST_MATH__ or similar) when called with -ffast-math.  This gives
us the necessary fast and not-so-accurate implementations that a lot
of people seem to want.  I had assumed that __FINITE_MATH_ONLY__ was
the right place for it, but Andreas pointed out that by definition, it
is not.

Siddhesh
--
http://siddhesh.in

Re: [PATCH][BZ #14412] Define __sincos_finite as a fast version of sincos

[Ondřej Bílka]

On Mon, Apr 29, 2013 at 01:34:42PM +0000, Joseph S. Myers wrote:
> On Mon, 29 Apr 2013, Siddhesh Poyarekar wrote:
> 
> > This patch brings back the assembly implementation of sincos (with
> > some changes) to give a fast alternative to the default sincos
> > implementation.  This is defined as __sincos_finite and is used if the
> > implementing program is compiled with the -ffinite-math-only gcc flag.
> 
> The changes don't seem to include accurate range reduction.  Without that, 
> I think this is inappropriate, as it will result in wildly inaccurate 
> results for large but finite inputs.
>
These inputs contain zero significant digits. You cannot expect any
accuracty from them.
 
> (I've stated before that all libm tests with finite inputs and outputs 
> should be run with -ffinite-math-only - and I consider that they should 
> pass when they pass without that option, and should not need different 
> ulps for the different ways of running them.)
> 
> -- 
> Joseph S. Myers
> joseph@codesourcery.com

Re: [PATCH][BZ #14412] Define __sincos_finite as a fast version of sincos

[Andrew Haley]

On 04/29/2013 03:15 PM, Ondřej Bílka wrote:
> On Mon, Apr 29, 2013 at 01:34:42PM +0000, Joseph S. Myers wrote:
>> On Mon, 29 Apr 2013, Siddhesh Poyarekar wrote:
>>
>>> This patch brings back the assembly implementation of sincos (with
>>> some changes) to give a fast alternative to the default sincos
>>> implementation.  This is defined as __sincos_finite and is used if the
>>> implementing program is compiled with the -ffinite-math-only gcc flag.
>>
>> The changes don't seem to include accurate range reduction.  Without that, 
>> I think this is inappropriate, as it will result in wildly inaccurate 
>> results for large but finite inputs.
>>
> These inputs contain zero significant digits. You cannot expect any
> accuracty from them.

You can't possibly know that.  If the caller asks for sin(2^80) you should
return a close approximation to sin(2^80), not garbage.

Andrew.

Re: [PATCH][BZ #14412] Define __sincos_finite as a fast version of sincos

[Rich Felker]

On Mon, Apr 29, 2013 at 03:53:00PM +0100, Andrew Haley wrote:
> On 04/29/2013 03:15 PM, Ondřej Bílka wrote:
> > On Mon, Apr 29, 2013 at 01:34:42PM +0000, Joseph S. Myers wrote:
> >> On Mon, 29 Apr 2013, Siddhesh Poyarekar wrote:
> >>
> >>> This patch brings back the assembly implementation of sincos (with
> >>> some changes) to give a fast alternative to the default sincos
> >>> implementation.  This is defined as __sincos_finite and is used if the
> >>> implementing program is compiled with the -ffinite-math-only gcc flag.
> >>
> >> The changes don't seem to include accurate range reduction.  Without that, 
> >> I think this is inappropriate, as it will result in wildly inaccurate 
> >> results for large but finite inputs.
> >>
> > These inputs contain zero significant digits. You cannot expect any
> > accuracty from them.
> 
> You can't possibly know that.  If the caller asks for sin(2^80) you should
> return a close approximation to sin(2^80), not garbage.

Ondřej is simply repeating the standard fallacy from people who think
floating point numbers represent intervals or some sort of
approximation with a distribution of possible "actual values" centered
on the nominal value. These views are all incorrect. Floating point
values are always exact values. Floating point _calculations_,
including conversion from decimal strings, can be inexact, but the
values represented, and used as inputs to floating point functions and
operators, are exact.

Rich

Re: [PATCH][BZ #14412] Define __sincos_finite as a fast version of sincos

[Ondřej Bílka]

On Mon, Apr 29, 2013 at 03:57:40PM +0530, Siddhesh Poyarekar wrote:
> Hi,
> 
> The assembly implementation of sincos on x86_64 was removed in favour
> of the default implementation since the former was inaccurate for some
> inputs.  This led to BZ #14412 being reported for a performance
> regression in sincos.
> 
> This patch brings back the assembly implementation of sincos (with
> some changes) to give a fast alternative to the default sincos
> implementation.  This is defined as __sincos_finite and is used if the
> implementing program is compiled with the -ffinite-math-only gcc flag.
> 
Could you add test that input is less than 1000 and 1/1000 away from
multiple of pi? If so call precise version.
Assembly should be precise on these inputs so you do not have to add
-ffast math hack.

> I have tested that this works correctly for x86_64 and i686, where the
> former uses the assembly implementation of __sincos_finite and the
> latter uses the weak alias to sincos.  I have also verified that there
> are no regressions in the testsuite on x86_64.
> 
> The patch also has changes to m68k and ia64 (in addition to the
> abilist update) since they have custom implementations of sincos.  I
> have not tested those, so I would request the maintainers to let me
> know if the changes break anything on their architectures.
> 
> Siddhesh
> 

Re: [PATCH][BZ #14412] Define __sincos_finite as a fast version of sincos

[Siddhesh Poyarekar]

On 30 April 2013 14:28, Ondřej Bílka <neleai@seznam.cz> wrote:
> Could you add test that input is less than 1000 and 1/1000 away from
> multiple of pi? If so call precise version.
> Assembly should be precise on these inputs so you do not have to add
> -ffast math hack.
>

I mentioned in the bug report that I'm going to first look at cleaning
up the current sincos implementation and then, if necessary, work on
this patch.

Siddhesh
--
http://siddhesh.in