[PATCH v5 2/3] server: Add USD support with timestamp updates.
Rémi Bernon
rbernon at codeweavers.com
Sat May 9 04:22:45 CDT 2020
On 5/9/20 1:18 AM, Jacek Caban wrote:
> 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
>
So would it be acceptable to extend the check for clang and simply error
in the case where __atomic builtins aren't supported? Or should we keep
the fallback but check that it's on X86 only?
There's also a possible intermediate but heavier fallback, usable on
clang 3.0 or gcc >= 4.1, with __sync_synchronize, but still GCC-like
specific again.
Or, do the complete portable implementation with the asm variants for
all the architectures we need. What compilers / architectures are we
willing for wineserver?
--
Rémi Bernon <rbernon at codeweavers.com>
More information about the wine-devel
mailing list