diff mbox

[2/2] x86_64: Use __seg_tls for thread access when available

Message ID 1444276705-6797-3-git-send-email-rth@twiddle.net
State New
Headers show

Commit Message

Richard Henderson Oct. 8, 2015, 3:58 a.m. UTC 
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

Florian Weimer Oct. 8, 2015, 9:22 a.m. UTC | #1 
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
Richard Henderson Oct. 8, 2015, 9:54 a.m. UTC | #2 
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~
diff mbox

Patch

diff --git a/sysdeps/unix/sysv/linux/x86_64/arch-fork.h b/sysdeps/unix/sysv/linux/x86_64/arch-fork.h
index bb4d3ea..6698441 100644
--- a/sysdeps/unix/sysv/linux/x86_64/arch-fork.h
+++ b/sysdeps/unix/sysv/linux/x86_64/arch-fork.h
@@ -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)
diff --git a/sysdeps/x86_64/nptl/tls.h b/sysdeps/x86_64/nptl/tls.h
index bbc45f0..62634e2 100644
--- a/sysdeps/x86_64/nptl/tls.h
+++ b/sysdeps/x86_64/nptl/tls.h
@@ -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"		      \