[PATCH v5 0/3] MR7237: Draft: include: Fix ReadNoFence64 on i386.

[Jinoh Kang (＠iamahuman)]

64-bit volatile accesses are not atomic on i386.  Both
i686-w64-mingw32-gcc and x86 MSVC splits 64-bit loads into a pair of
load/store ops.

Fix this by using a FILD/FISTP pair, which is also used to implement C11
atomics by i686-w64-mingw32-gcc.

Fixes: f82b1c1fcf770a5d6fa02c3f286282be79a201b8

--
  v5: include: Prevent misuse of __WINE_ATOMIC_* helper macros for non-atomic large accesses.
      server: Fix incorrect usage of __WINE_ATOMIC_STORE_RELEASE in SHARED_WRITE_BEGIN/SHARED_WRITE_END.
      include: Fix ReadNoFence64 on i386.

https://gitlab.winehq.org/wine/wine/-/merge_requests/7237

[PATCH v5 1/3] include: Fix ReadNoFence64 on i386.

[Jinoh Kang]

From: Jinoh Kang

64-bit volatile loads are not atomic on i386.  Both i686-w64-mingw32-gcc
and x86 MSVC splits 64-bit loads into a pair of load ops.

Fix this by using a FILD/FISTP pair, which is also used to implement C11
atomic_load() by i686-w64-mingw32-gcc.

Fixes: f82b1c1fcf770a5d6fa02c3f286282be79a201b8
---
 include/winnt.h | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/include/winnt.h b/include/winnt.h
index 2547f841b7c..ef9e7bf6713 100644
--- a/include/winnt.h
+++ b/include/winnt.h
@@ -7122,7 +7122,16 @@ static FORCEINLINE LONG ReadNoFence( LONG const volatile *src )

static FORCEINLINE LONG64 ReadNoFence64( LONG64 const volatile *src )
 {
-    LONG64 value = __WINE_LOAD64_NO_FENCE( (__int64 const volatile *)src );
+    LONG64 value;
+#if defined(__i386__) && _MSC_VER < 1700
+    __asm {
+        mov   eax, src
+        fild  qword ptr [eax]
+        fistp value
+    }
+#else
+    value = __WINE_LOAD64_NO_FENCE( (__int64 const volatile *)src );
+#endif
     return value;
 }

@@ -7315,7 +7324,11 @@ static FORCEINLINE LONG ReadNoFence( LONG const volatile *src )
 static FORCEINLINE LONG64 ReadNoFence64( LONG64 const volatile *src )
 {
     LONG64 value;
+#ifdef __i386__
+    __asm__ __volatile__( "fildq %1\n\tfistpq %0" : "=m" (value) : "m" (*src) : "memory", "st" );
+#else
     __WINE_ATOMIC_LOAD_RELAXED( src, &value );
+#endif
     return value;
 }

[PATCH v5 2/3] server: Fix incorrect usage of __WINE_ATOMIC_STORE_RELEASE in SHARED_WRITE_BEGIN/SHARED_WRITE_END.

[Jinoh Kang]

From: Jinoh Kang

64-bit volatile stores are not atomic on i386.  Both
i686-w64-mingw32-gcc and x86 MSVC splits 64-bit stores into a pair of
store ops.

Fix this by using a FILD/FISTP pair, which is also used to implement C11
atomic_store() by i686-w64-mingw32-gcc.

Fixes: fac940dfac314c2b1c120cf9ff8503259153c5a0
---
 include/winnt.h | 26 ++++++++++++++++++++++++++
 server/file.h   |  4 ++--
 2 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/include/winnt.h b/include/winnt.h
index ef9e7bf6713..7e8ebdb6ca7 100644
--- a/include/winnt.h
+++ b/include/winnt.h
@@ -7081,13 +7081,16 @@ static FORCEINLINE void MemoryBarrier(void)
 #pragma intrinsic(__iso_volatile_load32)
 #pragma intrinsic(__iso_volatile_load64)
 #pragma intrinsic(__iso_volatile_store32)
+#pragma intrinsic(__iso_volatile_store64)
 #define __WINE_LOAD32_NO_FENCE(src) (__iso_volatile_load32(src))
 #define __WINE_LOAD64_NO_FENCE(src) (__iso_volatile_load64(src))
 #define __WINE_STORE32_NO_FENCE(dest, value) (__iso_volatile_store32(dest, value))
+#define __WINE_STORE64_NO_FENCE(dest, value) (__iso_volatile_store64(dest, value))
 #else  /* _MSC_VER >= 1700 */
 #define __WINE_LOAD32_NO_FENCE(src) (*(src))
 #define __WINE_LOAD64_NO_FENCE(src) (*(src))
 #define __WINE_STORE32_NO_FENCE(dest, value) ((void)(*(dest) = (value)))
+#define __WINE_STORE64_NO_FENCE(dest, value) ((void)(*(dest) = (value)))
 #endif  /* _MSC_VER >= 1700 */

#if defined(__i386__) || defined(__x86_64__)
@@ -7141,6 +7144,20 @@ static FORCEINLINE void WriteRelease( LONG volatile *dest, LONG value )
     __WINE_STORE32_NO_FENCE( (int volatile *)dest, value );
 }

