Re: [PATCH v2 4/6] i386: Remove bogus THREAD_ATOMIC_* macros

[Adhemerval Zanella <adhemerval dot zanella at linaro dot org>]

On 25/01/2019 13:16, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> The x86 defines optimized THREAD_ATOMIC_* macros where reference always
>> the current thread instead of the one indicated by input 'descr' argument.
>> It work as long the input is the self thread pointer, however it generates
>> wrong code is the semantic is to set a bit atomicialy from another thread.
> 
> The entire set of THREAD_* macros is poorly documented, but this is part
> of the interface: On some targets, it is expensive to obtain the thread
> pointer, so it makes sense to cache it.  On other targets, including
> x86-64 or i386, access to the thread pointer is implicit (or can be
> cached by the compiler), so an explicit cached descriptor is
> unnecessary.
> 
> I think we can improve this a lot: We could use a new type for the
> cache, to make clear it is a cache, and make the macros type-safe.  Or
> we could try to have the compiler cache the value for us, like we do
> with __errno_location.
> 
> Thanks,
> Florian
> 

It seems that besides the attribute const annotation for __errno_location,
gcc does some conditional dead call elimination (gcc/tree-call-cdce.c) to
optimize errno handling. Are you suggesting some similar? 

It also seems that some targets support __builtin_thread_pointer, maybe
this might be better option (it might simplify compiler analysis). And 
I am not sure how compiler will handle when it is based on kernel ABI.

Anyway, I agree that adding some type-safety, document the THREAD_* macros,
evaluate if arch-specific do yield better code-gen, and check for
inconsistency is indeed a gain.