[PATCH] x86-64: Use direct TLS initial-exec access to update errno

[PATCH] x86-64: Use direct TLS initial-exec access to update errno

[H.J. Lu]

Replace __errno_location call with direct TLS initial-exec access to
update errno.

Any comments?

H.J.
---
	* sysdeps/x86_64/fpu/s_cosf.S: Use direct TLS initial-exec
	access to update errno.
	* sysdeps/x86_64/fpu/s_sincosf.S: Likewise.
	* sysdeps/x86_64/fpu/s_sinf.S: Likewise.
---
 sysdeps/x86_64/fpu/s_cosf.S    | 11 ++---------
 sysdeps/x86_64/fpu/s_sincosf.S | 11 ++---------
 sysdeps/x86_64/fpu/s_sinf.S    | 11 ++---------
 3 files changed, 6 insertions(+), 27 deletions(-)

diff --git a/sysdeps/x86_64/fpu/s_cosf.S b/sysdeps/x86_64/fpu/s_cosf.S
index 327fd27fd5..33fff5febc 100644
--- a/sysdeps/x86_64/fpu/s_cosf.S
+++ b/sysdeps/x86_64/fpu/s_cosf.S
@@ -310,15 +310,8 @@ L(arg_inf_or_nan):
 	/* Here if |x| is Inf or NAN */
 	jne	L(skip_errno_setting)	/* in case of x is NaN */
 
-	/* Align stack to 16 bytes.  */
-	subq	$8, %rsp
-	cfi_adjust_cfa_offset (8)
-	/* Here if x is Inf. Set errno to EDOM.  */
-	call	JUMPTARGET(__errno_location)
-	addq	$8, %rsp
-	cfi_adjust_cfa_offset (-8)
-
-	movl	$EDOM, (%rax)
+	movq	errno@gottpoff(%rip), %rax
+	movl	$EDOM, %fs:(%rax)
 
 	.p2align	4
 L(skip_errno_setting):
diff --git a/sysdeps/x86_64/fpu/s_sincosf.S b/sysdeps/x86_64/fpu/s_sincosf.S
index f608aa948f..8549537cbb 100644
--- a/sysdeps/x86_64/fpu/s_sincosf.S
+++ b/sysdeps/x86_64/fpu/s_sincosf.S
@@ -354,15 +354,8 @@ L(arg_inf_or_nan):
 	/* Here if |x| is Inf or NAN */
 	jne	L(skip_errno_setting)	/* in case of x is NaN */
 
-	/* Align stack to 16 bytes.  */
-	subq	$8, %rsp
-	cfi_adjust_cfa_offset (8)
-	/* Here if x is Inf. Set errno to EDOM.  */
-	call	JUMPTARGET(__errno_location)
-	addq	$8, %rsp
-	cfi_adjust_cfa_offset (-8)
-
-	movl	$EDOM, (%rax)
+	movq	errno@gottpoff(%rip), %rax
+	movl	$EDOM, %fs:(%rax)
 
 	.p2align	4
 L(skip_errno_setting):
diff --git a/sysdeps/x86_64/fpu/s_sinf.S b/sysdeps/x86_64/fpu/s_sinf.S
index c505d60091..ddf8dedbf9 100644
--- a/sysdeps/x86_64/fpu/s_sinf.S
+++ b/sysdeps/x86_64/fpu/s_sinf.S
@@ -336,15 +336,8 @@ L(arg_inf_or_nan):
 	/* Here if |x| is Inf or NAN */
 	jne	L(skip_errno_setting)	/* in case of x is NaN */
 
-	/* Align stack to 16 bytes.  */
-	subq	$8, %rsp
-	cfi_adjust_cfa_offset (8)
-	/* Here if x is Inf. Set errno to EDOM.  */
-	call	JUMPTARGET(__errno_location)
-	addq	$8, %rsp
-	cfi_adjust_cfa_offset (-8)
-
-	movl	$EDOM, (%rax)
+	movq	errno@gottpoff(%rip), %rax
+	movl	$EDOM, %fs:(%rax)
 
 	.p2align	4
 L(skip_errno_setting):
-- 
2.14.3

Re: [PATCH] x86-64: Use direct TLS initial-exec access to update errno

[Carlos O'Donell]

On 12/04/2017 05:13 PM, H.J. Lu wrote:
> Replace __errno_location call with direct TLS initial-exec access to
> update errno.
> 
> Any comments?

What are the consequences of this change?

Have you tried debugging this code in gdb and does it make it easier or
harder to debug a single threaded application?

Why make these changes?

Cheers,
Carlos.

Re: [PATCH] x86-64: Use direct TLS initial-exec access to update errno

[H.J. Lu]

On Mon, Dec 4, 2017 at 3:33 PM, Carlos O'Donell <carlos@redhat.com> wrote:
> On 12/04/2017 05:13 PM, H.J. Lu wrote:
>> Replace __errno_location call with direct TLS initial-exec access to
>> update errno.
>>
>> Any comments?
>
> What are the consequences of this change?

3 function calls are removed.

> Have you tried debugging this code in gdb and does it make it easier or
> harder to debug a single threaded application?

gdb 8 works fine:

(gdb) r
Starting program: /export/build/gnu/glibc/build-x86_64-linux/math/test-float-sin
testing float (without inline functions)

Breakpoint 1, sinf () at ../sysdeps/x86_64/fpu/s_sinf.S:339
339 movq errno@gottpoff(%rip), %rax
(gdb) p errno
$1 = 0
(gdb) next
340 movl $EDOM, %fs:(%rax)
(gdb)
345 movaps %xmm7, %xmm0 /* load x */
(gdb) p errno
$2 = 33
(gdb)


> Why make these changes?

It is motivated by

https://sourceware.org/ml/libc-alpha/2017-12/msg00101.html

-- 
H.J.

Re: [PATCH] x86-64: Use direct TLS initial-exec access to update errno

[Szabolcs Nagy]

On 05/12/17 00:21, H.J. Lu wrote:
> On Mon, Dec 4, 2017 at 3:33 PM, Carlos O'Donell <carlos@redhat.com> wrote:
>> Why make these changes?
> 
> It is motivated by
> 
> https://sourceware.org/ml/libc-alpha/2017-12/msg00101.html
> 

the failure paths of math functions are not
performance critical, ideally there should
be no errno access at all just a tail call
to something like __math_invalidf (see
flt-32/math_errf.c): reduces code size and
does all error handling at a single place
without tls abi dependent code in asm.

Re: [PATCH] x86-64: Use direct TLS initial-exec access to update errno

[Florian Weimer]

On 12/05/2017 12:13 AM, H.J. Lu wrote:
> -	/* Align stack to 16 bytes.  */
> -	subq	$8, %rsp
> -	cfi_adjust_cfa_offset (8)
> -	/* Here if x is Inf. Set errno to EDOM.  */
> -	call	JUMPTARGET(__errno_location)
> -	addq	$8, %rsp
> -	cfi_adjust_cfa_offset (-8)
> -
> -	movl	$EDOM, (%rax)
> +	movq	errno@gottpoff(%rip), %rax
> +	movl	$EDOM, %fs:(%rax)

If you replace subq/addq with push/pop of a dummy register, then, the 
__errno_location approach results in more compact code, I think.

Thanks,
Florian