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

[Richard Henderson]

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~