Message ID | 20171204231315.GA17402@intel.com |
---|---|
State | New |
Headers | show |
Series | x86-64: Use direct TLS initial-exec access to update errno | expand |
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.
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
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.
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
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):