This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 1/2] Optimize generic spinlock code and use C11 like atomic macros.


On 23/03/17 16:15, Stefan Liebler wrote:
> On 03/22/2017 01:56 PM, Szabolcs Nagy wrote:
>> the performance of the unconteded case can be improved
>> slightly by reverting the unlock change (the release
>> store is stronger than the barrier was, conceptually
>> there is a barrier before and after an armv8 release
>> store to prevent an independent load-acquire to get
>> reordered with it in either direction)
>>
> Thus you mean something like the following?
>   atomic_thread_fence_release ();
>   atomic_store_relaxed (lock, 0);
> (Info: I've used scripts/build-many-glibcs.py to get the following objdumps. The
> sysdeps/powerpc/nptl/pthread_spin_unlock.c is using atomic_store_release, too. For the following
> powerpc64-linux-gnu objdumps, I've removed the powerpc-spinlock implementation to see the differences)
> =>aarch64-linux-gnu
> 0000000000000000 <pthread_spin_unlock>:
>    0:   d5033bbf        dmb     ish
>    4:   b900001f        str     wzr, [x0]
>    8:   52800000        mov     w0, #0x0                        // #0
>    c:   d65f03c0        ret
> =>powerpc64-linux-gnu:
> 0000000000000000 <.pthread_spin_unlock>:
>    0:    7c 69 1b 78     mr      r9,r3
>    4:    7c 20 04 ac     lwsync
>    8:    39 40 00 00     li      r10,0
>    c:    38 60 00 00     li      r3,0
>   10:    91 49 00 00     stw     r10,0(r9)
>   14:    4e 80 00 20     blr
> 
> Here is the upstream code as comparison:
>   atomic_full_barrier ();
>   *lock = 0;
> =>aarch64-linux-gnu
> 0000000000000000 <pthread_spin_unlock>:
>    0:   aa0003e1        mov     x1, x0
>    4:   d5033bbf        dmb     ish
>    8:   52800000        mov     w0, #0x0                        // #0
>    c:   b900003f        str     wzr, [x1]
>   10:   d65f03c0        ret
> =>powerpc64-linux-gnu:
> 0000000000000000 <.pthread_spin_unlock>:
>    0:    7c 69 1b 78     mr      r9,r3
>    4:    7c 00 04 ac     hwsync
>    8:    39 40 00 00     li      r10,0
>    c:    38 60 00 00     li      r3,0
>   10:    91 49 00 00     stw     r10,0(r9)
>   14:    4e 80 00 20     blr
> 

i compared this (full barrier) to

> And the code of my latest patch:
>   atomic_store_release (lock, 0);
> =>aarch64-linux-gnu
> 0000000000000000 <pthread_spin_unlock>:
>    0:   889ffc1f        stlr    wzr, [x0]
>    4:   52800000        mov     w0, #0x0                        // #0
>    8:   d65f03c0        ret
> =>powerpc64-linux-gnu:
> 0000000000000000 <.pthread_spin_unlock>:
>    0:    7c 69 1b 78     mr      r9,r3
>    4:    7c 20 04 ac     lwsync
>    8:    39 40 00 00     li      r10,0
>    c:    38 60 00 00     li      r3,0
>   10:    91 49 00 00     stw     r10,0(r9)
>   14:    4e 80 00 20     blr
> 

to this (release store).

but meanwhile i convinced myself that stlr
makes more sense architecturally (even though
on the particular implementation i tested
this on it was slower).

so i'd prefer keeping the atomic_store_release.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]