public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v2 8/9] i386: Move hypot implementation to C
@ 2021-10-28 13:43 Wilco Dijkstra
  2021-11-01 17:09 ` Adhemerval Zanella
  0 siblings, 1 reply; 5+ messages in thread
From: Wilco Dijkstra @ 2021-10-28 13:43 UTC (permalink / raw)
  To: Adhemerval Zanella; +Cc: 'GNU C Library'

Hi Adhemerval,

+double
+__ieee754_hypot (double x, double y)
+{
+  if ((isinf (x) || isinf (y))
+      && !issignaling (x) && !issignaling (y))
+    return INFINITY;
+  if (isnan (x) || isnan (y))
+    return x + y;
+
+  long double lx = x;
+  long double ly = y;
+  double r = math_narrow_eval (sqrtl (lx * lx + ly * ly));
+  math_check_force_underflow_nonneg (r);
+  return r;
+}

This looks very generic, so wouldn't it be possible to move to
sysdeps/ieee754/ldbl-96?

Wilco

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 8/9] i386: Move hypot implementation to C
  2021-10-28 13:43 [PATCH v2 8/9] i386: Move hypot implementation to C Wilco Dijkstra
@ 2021-11-01 17:09 ` Adhemerval Zanella
  0 siblings, 0 replies; 5+ messages in thread
From: Adhemerval Zanella @ 2021-11-01 17:09 UTC (permalink / raw)
  To: Wilco Dijkstra; +Cc: 'GNU C Library'



On 28/10/2021 10:43, Wilco Dijkstra wrote:
> Hi Adhemerval,
> 
> +double
> +__ieee754_hypot (double x, double y)
> +{
> +  if ((isinf (x) || isinf (y))
> +      && !issignaling (x) && !issignaling (y))
> +    return INFINITY;
> +  if (isnan (x) || isnan (y))
> +    return x + y;
> +
> +  long double lx = x;
> +  long double ly = y;
> +  double r = math_narrow_eval (sqrtl (lx * lx + ly * ly));
> +  math_check_force_underflow_nonneg (r);
> +  return r;
> +}
> 
> This looks very generic, so wouldn't it be possible to move to
> sysdeps/ieee754/ldbl-96?

I am not sure this does make sense since ldbl-96 only contains
symbols related to 'long double' and adding symbols for other
floating point types is not the best code organization imho.

Another reason is it really dependable of the underlying hardware,
since 'long double' might be implemented with software emulation
and using float/double operation might yield better performance.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 8/9] i386: Move hypot implementation to C
  2021-10-25 17:10   ` Joseph Myers
@ 2021-11-01 17:23     ` Adhemerval Zanella
  0 siblings, 0 replies; 5+ messages in thread
From: Adhemerval Zanella @ 2021-11-01 17:23 UTC (permalink / raw)
  To: Joseph Myers; +Cc: libc-alpha



On 25/10/2021 14:10, Joseph Myers wrote:
> On Mon, 25 Oct 2021, Adhemerval Zanella via Libc-alpha wrote:
> 
>> +  double r = math_narrow_eval (sqrtl (lx * lx + ly * ly));
> 
> I'd expect math_narrow_eval to be called on the result cast to double, not 
> on a value of long double type.
> 

