| Message ID | 20241104023256.2983322-2-caiyinyu@loongson.cn |
|---|---|
| State | New |
| Headers | show |
| Series | nptl: fix __builtin_thread_pointer detection on LoongArch | expand |
> This is the version 2, disscussed in https://sourceware.org/pipermail/libc-alpha/2024-November/161208.html > v1: https://sourceware.org/pipermail/libc-alpha/2024-November/161185.html > --- > sysdeps/loongarch/nptl/thread_pointer.h | 8 +------- > 1 file changed, 1 insertion(+), 7 deletions(-) > > diff --git a/sysdeps/loongarch/nptl/thread_pointer.h b/sysdeps/loongarch/nptl/thread_pointer.h > index 5dec2ef4c6..96ac47ecbf 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> > +register void *__thread_register 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 > } > > #endif /* _SYS_THREAD_POINTER_H */ This may result in: warning: register of ‘tp’ used for multiple global register variables with GCC if the application already uses a similar construct. I don't know if the warning is just cosmetic, or results in code generation problems. Thanks, Florian
On Mon, 2024-11-04 at 07:28 +0100, Florian Weimer wrote: > > This is the version 2, disscussed in https://sourceware.org/pipermail/libc-alpha/2024-November/161208.html > > v1: https://sourceware.org/pipermail/libc-alpha/2024-November/161185.html > > --- > > sysdeps/loongarch/nptl/thread_pointer.h | 8 +------- > > 1 file changed, 1 insertion(+), 7 deletions(-) > > > > diff --git a/sysdeps/loongarch/nptl/thread_pointer.h b/sysdeps/loongarch/nptl/thread_pointer.h > > index 5dec2ef4c6..96ac47ecbf 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> > > +register void *__thread_register 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 > > } > > > > #endif /* _SYS_THREAD_POINTER_H */ > > This may result in: > > warning: register of ‘tp’ used for multiple global register variables > > with GCC if the application already uses a similar construct. I don't > know if the warning is just cosmetic, or results in code generation > problems. This file isn't installed, so as long as the warning does not alarm when we build Glibc it should be fine.
On 2024-11-04 01:45, Xi Ruoyao wrote: > > This file isn't installed, so as long as the warning does not alarm when > we build Glibc it should be fine. For the record the global 'register' variable was my first approach but it resulted in build failures on other architectures where the same register was used in another header. There was also some talk of eventually promoting 'thread_pointer.h' to an installed header. That being said, any working solution is fine with me, I just want to use __thread_pointer () in a future rseq related patch :). Thanks, Michael
在 2024/11/4 下午2:28, Florian Weimer 写道: >> This is the version 2, disscussed in https://sourceware.org/pipermail/libc-alpha/2024-November/161208.html >> v1: https://sourceware.org/pipermail/libc-alpha/2024-November/161185.html >> --- >> sysdeps/loongarch/nptl/thread_pointer.h | 8 +------- >> 1 file changed, 1 insertion(+), 7 deletions(-) >> >> diff --git a/sysdeps/loongarch/nptl/thread_pointer.h b/sysdeps/loongarch/nptl/thread_pointer.h >> index 5dec2ef4c6..96ac47ecbf 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> >> +register void *__thread_register 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 >> } >> >> #endif /* _SYS_THREAD_POINTER_H */ > This may result in: > > warning: register of ‘tp’ used for multiple global register variables Indeed, a warning has been generated. we use " register void *__thread_self asm ("$tp"); " in sysdeps/loongarch/nptl/tls.h like riscv or1k ... So let's go with the following: (ps: When using the |-O2|optimization level during compilation, all implementations have no differences at the assembly level. ) 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 */ > > with GCC if the application already uses a similar construct. I don't > know if the warning is just cosmetic, or results in code generation > problems. > > Thanks, > Florian
sorry for the mess on libalpha :( this is re-send. see https://sourceware.org/pipermail/libc-alpha/2024-November/161237.html 在 2024/11/5 上午10:28, caiyinyu 写道: > > > 在 2024/11/4 下午2:28, Florian Weimer 写道: >>> This is the version 2, disscussed inhttps://sourceware.org/pipermail/libc-alpha/2024-November/161208.html >>> v1:https://sourceware.org/pipermail/libc-alpha/2024-November/161185.html >>> --- >>> sysdeps/loongarch/nptl/thread_pointer.h | 8 +------- >>> 1 file changed, 1 insertion(+), 7 deletions(-) >>> >>> diff --git a/sysdeps/loongarch/nptl/thread_pointer.h b/sysdeps/loongarch/nptl/thread_pointer.h >>> index 5dec2ef4c6..96ac47ecbf 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> >>> +register void *__thread_register 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 >>> } >>> >>> #endif /* _SYS_THREAD_POINTER_H */ >> This may result in: >> >> warning: register of ‘tp’ used for multiple global register variables > Indeed, a warning has been generated. > we use " register void *__thread_self asm ("$tp"); " in > sysdeps/loongarch/nptl/tls.h like riscv or1k ... > > So let's go with the following: (ps: When using the |-O2|optimization > level during compilation, all implementations have no differences at > the assembly level. ) > > 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 */ > >> with GCC if the application already uses a similar construct. I don't >> know if the warning is just cosmetic, or results in code generation >> problems. >> >> Thanks, >> Florian
diff --git a/sysdeps/loongarch/nptl/thread_pointer.h b/sysdeps/loongarch/nptl/thread_pointer.h index 5dec2ef4c6..96ac47ecbf 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> +register void *__thread_register 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 } #endif /* _SYS_THREAD_POINTER_H */