Re: [Qemu-devel] [PATCH v5 07/18] qemu-thread: add simple test-and-set spinlock

[Sergey Fedorov]

On 17/05/16 22:57, Sergey Fedorov wrote:
> On 17/05/16 22:19, Richard Henderson wrote:
>> On 05/17/2016 10:13 AM, Sergey Fedorov wrote:
>>>>> +static inline void qemu_spin_lock(QemuSpin *spin)
>>>>> +{
>>>>> +    while (atomic_test_and_set_acquire(&spin->value)) {
>>> >From gcc-4.8 info page, node "__atomic Builtins", description of
>>> __atomic_test_and_set():
>>>
>>>     It should be only used for operands of type 'bool' or 'char'.
>>>
>> Hum.  I thought I remembered all operand sizes there, but I've just 
>> re-checked
>> and you're right about bool (and really only bool).
>>
>> Perhaps we should just stick with __sync_test_and_set then.  I'm thinking 
>> here
>> of e.g. armv6, a reasonable host, which can't operate on 1 byte atomic 
>> values.
>>
>
> Sorry, I can't see reading ARMv6 ARM that 1-byte access can't be
> atomic. What I've found:
>
>     B2.4.1 Normal memory attribute
>     (snip)
>     Shared Normal memory
>
>         (snip)
>         ... Reads to Shared Normal Memory that are aligned in memory
>         to the size of the access must be atomic.
>
>

Actually, here's the sample code:

    #include <stdbool.h>

    struct foo {
        bool b;
        int i;
    };

    int main(void)
    {
        struct foo f;
        __atomic_store_n(&f.b, 0, __ATOMIC_SEQ_CST);
        __atomic_store_n(&f.i, 0, __ATOMIC_SEQ_CST);
        return 0;
    }


compiles with:

    arm-linux-gnueabi-gcc -march=armv6 -O2 -c a.c

and disasm:

    00000000 <main>:
       0:    e24dd008     sub    sp, sp, #8
       4:    ee070fba     mcr    15, 0, r0, cr7, cr10, {5}
       8:    e3a03000     mov    r3, #0
       c:    e5cd3000     strb    r3, [sp]
      10:    ee070fba     mcr    15, 0, r0, cr7, cr10, {5}
      14:    ee070fba     mcr    15, 0, r0, cr7, cr10, {5}
      18:    e1a00003     mov    r0, r3
      1c:    e58d3004     str    r3, [sp, #4]
      20:    ee070fba     mcr    15, 0, r0, cr7, cr10, {5}
      24:    e28dd008     add    sp, sp, #8
      28:    e12fff1e     bx    lr

Looks like GCC has no trouble generating __atomic_store_n() for 1-byte
bool...

Kind regards,
Sergey