diff mbox series

nptl: Add <thread_pointer.h> for RISC-V

Message ID 20240914142742.9592-1-mjeanson@efficios.com
State New
Headers show
Series nptl: Add <thread_pointer.h> for RISC-V | expand

Commit Message

Michael Jeanson Sept. 14, 2024, 2:27 p.m. UTC
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

Comments

Florian Weimer Oct. 2, 2024, 8:51 a.m. UTC | #1
* 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
Michael Jeanson Oct. 8, 2024, 7:06 p.m. UTC | #2
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
Andreas Schwab Oct. 8, 2024, 7:28 p.m. UTC | #3
On Okt 08 2024, Michael Jeanson wrote:

> #if defined (__has_builtin) && __has_builtin (__builtin_thread_pointer)

This is a parse error in gcc <= 8.
Palmer Dabbelt Oct. 8, 2024, 7:49 p.m. UTC | #4
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
Michael Jeanson Oct. 10, 2024, 5:25 p.m. UTC | #5
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 ?
Michael Jeanson Oct. 10, 2024, 5:26 p.m. UTC | #6
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
Florian Weimer Oct. 10, 2024, 5:58 p.m. UTC | #7
* 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;
}
Michael Jeanson Oct. 10, 2024, 6:05 p.m. UTC | #8
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 mbox series

Patch

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 */