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

[Florian Weimer <fweimer at redhat dot com>]

* Adhemerval Zanella:

> 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?

Yes, that was my idea.  But if GCC hard-codes the name of
__errno_location, that's probably not possible to do.

> 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.

It would actually need Richard Henderson's namespace patches because
some architectures use something close to that (x86 for instance).
Getting the thread pointer on these architectures requires a load of a
TLS variable.

Thanks,
Florian