[PATCH v2 3/4] Remove usage of TLS_MULTIPLE_THREADS_IN_TCB

Adhemerval Zanella adhemerval.zanella@linaro.org
Thu Jun 16 17:23:50 GMT 2022



> On 16 Jun 2022, at 05:48, Wilco Dijkstra <Wilco.Dijkstra@arm.com> wrote:
> 
> Hi Adhemerval,
> 
>>> For use in acquire/release atomics, it is required since code hoisting and other
>>> optimizations must be prevented. So the old implementation was buggy, and this
>>> is why we need to remove these target specific hacks.
>> 
>> Yes, I noted this checking out the Linux kernel implementation. Although I am not
>> sure if it really matter since we already have a volatile asm to should prevent code
>> hoisting. 
> 
> Yes it really does matter - volatile asm does not block any optimizations across it.
> This example shows how it fails:
> 
> int x, y;
> int g(void)
> {
>   y = 3;
>   //__atomic_fetch_add (&x, 1, __ATOMIC_ACQUIRE);
>   asm volatile ("lock add %1, 1" : "+m" (x) ::  );
>   return x + y;
> }
> 
> The value of y propagates across the acquire without reloading it:
> 
>        mov     DWORD PTR y[rip], 3
>        lock add DWORD PTR x[rip], 1
>        mov     eax, DWORD PTR x[rip]
>        add     eax, 3      // bug - no reload of y!!!
>        ret
> 
> With the atomic or "memory" constraint we get the correct:
> 
>        mov     DWORD PTR y[rip], 3
>        lock add        DWORD PTR x[rip], 1
>        mov     eax, DWORD PTR y[rip]
>        add     eax, DWORD PTR x[rip]
>        ret

Interesting, a good point to move away from rei-implement atomics operations
now that we have proper compiler support (specially now that we don’t support
tricky ABI like sparcv7).

> 
>>> Any single-threaded optimizations should be done on a much higher level and only where
>>> there is a clear performance gain. So we should get rid of all the atomic-machine headers.
>> 
>> Complete agree, I have started to clean up by first moving some architectures to use
>> compiler builtins [1]. I will check which are the architectures that still don’t use compiler
>> builtin and see if we move them.
> 
> Yes I think it should be possible to move everything to use USE_ATOMIC_COMPILER_BUILTINS.
> However targets that already use it still have a significant amount of atomic macros.

The next step will be to consolidate all the atomics macros on the generic atomic.h.



More information about the Libc-alpha mailing list