nptl: fix __builtin_thread_pointer detection on LoongArch
Checks
| Context |
Check |
Description |
| redhat-pt-bot/TryBot-apply_patch |
success
|
Patch applied to master at the time it was sent
|
| linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 |
success
|
Build passed
|
| redhat-pt-bot/TryBot-32bit |
success
|
Build for i686
|
| linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 |
success
|
Test passed
|
| linaro-tcwg-bot/tcwg_glibc_build--master-arm |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_glibc_check--master-arm |
success
|
Test passed
|
Commit Message
this is version 3 disscussed in https://sourceware.org/pipermail/libc-alpha/2024-November/161236.html
v2: https://sourceware.org/pipermail/libc-alpha/2024-November/161213.html
v1: https://sourceware.org/pipermail/libc-alpha/2024-November/161185.html
---
sysdeps/loongarch/nptl/thread_pointer.h | 6 ------
1 file changed, 6 deletions(-)
Comments
On Tue, 2024-11-05 at 10:34 +0800, caiyinyu wrote:
> this is version 3 disscussed in https://sourceware.org/pipermail/libc-alpha/2024-November/161236.html
> v2: https://sourceware.org/pipermail/libc-alpha/2024-November/161213.html
> v1: https://sourceware.org/pipermail/libc-alpha/2024-November/161185.html
Technically, 'register void *tp asm("tp");' may produce better code than
using asm("move %0, tp"). The reason is in the latter the compiler does
not know we are just doing a move (as it does not parse assembly).
For example
register void *_tp asm("tp");
void *tp()
{
return _tp;
}
int tp_rel_load()
{
return *((int*)tp() + 384);
}
gives:
ldptr.w $r4,$r2,1536
But if we use asm("move ...") for tp(), we'll get:
#APP
# 5 "t.c" 1
move $r12, $tp
# 0 "" 2
#NO_APP
ldptr.w $r4,$r12,1536
jr $r1
i.e. we'll be wasting one insn with asm("move ...").
Thus IMO a better approach would be moving __thread_self into
thread_pointer.h, and including thread_pointer.h from tls.h? Like:
diff --git a/sysdeps/loongarch/nptl/thread_pointer.h b/sysdeps/loongarch/nptl/thread_pointer.h
index 5dec2ef4c6..930c73606c 100644
--- a/sysdeps/loongarch/nptl/thread_pointer.h
+++ b/sysdeps/loongarch/nptl/thread_pointer.h
@@ -21,16 +21,12 @@
#include <sys/cdefs.h>
+register void *__thread_self asm("tp");
+
static inline void *
__thread_pointer (void)
{
-#if __glibc_has_builtin (__builtin_thread_pointer)
- return __builtin_thread_pointer ();
-#else
- void *__thread_register;
- __asm__ ("move %0, $tp" : "=r" (__thread_register));
- return __thread_register;
-#endif
+ return __thread_self;
}
#endif /* _SYS_THREAD_POINTER_H */
diff --git a/sysdeps/loongarch/nptl/tls.h b/sysdeps/loongarch/nptl/tls.h
index ac1a92ea7b..675c0d27c1 100644
--- a/sysdeps/loongarch/nptl/tls.h
+++ b/sysdeps/loongarch/nptl/tls.h
@@ -26,8 +26,8 @@
#include <stddef.h>
#include <stdint.h>
#include <dl-dtv.h>
+#include "thread_pointer.h"
-register void *__thread_self asm ("$tp");
#define READ_THREAD_POINTER() ({ __thread_self; })
/* Get system call information. */
?
> ---
> sysdeps/loongarch/nptl/thread_pointer.h | 6 ------
> 1 file changed, 6 deletions(-)
>
> diff --git a/sysdeps/loongarch/nptl/thread_pointer.h b/sysdeps/loongarch/nptl/thread_pointer.h
> index 5dec2ef4c6..3c5367af24 100644
> --- a/sysdeps/loongarch/nptl/thread_pointer.h
> +++ b/sysdeps/loongarch/nptl/thread_pointer.h
> @@ -19,18 +19,12 @@
> #ifndef _SYS_THREAD_POINTER_H
> #define _SYS_THREAD_POINTER_H
>
> -#include <sys/cdefs.h>
> -
> static inline void *
> __thread_pointer (void)
> {
> -#if __glibc_has_builtin (__builtin_thread_pointer)
> - return __builtin_thread_pointer ();
> -#else
> void *__thread_register;
> __asm__ ("move %0, $tp" : "=r" (__thread_register));
> return __thread_register;
> -#endif
> }
>
> #endif /* _SYS_THREAD_POINTER_H */
@@ -19,18 +19,12 @@
#ifndef _SYS_THREAD_POINTER_H
#define _SYS_THREAD_POINTER_H
-#include <sys/cdefs.h>
-
static inline void *
__thread_pointer (void)
{
-#if __glibc_has_builtin (__builtin_thread_pointer)
- return __builtin_thread_pointer ();
-#else
void *__thread_register;
__asm__ ("move %0, $tp" : "=r" (__thread_register));
return __thread_register;
-#endif
}
#endif /* _SYS_THREAD_POINTER_H */