[PATCH v5 2/3] server: Add USD support with timestamp updates.

[Jacek Caban]

On 08.05.2020 22:12, Alexandre Julliard wrote:
> Rémi Bernon <rbernon at codeweavers.com> writes:
>
>> On 5/7/20 10:45 PM, Alexandre Julliard wrote:
>>> Rémi Bernon <rbernon at codeweavers.com> writes:
>>>
>>>> +#if defined(__GNUC__) && ((__GNUC__ > 4) || ((__GNUC__ == 4) && (__GNUC_MINOR__ >= 7)))
>>>> +    __atomic_store_n(&ptr->SystemTime.High2Time, system_time_high, __ATOMIC_SEQ_CST);
>>>> +    __atomic_store_n(&ptr->SystemTime.LowPart, system_time_low, __ATOMIC_SEQ_CST);
>>>> +    __atomic_store_n(&ptr->SystemTime.High1Time, system_time_high, __ATOMIC_SEQ_CST);
>>>> +
>>>> +    __atomic_store_n(&ptr->InterruptTime.High2Time, interrupt_time_high, __ATOMIC_SEQ_CST);
>>>> +    __atomic_store_n(&ptr->InterruptTime.LowPart, interrupt_time_low, __ATOMIC_SEQ_CST);
>>>> +    __atomic_store_n(&ptr->InterruptTime.High1Time, interrupt_time_high, __ATOMIC_SEQ_CST);
>>>> +
>>>> +    __atomic_store_n(&ptr->TickCount.High2Time, tick_count_high, __ATOMIC_SEQ_CST);
>>>> +    __atomic_store_n(&ptr->TickCount.LowPart, tick_count_low, __ATOMIC_SEQ_CST);
>>>> +    __atomic_store_n(&ptr->TickCount.High1Time, tick_count_high, __ATOMIC_SEQ_CST);
>>>> +    __atomic_store_n(&ptr->TickCountLowDeprecated, tick_count_low, __ATOMIC_SEQ_CST);
>>> Is this gcc-specific code really necessary?
>>>
>> I'm not sure it is strictly necessary but then it depends what
>> guarantees we want to have. The volatile qualifier only acts as a
>> compiler barrier and will make sure that the stores aren't optimized
>> or reordered by the compiler. However it doesn't enforce anything on
>> the CPU. I believe that X86 has global store ordering guarantees, but
>> a quick search tells me that ARM or POWER don't, so the stores to the
>> various members may be seen out of order from another CPU depending on
>> the architecture.
> My concern is that if the code is necessary, it means that the fallback
> path for older gccs would yield a broken implementation. But if the
> fallback works correctly on x86 that's probably good enough.


The check should probably be adjusted for clang through. It pretends to 
be GCC 4.2 and supports __atomic_store_n (and is a required compiler on 
aarch64).


Jacek