[1/5] nptl: Add <thread_pointer.h> for defining __thread_pointer
Checks
Context |
Check |
Description |
dj/TryBot-apply_patch |
success
|
Patch applied to master at the time it was sent
|
Commit Message
<tls.h> already contains a definition that is quite similar,
but it is not readily accessible.
---
sysdeps/csky/nptl/thread_pointer.h | 25 ++++++++++++++++++
sysdeps/hppa/nptl/thread_pointer.h | 25 ++++++++++++++++++
sysdeps/ia64/nptl/thread_pointer.h | 19 ++++++++++++++
sysdeps/m68k/nptl/thread_pointer.h | 20 ++++++++++++++
sysdeps/microblaze/nptl/thread_pointer.h | 19 ++++++++++++++
sysdeps/nptl/thread_pointer.h | 19 ++++++++++++++
sysdeps/powerpc/nptl/thread_pointer.h | 23 +++++++++++++++++
sysdeps/sparc/nptl/thread_pointer.h | 19 ++++++++++++++
sysdeps/x86/nptl/thread_pointer.h | 33 ++++++++++++++++++++++++
9 files changed, 202 insertions(+)
create mode 100644 sysdeps/csky/nptl/thread_pointer.h
create mode 100644 sysdeps/hppa/nptl/thread_pointer.h
create mode 100644 sysdeps/ia64/nptl/thread_pointer.h
create mode 100644 sysdeps/m68k/nptl/thread_pointer.h
create mode 100644 sysdeps/microblaze/nptl/thread_pointer.h
create mode 100644 sysdeps/nptl/thread_pointer.h
create mode 100644 sysdeps/powerpc/nptl/thread_pointer.h
create mode 100644 sysdeps/sparc/nptl/thread_pointer.h
create mode 100644 sysdeps/x86/nptl/thread_pointer.h
Comments
----- On Dec 6, 2021, at 8:46 AM, Florian Weimer fweimer@redhat.com wrote:
> <tls.h> already contains a definition that is quite similar,
> but it is not readily accessible.
[...]
> +++ b/sysdeps/csky/nptl/thread_pointer.h
[...]
> +static inline void *
> +__thread_pointer (void)
> +{
> + void *__result;
> + __asm__ volatile ("mov %0, r31" : "=r" (__result));
Do we want volatile here ?
> + return __result;
> +}
[...]
> +++ b/sysdeps/hppa/nptl/thread_pointer.h
[...]
> +
> +static inline void *
> +__thread_pointer (void)
> +{
> + void *__result;
> + __asm__ ("mfctl %%cr27, %0" : "=r" (__result));
> + return __result;
> +}
[...]
> +++ b/sysdeps/ia64/nptl/thread_pointer.h
[...]
> +
> +#define __thread_pointer() ({ register void *__reg asm ("r13"); __reg; })
[...]
Why do we find a mix of macros and static inline across architectures ?
I would expect that we stick to one or the other.
Static inline have some advantages in terms of type-awareness. Arguably here
everything is a (void *), so there is little benefit from it.
Defining macros end up clobbering the preprocessor namespace (output of "gcc -dM -E"),
which is one reason why static inlines are sometimes preferred. Or do we intend users
to use this for feature detection ? e.g. #ifdef __thread_pointer ?
OTOH, static inline functions will not be inlined when compiling with "-fno-inline"
(default at -O0). An attribute "always_inline" is needed for this.
> +++ b/sysdeps/m68k/nptl/thread_pointer.h
[...]
> +extern void * __m68k_read_tp (void);
> +#define __thread_pointer() __m68k_read_tp ()
[...]
> +++ b/sysdeps/microblaze/nptl/thread_pointer.h
[...]
> +#define __thread_pointer() ({ register void *__reg asm ("r21"); __reg; })
Not that applications are supposed to use variable names with a leading
"__" prefix (AFAIU this is reserved to libc), but one downside of these
statement expressions is that they would shadow any "__reg" variables.
One example where things could theoretically be a problem is found in
sysdeps/x86/sys/platform/x86.h within glibc, which defines a __reg
variable.
[...]
> +++ b/sysdeps/nptl/thread_pointer.h
[...]
> +#define __thread_pointer() __builtin_thread_pointer ()
[...]
> +++ b/sysdeps/powerpc/nptl/thread_pointer.h
[...]
> +#ifdef __powerpc64__
> +# define __thread_pointer() ({ register void *__reg asm ("r13"); __reg; })
> +#else
> +# define __thread_pointer() ({ register void *__reg asm ("r2"); __reg; })
> +#endif
[ ...]
> +++ b/sysdeps/sparc/nptl/thread_pointer.h
[...]
> +#define __thread_pointer() ({ register void *__reg asm ("%g7"); __reg; })
[...]
> +++ b/sysdeps/x86/nptl/thread_pointer.h
[...]
> +#if __GNUC_PREREQ (11, 1)
I suspect this requires a #include "features.h" ?
> +# define __thread_pointer() __builtin_thread_pointer ()
> +#else
> +static inline void *
> +__thread_pointer (void)
> +{
> + void *__result;
> +# ifdef __x86_64__
> + __asm__ volatile ("mov %%fs:0, %0" : "=r" (__result));
Just out of curiosity, is x32 supported by glibc, and is this the
expected behavior ?
> +# else
> + __asm__ volatile ("mov %%gs:0, %0" : "=r" (__result));
Do we really want a "volatile" asm here ? E.g. see:
sysdeps/x86_64/nptl/tls.h:
/* Return the thread descriptor for the current thread.
The contained asm must *not* be marked volatile since otherwise
assignments like
pthread_descr self = thread_self();
do not get optimized away. */
# if __GNUC_PREREQ (6, 0)
# define THREAD_SELF \
(*(struct pthread *__seg_fs *) offsetof (struct pthread, header.self))
# else
# define THREAD_SELF \
({ struct pthread *__self; \
asm ("mov %%fs:%c1,%0" : "=r" (__self) \
: "i" (offsetof (struct pthread, header.self))); \
__self;})
# endif
Is this expected to be a complete list of all architectures ? I notably identify
that arm, aarch64, and s390 are missing. Or is it that all the other architectures
map to "sysdeps/nptl/thread_pointers.h" ? Maybe it would be good to document this
in the commit message.
Thanks,
Mathieu
> +#endif
> + return __result;
> +}
> +#endif
* Mathieu Desnoyers:
> Why do we find a mix of macros and static inline across architectures ?
It's following the existing snippets in <tls.h> for those architectures
which cannot use __builtin_thread_pointer.
> Defining macros end up clobbering the preprocessor namespace (output
> of "gcc -dM -E"), which is one reason why static inlines are sometimes
> preferred. Or do we intend users to use this for feature detection ?
> e.g. #ifdef __thread_pointer ?
It's currently not an installed header, so this isn't a concern.
>
> OTOH, static inline functions will not be inlined when compiling with "-fno-inline"
> (default at -O0). An attribute "always_inline" is needed for this.
>
>
>> +++ b/sysdeps/m68k/nptl/thread_pointer.h
> [...]
>> +extern void * __m68k_read_tp (void);
>> +#define __thread_pointer() __m68k_read_tp ()
> [...]
>
>> +++ b/sysdeps/microblaze/nptl/thread_pointer.h
> [...]
>> +#define __thread_pointer() ({ register void *__reg asm ("r21"); __reg; })
>
> Not that applications are supposed to use variable names with a leading
> "__" prefix (AFAIU this is reserved to libc), but one downside of these
> statement expressions is that they would shadow any "__reg" variables.
It doesn't matter in this context because there are no macro arguments.
The only thing that would break this would be a __reg macro.
>> +++ b/sysdeps/x86/nptl/thread_pointer.h
> [...]
>> +#if __GNUC_PREREQ (11, 1)
>
> I suspect this requires a #include "features.h" ?
It's an internal header. We'd also need include guards if we made this
into an installed header.
>> +# define __thread_pointer() __builtin_thread_pointer ()
>> +#else
>> +static inline void *
>> +__thread_pointer (void)
>> +{
>> + void *__result;
>> +# ifdef __x86_64__
>> + __asm__ volatile ("mov %%fs:0, %0" : "=r" (__result));
>
> Just out of curiosity, is x32 supported by glibc, and is this the
> expected behavior ?
x32 is supported and uses %fs.
>> +# else
>> + __asm__ volatile ("mov %%gs:0, %0" : "=r" (__result));
>
> Do we really want a "volatile" asm here ? E.g. see:
With sufficiently separate compilation, we do not need it. It's
currently not needed.
THREAD_SELF should not use it because it is often used in ways that
isn't needed after all, e.g. if THREAD_SELF is passed to THREAD_GETMEM
on an architecture that does not need a materialized thread pointer to
change TCB fields.
> Is this expected to be a complete list of all architectures ? I
> notably identify that arm, aarch64, and s390 are missing. Or is it
> that all the other architectures map to
> "sysdeps/nptl/thread_pointers.h" ? Maybe it would be good to document
> this in the commit message.
Yes, those can use __builtin_thread_pointer. The patch as is builds on
all targets with GCC 11.
Thanks,
Florian
* Florian Weimer:
> * Mathieu Desnoyers:
>
>> Why do we find a mix of macros and static inline across architectures ?
>
> It's following the existing snippets in <tls.h> for those architectures
> which cannot use __builtin_thread_pointer.
I'm reposting this separately. I was able to restrict the set of
architectures that need this.
Thanks,
Florian
new file mode 100644
@@ -0,0 +1,25 @@
+/* __thread_pointer definition. csky version.
+ Copyright (C) 2021 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/>. */
+
+static inline void *
+__thread_pointer (void)
+{
+ void *__result;
+ __asm__ volatile ("mov %0, r31" : "=r" (__result));
+ return __result;
+}
new file mode 100644
@@ -0,0 +1,25 @@
+/* __thread_pointer definition. hppa version.
+ Copyright (C) 2021 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/>. */
+
+static inline void *
+__thread_pointer (void)
+{
+ void *__result;
+ __asm__ ("mfctl %%cr27, %0" : "=r" (__result));
+ return __result;
+}
new file mode 100644
@@ -0,0 +1,19 @@
+/* __thread_pointer definition. ia64 version.
+ Copyright (C) 2021 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/>. */
+
+#define __thread_pointer() ({ register void *__reg asm ("r13"); __reg; })
new file mode 100644
@@ -0,0 +1,20 @@
+/* __thread_pointer definition. m68k version.
+ Copyright (C) 2021 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/>. */
+
+extern void * __m68k_read_tp (void);
+#define __thread_pointer() __m68k_read_tp ()
new file mode 100644
@@ -0,0 +1,19 @@
+/* __thread_pointer definition. microblaze version.
+ Copyright (C) 2021 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/>. */
+
+#define __thread_pointer() ({ register void *__reg asm ("r21"); __reg; })
new file mode 100644
@@ -0,0 +1,19 @@
+/* __thread_pointer definition. Generic version.
+ Copyright (C) 2021 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/>. */
+
+#define __thread_pointer() __builtin_thread_pointer ()
new file mode 100644
@@ -0,0 +1,23 @@
+/* __thread_pointer definition. powerpc version.
+ Copyright (C) 2021 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/>. */
+
+#ifdef __powerpc64__
+# define __thread_pointer() ({ register void *__reg asm ("r13"); __reg; })
+#else
+# define __thread_pointer() ({ register void *__reg asm ("r2"); __reg; })
+#endif
new file mode 100644
@@ -0,0 +1,19 @@
+/* __thread_pointer definition. sparc version.
+ Copyright (C) 2021 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/>. */
+
+#define __thread_pointer() ({ register void *__reg asm ("%g7"); __reg; })
new file mode 100644
@@ -0,0 +1,33 @@
+/* __thread_pointer definition. x86 version.
+ Copyright (C) 2021 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/>. */
+
+#if __GNUC_PREREQ (11, 1)
+# define __thread_pointer() __builtin_thread_pointer ()
+#else
+static inline void *
+__thread_pointer (void)
+{
+ void *__result;
+# ifdef __x86_64__
+ __asm__ volatile ("mov %%fs:0, %0" : "=r" (__result));
+# else
+ __asm__ volatile ("mov %%gs:0, %0" : "=r" (__result));
+#endif
+ return __result;
+}
+#endif