+static FORCEINLINE void WriteRelease64( LONG64 volatile *dest, LONG64 value )
+{
+#if defined(__i386__) && _MSC_VER < 1700
+    __asm {
+        mov   eax, dest
+        fild  value
+        fistp qword ptr [eax]
+    }
+#else
+    __wine_memory_barrier_acq_rel();
+    __WINE_STORE64_NO_FENCE( (__int64 volatile *)dest, value );
+#endif
+}
+
 static FORCEINLINE void WriteNoFence( LONG volatile *dest, LONG value )
 {
     __WINE_STORE32_NO_FENCE( (int volatile *)dest, value );
@@ -7337,6 +7354,15 @@ static FORCEINLINE void WriteRelease( LONG volatile *dest, LONG value )
     __WINE_ATOMIC_STORE_RELEASE( dest, &value );
 }

+static FORCEINLINE void WriteRelease64( LONG64 volatile *dest, LONG64 value )
+{
+#ifdef __i386__
+    __asm__ __volatile__( "fildq %1\n\tfistpq %0" : "=m" (*dest) : "m" (value) : "memory", "st" );
+#else
+    __WINE_ATOMIC_STORE_RELEASE( dest, &value );
+#endif
+}
+
 static FORCEINLINE void WriteNoFence( LONG volatile *dest, LONG value )
 {
     __WINE_ATOMIC_STORE_RELAXED( dest, &value );
diff --git a/server/file.h b/server/file.h
index 4f5fc7b26f1..7368640a674 100644
--- a/server/file.h
+++ b/server/file.h
@@ -205,13 +205,13 @@ extern struct obj_locator get_shared_object_locator( const volatile void *object
         shared_object_t *__obj = CONTAINING_RECORD( shared, shared_object_t, shm );  \
         LONG64 __seq = __obj->seq + 1, __end = __seq + 1;               \
         assert( (__seq & 1) != 0 );                                     \
-        __WINE_ATOMIC_STORE_RELEASE( &__obj->seq, &__seq );             \
+        WriteRelease64( &__obj->seq, __seq );                           \
         do

#define SHARED_WRITE_END                                                \
         while(0);                                                       \
         assert( __seq == __obj->seq );                                  \
-        __WINE_ATOMIC_STORE_RELEASE( &__obj->seq, &__end );             \
+        WriteRelease64( &__obj->seq, __end );                           \
     } while(0)

/* device functions */

[PATCH v5 3/3] include: Prevent misuse of __WINE_ATOMIC_* helper macros for non-atomic large accesses.

[Jinoh Kang]

From: Jinoh Kang

---
 include/winnt.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/winnt.h b/include/winnt.h
index 7e8ebdb6ca7..f671b7d64a5 100644
--- a/include/winnt.h
+++ b/include/winnt.h
@@ -7313,10 +7313,10 @@ static FORCEINLINE void MemoryBarrier(void)

#if defined(__x86_64__) || defined(__i386__)
 /* On x86, Support old GCC with either no or buggy (GCC BZ#81316) __atomic_* support */
-#define __WINE_ATOMIC_LOAD_ACQUIRE(ptr, ret) do { *(ret) = *(ptr); __asm__ __volatile__( "" ::: "memory" ); } while (0)
-#define __WINE_ATOMIC_LOAD_RELAXED(ptr, ret) do { *(ret) = *(ptr); } while (0)
-#define __WINE_ATOMIC_STORE_RELEASE(ptr, val) do { __asm__ __volatile__( "" ::: "memory" ); *(ptr) = *(val); } while (0)
-#define __WINE_ATOMIC_STORE_RELAXED(ptr, val) do { *(ptr) = *(val); } while (0)
+#define __WINE_ATOMIC_LOAD_ACQUIRE(ptr, ret) do { C_ASSERT(sizeof(*(ptr)) <= sizeof(void *)); *(ret) = *(ptr); __asm__ __volatile__( "" ::: "memory" ); } while (0)
+#define __WINE_ATOMIC_LOAD_RELAXED(ptr, ret) do { C_ASSERT(sizeof(*(ptr)) <= sizeof(void *)); *(ret) = *(ptr); } while (0)
+#define __WINE_ATOMIC_STORE_RELEASE(ptr, val) do { C_ASSERT(sizeof(*(ptr)) <= sizeof(void *)); __asm__ __volatile__( "" ::: "memory" ); *(ptr) = *(val); } while (0)
+#define __WINE_ATOMIC_STORE_RELAXED(ptr, val) do { C_ASSERT(sizeof(*(ptr)) <= sizeof(void *)); *(ptr) = *(val); } while (0)
 #else
 #define __WINE_ATOMIC_LOAD_ACQUIRE(ptr, ret) __atomic_load(ptr, ret, __ATOMIC_ACQUIRE)
 #define __WINE_ATOMIC_LOAD_RELAXED(ptr, ret) __atomic_load(ptr, ret, __ATOMIC_RELAXED)

[Jinoh Kang (＠iamahuman)]

v5: Fix more incorrect 64-bit atomic ops, reword commit messages a bit