On 5/9/20 1:18 AM, Jacek Caban wrote:
On 08.05.2020 22:12, Alexandre Julliard wrote:
Rémi Bernon rbernon@codeweavers.com writes:
On 5/7/20 10:45 PM, Alexandre Julliard wrote:
Rémi Bernon rbernon@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?