[PATCH v4 0/2] MR10463: winemac: Replace OSAtomic functions with builtin __atomic counterparts.
This fixes deprecated build warnings: `wine/dlls/winemac.drv/cocoa_event.m:280:21: warning: 'OSAtomicDecrement32Barrier' is deprecated: first deprecated in macOS 10.12` Also relaxes memory barriers where appropriate. -- v4: winemac: Relax builtin __atomic memory barriers. winemac: Replace OSAtomic functions with builtin __atomic counterparts. https://gitlab.winehq.org/wine/wine/-/merge_requests/10463
From: Marc-Aurel Zent <mzent@codeweavers.com> Also relaxes memory order when appropiate. --- dlls/winemac.drv/cocoa_event.m | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/dlls/winemac.drv/cocoa_event.m b/dlls/winemac.drv/cocoa_event.m index 7c86fc1d5ff..d3fc4e8f22c 100644 --- a/dlls/winemac.drv/cocoa_event.m +++ b/dlls/winemac.drv/cocoa_event.m @@ -21,7 +21,6 @@ #include <sys/types.h> #include <sys/event.h> #include <sys/time.h> -#include <libkern/OSAtomic.h> #include "macdrv_cocoa.h" #import "cocoa_event.h" @@ -277,7 +276,7 @@ - (MacDrvEvent*) getEventMatchingMask:(macdrv_event_mask)mask [events removeObjectAtIndex:index]; if (event->event->deliver == INT_MAX || - OSAtomicDecrement32Barrier(&event->event->deliver) >= 0) + __atomic_sub_fetch(&event->event->deliver, 1, __ATOMIC_RELAXED) >= 0) { ret = event; break; @@ -647,7 +646,7 @@ int macdrv_copy_event_from_queue(macdrv_event_queue queue, */ macdrv_event* macdrv_retain_event(macdrv_event *event) { - OSAtomicIncrement32Barrier(&event->refs); + __atomic_add_fetch(&event->refs, 1, __ATOMIC_RELAXED); return event; } @@ -662,8 +661,9 @@ void macdrv_release_event(macdrv_event *event) { @autoreleasepool { - if (OSAtomicDecrement32Barrier(&event->refs) <= 0) + if (__atomic_sub_fetch(&event->refs, 1, __ATOMIC_RELEASE) <= 0) { + __atomic_thread_fence(__ATOMIC_ACQUIRE); switch (event->type) { case IM_SET_TEXT: @@ -706,7 +706,7 @@ void macdrv_release_event(macdrv_event *event) */ macdrv_query* macdrv_retain_query(macdrv_query *query) { - OSAtomicIncrement32Barrier(&query->refs); + __atomic_add_fetch(&query->refs, 1, __ATOMIC_RELAXED); return query; } @@ -715,8 +715,9 @@ void macdrv_release_event(macdrv_event *event) */ void macdrv_release_query(macdrv_query *query) { - if (OSAtomicDecrement32Barrier(&query->refs) <= 0) + if (__atomic_sub_fetch(&query->refs, 1, __ATOMIC_RELEASE) <= 0) { + __atomic_thread_fence(__ATOMIC_ACQUIRE); switch (query->type) { case QUERY_DRAG_DROP_ENTER: -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10463
From: Marc-Aurel Zent <mzent@codeweavers.com> --- dlls/winemac.drv/cocoa_event.m | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/dlls/winemac.drv/cocoa_event.m b/dlls/winemac.drv/cocoa_event.m index d3fc4e8f22c..d01df4da3d8 100644 --- a/dlls/winemac.drv/cocoa_event.m +++ b/dlls/winemac.drv/cocoa_event.m @@ -276,7 +276,7 @@ - (MacDrvEvent*) getEventMatchingMask:(macdrv_event_mask)mask [events removeObjectAtIndex:index]; if (event->event->deliver == INT_MAX || - __atomic_sub_fetch(&event->event->deliver, 1, __ATOMIC_RELAXED) >= 0) + __atomic_sub_fetch(&event->event->deliver, 1, __ATOMIC_SEQ_CST) >= 0) { ret = event; break; @@ -646,7 +646,7 @@ int macdrv_copy_event_from_queue(macdrv_event_queue queue, */ macdrv_event* macdrv_retain_event(macdrv_event *event) { - __atomic_add_fetch(&event->refs, 1, __ATOMIC_RELAXED); + __atomic_add_fetch(&event->refs, 1, __ATOMIC_SEQ_CST); return event; } @@ -661,9 +661,8 @@ void macdrv_release_event(macdrv_event *event) { @autoreleasepool { - if (__atomic_sub_fetch(&event->refs, 1, __ATOMIC_RELEASE) <= 0) + if (__atomic_sub_fetch(&event->refs, 1, __ATOMIC_SEQ_CST) <= 0) { - __atomic_thread_fence(__ATOMIC_ACQUIRE); switch (event->type) { case IM_SET_TEXT: @@ -706,7 +705,7 @@ void macdrv_release_event(macdrv_event *event) */ macdrv_query* macdrv_retain_query(macdrv_query *query) { - __atomic_add_fetch(&query->refs, 1, __ATOMIC_RELAXED); + __atomic_add_fetch(&query->refs, 1, __ATOMIC_SEQ_CST); return query; } @@ -715,9 +714,8 @@ void macdrv_release_event(macdrv_event *event) */ void macdrv_release_query(macdrv_query *query) { - if (__atomic_sub_fetch(&query->refs, 1, __ATOMIC_RELEASE) <= 0) + if (__atomic_sub_fetch(&query->refs, 1, __ATOMIC_SEQ_CST) <= 0) { - __atomic_thread_fence(__ATOMIC_ACQUIRE); switch (query->type) { case QUERY_DRAG_DROP_ENTER: -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10463
On Sat Mar 28 09:18:32 2026 +0000, Marc-Aurel Zent wrote:
changed this line in [version 4 of the diff](/wine/wine/-/merge_requests/10463/diffs?diff_id=255276&start_sha=be4cc396e4dc3ad3169747722a2e231150ee8ca1#4dded6b0410796e9850e39dac1acc312d3503541_279_279) Ah I see what you mean now... `deliver` is only meaningfully observed in `postEventObject`, and that is after function return, so the visibility of the event is already synchronized by eventsLock.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10463#note_134212
Is there a reason to use builtins rather than the C11 \<stdatomic.h\> functions?
Yeah I chose builtins for a few reasons: * I didn't know what the policy of using C11 in general in wine is, or `<stdatomic.h>`. * It is also not used in wines codebase at all, whereas __atomic builtins are used commonly * C11 atomics are implemented on top of these builtins (and any version of gcc/clang that supports C11, also supports them, so nothing compatibility wise is gained here) * They also produce identical machine code * C11 requires `_Atomic` types as well, so it would be a more invasive change Apart from Apple's inline helpers using the C11 version, the only other reason I can think of using them is: * Works across outside the clang/gcc ecosystem (which IMO is a moot point here) * Maybe looks prettier * Better compile time type checking for atomic access etc via `_Atomic`
Also I think it would be best to transition to new functions and then relax barriers in separate commits.
Yeah makes sense, the latest version does so now. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10463#note_134213
participants (2)
-
Marc-Aurel Zent -
Marc-Aurel Zent (＠mzent)