Ack.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 8/9] i386: Move hypot implementation to C
  2021-10-25 11:57 ` [PATCH v2 8/9] i386: Move hypot implementation to C Adhemerval Zanella
@ 2021-10-25 17:10   ` Joseph Myers
  2021-11-01 17:23     ` Adhemerval Zanella
  0 siblings, 1 reply; 5+ messages in thread
From: Joseph Myers @ 2021-10-25 17:10 UTC (permalink / raw)
  To: Adhemerval Zanella; +Cc: libc-alpha

On Mon, 25 Oct 2021, Adhemerval Zanella via Libc-alpha wrote:

> +  double r = math_narrow_eval (sqrtl (lx * lx + ly * ly));

I'd expect math_narrow_eval to be called on the result cast to double, not 
on a value of long double type.

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v2 8/9] i386: Move hypot implementation to C
  2021-10-25 11:57 [PATCH v2 0/9] Improve hypot() Adhemerval Zanella
@ 2021-10-25 11:57 ` Adhemerval Zanella
  2021-10-25 17:10   ` Joseph Myers
  0 siblings, 1 reply; 5+ messages in thread
From: Adhemerval Zanella @ 2021-10-25 11:57 UTC (permalink / raw)
  To: libc-alpha

The generic hypotf is slight slower, mostly due the tricks the assembly
does to optimize the isinf/isnan/issignaling.  The generic hypot is way
slower, since the optimized implementation uses the i386 default
excessive precision to issue the operation directly.  A similar
implementation is provided instead of using the generic implementation:

Checked on i686-linux-gnu.
---
 sysdeps/i386/fpu/e_hypot.S  | 75 -------------------------------------
 sysdeps/i386/fpu/e_hypot.c  | 44 ++++++++++++++++++++++
 sysdeps/i386/fpu/e_hypotf.S | 64 -------------------------------
 3 files changed, 44 insertions(+), 139 deletions(-)
 delete mode 100644 sysdeps/i386/fpu/e_hypot.S
 create mode 100644 sysdeps/i386/fpu/e_hypot.c
 delete mode 100644 sysdeps/i386/fpu/e_hypotf.S

diff --git a/sysdeps/i386/fpu/e_hypot.S b/sysdeps/i386/fpu/e_hypot.S
deleted file mode 100644
index f2c956b77a..0000000000
--- a/sysdeps/i386/fpu/e_hypot.S
+++ /dev/null
@@ -1,75 +0,0 @@
-/* Compute the hypothenuse of X and Y.
-   Copyright (C) 1998-2021 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
-   <https://www.gnu.org/licenses/>.  */
-
-#include <sysdep.h>
-#include <i386-math-asm.h>
-#include <libm-alias-finite.h>
-
-DEFINE_DBL_MIN
-
-#ifdef PIC
-# define MO(op) op##@GOTOFF(%edx)
-#else
-# define MO(op) op
-#endif
-
-	.text
-ENTRY(__ieee754_hypot)
-#ifdef  PIC
-	LOAD_PIC_REG (dx)
-#endif
-	fldl	4(%esp)		// x
-	fxam
-	fnstsw
-	fldl	12(%esp)	// y : x
-	movb	%ah, %ch
-	fxam
-	fnstsw
-	movb	%ah, %al
-	orb	%ch, %ah
-	sahf
-	jc	1f
-	fmul	%st(0)		// y * y : x
-	fxch			// x : y * y
-	fmul	%st(0)		// x * x : y * y
-	faddp			// x * x + y * y
-	fsqrt
-	DBL_NARROW_EVAL_UFLOW_NONNEG
-2:	ret
-
-	// We have to test whether any of the parameters is Inf.
-	// In this case the result is infinity.
-1:	andb	$0x45, %al
-	cmpb	$5, %al
-	je	3f		// jump if y is Inf
-	andb	$0x45, %ch
-	cmpb	$5, %ch
-	jne	4f		// jump if x is not Inf
-	fxch
-3:	fstp	%st(1)
-	fabs
-	jmp	2b
-
-4:	testb	$1, %al
-	jnz	5f		// y is NaN
-	fxch
-5:	fstp	%st(1)
-	jmp	2b
-
-END(__ieee754_hypot)
-libm_alias_finite (__ieee754_hypot, __hypot)
diff --git a/sysdeps/i386/fpu/e_hypot.c b/sysdeps/i386/fpu/e_hypot.c
new file mode 100644
index 0000000000..d3473f2a6a
--- /dev/null
+++ b/sysdeps/i386/fpu/e_hypot.c
@@ -0,0 +1,44 @@
+/* Euclidean distance function.  Double/Binary64 i386 version.
+   Copyright (C) 2021 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
+   <https://www.gnu.org/licenses/>.  */
+
+#include <math.h>
+#include <math_private.h>
+#include <math-underflow.h>
+#include <math-narrow-eval.h>
+#include <libm-alias-finite.h>
+#include <math_config.h>
+
+/* The i386 allows to use the default excess of precision to optimize the
+   hypot implementation, since internal multiplication and sqrt is carried
+   with 80-bit FP type.  */
+double
+__ieee754_hypot (double x, double y)
+{
+  if ((isinf (x) || isinf (y))
+      && !issignaling (x) && !issignaling (y))
+    return INFINITY;
+  if (isnan (x) || isnan (y))
+    return x + y;
+
+  long double lx = x;
+  long double ly = y;
+  double r = math_narrow_eval (sqrtl (lx * lx + ly * ly));
+  math_check_force_underflow_nonneg (r);
+  return r;
+}
+libm_alias_finite (__ieee754_hypot, __hypot)
diff --git a/sysdeps/i386/fpu/e_hypotf.S b/sysdeps/i386/fpu/e_hypotf.S
deleted file mode 100644
index cec5d15403..0000000000
--- a/sysdeps/i386/fpu/e_hypotf.S
+++ /dev/null
@@ -1,64 +0,0 @@
-/* Compute the hypothenuse of X and Y.
-   Copyright (C) 1998-2021 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
-   <https://www.gnu.org/licenses/>.  */
-
-#include <sysdep.h>
-#include <i386-math-asm.h>
-#include <libm-alias-finite.h>
-
-	.text
-ENTRY(__ieee754_hypotf)
-	flds	4(%esp)		// x
-	fxam
-	fnstsw
-	flds	8(%esp)		// y : x
-	movb	%ah, %ch
-	fxam
-	fnstsw
-	movb	%ah, %al
-	orb	%ch, %ah
-	sahf
-	jc	1f
-	fmul	%st(0)		// y * y : x
-	fxch			// x : y * y
-	fmul	%st(0)		// x * x : y * y
-	faddp			// x * x + y * y
-	fsqrt
-	FLT_NARROW_EVAL
-2:	ret
-
-	// We have to test whether any of the parameters is Inf.
-	// In this case the result is infinity.
-1:	andb	$0x45, %al
-	cmpb	$5, %al
-	je	3f		// jump if y is Inf
-	andb	$0x45, %ch
-	cmpb	$5, %ch
-	jne	4f		// jump if x is not Inf
-	fxch
-3:	fstp	%st(1)
-	fabs
-	jmp	2b
-
-4:	testb	$1, %al
-	jnz	5f		// y is NaN
-	fxch
-5:	fstp	%st(1)
-	jmp	2b
-
-END(__ieee754_hypotf)
-libm_alias_finite (__ieee754_hypotf, __hypotf)
-- 
2.32.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-11-01 17:23 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-28 13:43 [PATCH v2 8/9] i386: Move hypot implementation to C Wilco Dijkstra
2021-11-01 17:09 ` Adhemerval Zanella
  -- strict thread matches above, loose matches on Subject: below --
2021-10-25 11:57 [PATCH v2 0/9] Improve hypot() Adhemerval Zanella
2021-10-25 11:57 ` [PATCH v2 8/9] i386: Move hypot implementation to C Adhemerval Zanella
2021-10-25 17:10   ` Joseph Myers
2021-11-01 17:23     ` Adhemerval Zanella

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).