Message ID | 20240914142742.9592-1-mjeanson@efficios.com |
---|---|
State | New |
Headers | show |
Series | nptl: Add <thread_pointer.h> for RISC-V | expand |
* Michael Jeanson: > This will be required by the rseq extensible ABI implementation on all > Linux architectures exposing the '__rseq_size' and '__rseq_offset' > symbols to set the initial value of the 'cpu_id' field which can be used > by applications to test if rseq is available and registered. As long as > the symbols are exposed it is valid for an application to perform this > test even if rseq is not yet implemented in libc for this architecture. > > Signed-off-by: Michael Jeanson <mjeanson@efficios.com> > --- > Cc: Florian Weimer <fweimer@redhat.com> > Cc: Palmer Dabbelt <palmer@rivosinc.com> > Cc: Darius Rad <darius@bluespec.com> > --- > sysdeps/riscv/nptl/thread_pointer.h | 34 +++++++++++++++++++++++++++++ > 1 file changed, 34 insertions(+) > create mode 100644 sysdeps/riscv/nptl/thread_pointer.h > > diff --git a/sysdeps/riscv/nptl/thread_pointer.h b/sysdeps/riscv/nptl/thread_pointer.h > new file mode 100644 > index 0000000000..e7f6296e7d > --- /dev/null > +++ b/sysdeps/riscv/nptl/thread_pointer.h > @@ -0,0 +1,34 @@ > +/* __thread_pointer definition. riscv version. > + Copyright (C) 2021-2024 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/>. */ > + > +#ifndef _SYS_THREAD_POINTER_H > +#define _SYS_THREAD_POINTER_H > + > +static inline void * > +__thread_pointer (void) > +{ > +#if __GNUC_PREREQ (10, 3) > + return __builtin_thread_pointer (); > +#else > + void *__result; > + __asm__ ("mv %0, tp" : "=r" (__result)); > + return __result; > +#endif /* !GCC 10.3 */ > +} > + > +#endif /* _SYS_THREAD_POINTER_H */ Jeff, would you be able to find a reviewer for this? Would this fallback implementation be preferable for GCC? static inline void * __thread_pointer (void) { register void *__tp __asm__ ("tp"); return __tp; } (It does not work on Clang.) Can we use __has_builtin (__builtin_thread_pointer) instead of __GNUC_PREREQ (10, 3)? I assume Clang only flags the builtin as supported if it is actually implemented and does not result in a compilation error when actually used. Thanks, Florian
On 2024-10-02 04 h 51, Florian Weimer wrote: > Jeff, would you be able to find a reviewer for this? Would this > fallback implementation be preferable for GCC? > > static inline void * > __thread_pointer (void) > { > register void *__tp __asm__ ("tp"); > return __tp; > } > > (It does not work on Clang.) > > Can we use __has_builtin (__builtin_thread_pointer) instead of > __GNUC_PREREQ (10, 3)? I assume Clang only flags the builtin as > supported if it is actually implemented and does not result in a > compilation error when actually used. I tested the following code with various Clang versions on riscv64: static inline void * __thread_pointer (void) { #if defined (__has_builtin) && __has_builtin (__builtin_thread_pointer) return __builtin_thread_pointer (); #else register void *__tp __asm__ ("tp"); return __tp; #endif } It works starting with Clang 11.0, prior versions will pass the __has_builtin check but fail compilation with: fatal error: error in backend: Cannot select: intrinsic %llvm.thread.pointer
On Okt 08 2024, Michael Jeanson wrote:
> #if defined (__has_builtin) && __has_builtin (__builtin_thread_pointer)
This is a parse error in gcc <= 8.
On Tue, 08 Oct 2024 12:06:33 PDT (-0700), mjeanson@efficios.com wrote: > On 2024-10-02 04 h 51, Florian Weimer wrote: >> Jeff, would you be able to find a reviewer for this? Would this >> fallback implementation be preferable for GCC? >> >> static inline void * >> __thread_pointer (void) >> { >> register void *__tp __asm__ ("tp"); >> return __tp; >> } >> >> (It does not work on Clang.) >> >> Can we use __has_builtin (__builtin_thread_pointer) instead of >> __GNUC_PREREQ (10, 3)? I assume Clang only flags the builtin as >> supported if it is actually implemented and does not result in a >> compilation error when actually used. > > I tested the following code with various Clang versions on riscv64: > > static inline void * > __thread_pointer (void) > { > #if defined (__has_builtin) && __has_builtin (__builtin_thread_pointer) > return __builtin_thread_pointer (); > #else > register void *__tp __asm__ ("tp"); > return __tp; > #endif > } Sorry I missed this eariler. I'm not entirely sure why, but we're using register struct task_struct *riscv_current_is_tp __asm__("tp"); static __always_inline struct task_struct *get_current(void) { return riscv_current_is_tp; } in Linux for this and it seems to work fine in both GCC and LLVM -- or at least nobody's complained about it being broken, and we've got a lot of other workarounds piled up from various toolchain versioning oddities. > It works starting with Clang 11.0, prior versions will pass the > __has_builtin check but fail compilation with: > > fatal error: error in backend: Cannot select: intrinsic %llvm.thread.pointer
On 2024-10-08 21:28, Andreas Schwab wrote: > On Okt 08 2024, Michael Jeanson wrote: > >> #if defined (__has_builtin) && __has_builtin (__builtin_thread_pointer) > > This is a parse error in gcc <= 8. Would the proper way be to use '__glibc_has_builtin ()' from sys/cdefs.h ?
On 2024-10-08 21:49, Palmer Dabbelt wrote: > Sorry I missed this eariler. I'm not entirely sure why, but we're using > > register struct task_struct *riscv_current_is_tp __asm__("tp"); > static __always_inline struct task_struct *get_current(void) > { > return riscv_current_is_tp; > } > > in Linux for this and it seems to work fine in both GCC and LLVM -- or at least nobody's complained about it being broken, and we've got a lot of other workarounds piled up from various toolchain versioning oddities. I'll use this instead of the inline assembly in the next version of the patch. Thanks, Michael
* Michael Jeanson: > On 2024-10-02 04 h 51, Florian Weimer wrote: >> Jeff, would you be able to find a reviewer for this? Would this >> fallback implementation be preferable for GCC? >> >> static inline void * >> __thread_pointer (void) >> { >> register void *__tp __asm__ ("tp"); >> return __tp; >> } >> >> (It does not work on Clang.) >> >> Can we use __has_builtin (__builtin_thread_pointer) instead of >> __GNUC_PREREQ (10, 3)? I assume Clang only flags the builtin as >> supported if it is actually implemented and does not result in a >> compilation error when actually used. > > I tested the following code with various Clang versions on riscv64: > > static inline void * > __thread_pointer (void) > { > #if defined (__has_builtin) && __has_builtin (__builtin_thread_pointer) > return __builtin_thread_pointer (); > #else > register void *__tp __asm__ ("tp"); > return __tp; > #endif > } > > It works starting with Clang 11.0, prior versions will pass the > __has_builtin check but fail compilation with: > > fatal error: error in backend: Cannot select: intrinsic %llvm.thread.pointer Have you checked that it actually uses the register tp? With this: void * __thread_pointer (void) { register void *__tp __asm__ ("tp"); return __tp; } Clang does not use the register at all. As Palmer mentioned, you need to use something like this: register void *__tp __asm__ ("tp"); void * __thread_pointer (void) { return __tp; }
On 2024-10-10 19:58, Florian Weimer wrote: > Have you checked that it actually uses the register tp? > > With this: > > void * > __thread_pointer (void) > { > register void *__tp __asm__ ("tp"); > return __tp; > } > > Clang does not use the register at all. > > As Palmer mentioned, you need to use something like this: > > register void *__tp __asm__ ("tp"); > > void * > __thread_pointer (void) > { > return __tp; > } Oh, I missed the fact that the register variable in his code was global, I was indeed a bit puzzled by the generated asm. I'll wait a bit for other comments and spin a v3. Sorry about that, Michael
diff --git a/sysdeps/riscv/nptl/thread_pointer.h b/sysdeps/riscv/nptl/thread_pointer.h new file mode 100644 index 0000000000..e7f6296e7d --- /dev/null +++ b/sysdeps/riscv/nptl/thread_pointer.h @@ -0,0 +1,34 @@ +/* __thread_pointer definition. riscv version. + Copyright (C) 2021-2024 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/>. */ + +#ifndef _SYS_THREAD_POINTER_H +#define _SYS_THREAD_POINTER_H + +static inline void * +__thread_pointer (void) +{ +#if __GNUC_PREREQ (10, 3) + return __builtin_thread_pointer (); +#else + void *__result; + __asm__ ("mv %0, tp" : "=r" (__result)); + return __result; +#endif /* !GCC 10.3 */ +} + +#endif /* _SYS_THREAD_POINTER_H */
This will be required by the rseq extensible ABI implementation on all Linux architectures exposing the '__rseq_size' and '__rseq_offset' symbols to set the initial value of the 'cpu_id' field which can be used by applications to test if rseq is available and registered. As long as the symbols are exposed it is valid for an application to perform this test even if rseq is not yet implemented in libc for this architecture. Signed-off-by: Michael Jeanson <mjeanson@efficios.com> --- Cc: Florian Weimer <fweimer@redhat.com> Cc: Palmer Dabbelt <palmer@rivosinc.com> Cc: Darius Rad <darius@bluespec.com> --- sysdeps/riscv/nptl/thread_pointer.h | 34 +++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) create mode 100644 sysdeps/riscv/nptl/thread_pointer.h