[2/2] x86_64: Use __seg_tls for thread access when available
Commit Message
The change in ARCH_FORK is to force conversion to generic address
space before casting to an integral type; otherwise what we get is
the unhelpful %fs-based address of tid.
The change in THREAD_ATOMIC_CMPXCHG_VAL is to avoid __typeof copying
the address space from descr, leading to an error: automatic variable
with non-default address space.
---
sysdeps/unix/sysv/linux/x86_64/arch-fork.h | 2 +-
sysdeps/x86_64/nptl/tls.h | 44 ++++++++++++++++++++++--------
2 files changed, 34 insertions(+), 12 deletions(-)
Comments
On 10/08/2015 05:58 AM, Richard Henderson wrote:
> The change in ARCH_FORK is to force conversion to generic address
> space before casting to an integral type; otherwise what we get is
> the unhelpful %fs-based address of tid.
>
> The change in THREAD_ATOMIC_CMPXCHG_VAL is to avoid __typeof copying
> the address space from descr, leading to an error: automatic variable
> with non-default address space.
__SEG_TLS comes from the GCC patches. It needs to be documented
somewhere, see my response on gcc-patches.
> sysdeps/unix/sysv/linux/x86_64/arch-fork.h | 2 +-
> sysdeps/x86_64/nptl/tls.h | 44 ++++++++++++++++++++++--------
> 2 files changed, 34 insertions(+), 12 deletions(-)
Changelog is missing.
> /* Install new dtv for current thread. */
> # define INSTALL_NEW_DTV(dtvp) \
> - ({ struct pthread *__pd; \
> + ({ pthread_self_t *__pd = THREAD_SELF; \
> THREAD_SETMEM (__pd, header.dtv, (dtvp)); })
Does this change code in the !__SEG_TLS case? (THREAD_SETMEM does not
evaluate the __pd argument, which is why this worked before.)
> /* Atomic compare and exchange on TLS, returning old value. */
> # define THREAD_ATOMIC_CMPXCHG_VAL(descr, member, newval, oldval) \
> - ({ __typeof (descr->member) __ret; \
> + ({ __typeof (((struct pthread *)0)->member) __ret; \
> __typeof (oldval) __old = (oldval); \
> if (sizeof (descr->member) == 4) \
> asm volatile (LOCK_PREFIX "cmpxchgl %2, %%fs:%P3" \
The sizeofs should probably refer to __ret instead of descr->member for
consistency.
Florian
On 10/08/2015 08:22 PM, Florian Weimer wrote:
> On 10/08/2015 05:58 AM, Richard Henderson wrote:
>> The change in ARCH_FORK is to force conversion to generic address
>> space before casting to an integral type; otherwise what we get is
>> the unhelpful %fs-based address of tid.
>>
>> The change in THREAD_ATOMIC_CMPXCHG_VAL is to avoid __typeof copying
>> the address space from descr, leading to an error: automatic variable
>> with non-default address space.
>
> __SEG_TLS comes from the GCC patches. It needs to be documented
> somewhere, see my response on gcc-patches.
You're right, there's all sorts of gcc documentation missing.
>> sysdeps/unix/sysv/linux/x86_64/arch-fork.h | 2 +-
>> sysdeps/x86_64/nptl/tls.h | 44 ++++++++++++++++++++++--------
>> 2 files changed, 34 insertions(+), 12 deletions(-)
>
> Changelog is missing.
Yeah, I tend to be lazy and not write them until actually comitting.
>> /* Install new dtv for current thread. */
>> # define INSTALL_NEW_DTV(dtvp) \
>> - ({ struct pthread *__pd; \
>> + ({ pthread_self_t *__pd = THREAD_SELF; \
>> THREAD_SETMEM (__pd, header.dtv, (dtvp)); })
>
> Does this change code in the !__SEG_TLS case? (THREAD_SETMEM does not
> evaluate the __pd argument, which is why this worked before.)
No it shouldn't change code in the __SEG_TLS case. Exactly because it isn't
evaluated, the initialization should be deleted. It certainly is for all of
the generic uses throughout the rest of libc, so without looking I don't see
why it wouldn't here.
>
>> /* Atomic compare and exchange on TLS, returning old value. */
>> # define THREAD_ATOMIC_CMPXCHG_VAL(descr, member, newval, oldval) \
>> - ({ __typeof (descr->member) __ret; \
>> + ({ __typeof (((struct pthread *)0)->member) __ret; \
>> __typeof (oldval) __old = (oldval); \
>> if (sizeof (descr->member) == 4) \
>> asm volatile (LOCK_PREFIX "cmpxchgl %2, %%fs:%P3" \
>
> The sizeofs should probably refer to __ret instead of descr->member for
> consistency.
Fair enough.
r~
@@ -24,4 +24,4 @@
#define ARCH_FORK() \
INLINE_SYSCALL (clone, 4, \
CLONE_CHILD_SETTID | CLONE_CHILD_CLEARTID | SIGCHLD, 0, \
- NULL, &THREAD_SELF->tid)
+ NULL, (void *)&THREAD_SELF->tid)
@@ -80,7 +80,11 @@ typedef struct
void *__padding[8];
} tcbhead_t;
+# ifdef __SEG_TLS
+typedef struct pthread __seg_tls pthread_self_t;
+# else
typedef struct pthread pthread_self_t;
+# endif
#else /* __ASSEMBLER__ */
# include <tcb-offsets.h>
@@ -133,7 +137,7 @@ typedef struct pthread pthread_self_t;
/* Install new dtv for current thread. */
# define INSTALL_NEW_DTV(dtvp) \
- ({ struct pthread *__pd; \
+ ({ pthread_self_t *__pd = THREAD_SELF; \
THREAD_SETMEM (__pd, header.dtv, (dtvp)); })
/* Return dtv of given thread descriptor. */
@@ -182,18 +186,25 @@ typedef struct pthread pthread_self_t;
assignments like
pthread_descr self = thread_self();
do not get optimized away. */
-# define THREAD_SELF \
+# ifdef __SEG_TLS
+# define THREAD_SELF ((pthread_self_t *)0)
+# else
+# define THREAD_SELF \
({ struct pthread *__self; \
asm ("mov %%fs:%c1,%0" : "=r" (__self) \
: "i" (offsetof (struct pthread, header.self))); \
__self;})
+# endif
/* Magic for libthread_db to know how to do THREAD_SELF. */
# define DB_THREAD_SELF_INCLUDE <sys/reg.h> /* For the FS constant. */
# define DB_THREAD_SELF CONST_THREAD_AREA (64, FS)
/* Read member of the thread descriptor directly. */
-# define THREAD_GETMEM(descr, member) \
+# ifdef __SEG_TLS
+# define THREAD_GETMEM(descr, member) ({ (descr)->member; })
+# else
+# define THREAD_GETMEM(descr, member) \
({ __typeof (descr->member) __value; \
if (sizeof (__value) == 1) \
asm volatile ("movb %%fs:%P2,%b0" \
@@ -215,10 +226,13 @@ typedef struct pthread pthread_self_t;
: "i" (offsetof (struct pthread, member))); \
} \
__value; })
-
+# endif
/* Same as THREAD_GETMEM, but the member offset can be non-constant. */
-# define THREAD_GETMEM_NC(descr, member, idx) \
+# ifdef __SEG_TLS
+# define THREAD_GETMEM_NC(descr, member, idx) ({ (descr)->member[idx]; })
+# else
+# define THREAD_GETMEM_NC(descr, member, idx) \
({ __typeof (descr->member[0]) __value; \
if (sizeof (__value) == 1) \
asm volatile ("movb %%fs:%P2(%q3),%b0" \
@@ -242,7 +256,7 @@ typedef struct pthread pthread_self_t;
"r" (idx)); \
} \
__value; })
-
+# endif
/* Loading addresses of objects on x86-64 needs to be treated special
when generating PIC code. */
@@ -254,7 +268,11 @@ typedef struct pthread pthread_self_t;
/* Set member of the thread descriptor directly. */
-# define THREAD_SETMEM(descr, member, value) \
+# ifdef __SEG_TLS
+# define THREAD_SETMEM(descr, member, value) \
+ ({ (descr)->member = (value); (void)0; })
+# else
+# define THREAD_SETMEM(descr, member, value) \
({ if (sizeof (descr->member) == 1) \
asm volatile ("movb %b0,%%fs:%P1" : \
: "iq" (value), \
@@ -274,10 +292,14 @@ typedef struct pthread pthread_self_t;
: IMM_MODE ((uint64_t) cast_to_integer (value)), \
"i" (offsetof (struct pthread, member))); \
}})
-
+# endif
/* Same as THREAD_SETMEM, but the member offset can be non-constant. */
-# define THREAD_SETMEM_NC(descr, member, idx, value) \
+# ifdef __SEG_TLS
+# define THREAD_SETMEM_NC(descr, member, idx, value) \
+ ({ (descr)->member[idx] = (value); (void)0; })
+# else
+# define THREAD_SETMEM_NC(descr, member, idx, value) \
({ if (sizeof (descr->member[0]) == 1) \
asm volatile ("movb %b0,%%fs:%P1(%q2)" : \
: "iq" (value), \
@@ -300,11 +322,11 @@ typedef struct pthread pthread_self_t;
"i" (offsetof (struct pthread, member[0])), \
"r" (idx)); \
}})
-
+# endif
/* Atomic compare and exchange on TLS, returning old value. */
# define THREAD_ATOMIC_CMPXCHG_VAL(descr, member, newval, oldval) \
- ({ __typeof (descr->member) __ret; \
+ ({ __typeof (((struct pthread *)0)->member) __ret; \
__typeof (oldval) __old = (oldval); \
if (sizeof (descr->member) == 4) \
asm volatile (LOCK_PREFIX "cmpxchgl %2, %%fs:%P3" \