[PATCH v3 0/1] 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. -- v3: 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
On Thu Mar 26 22:17:20 2026 +0000, Jinoh Kang wrote:
If we're relaxing fences, maybe we should use acqrel for decrement too? Acquire release semantics are indeed enough here.
The last version uses only release even, and saves the acquire cost on every non-final decrement. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10463#note_133947
Jinoh Kang (@iamahuman) commented about dlls/winemac.drv/cocoa_event.m:
[events removeObjectAtIndex:index];
if (event->event->deliver == INT_MAX || - OSAtomicDecrement32Barrier(&event->event->deliver) >= 0) + __atomic_sub_fetch(&event->event->deliver, 1, __ATOMIC_RELAXED) >= 0)
Wouldn't the caller expect the return to happen after everything up to the delivery? /genq -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10463#note_133959
On Thu Mar 26 23:43:16 2026 +0000, Jinoh Kang wrote:
Wouldn't the caller expect the return to happen after everything up to the delivery? /genq Maybe I misunderstanding the question, but `deliver` seems to be used for the event to be consumed at most by N queues (and the lock is actually per-queue).
Nonetheless this is still behaving equivalently to the previous version. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10463#note_134005
Is there a reason to use builtins rather than the C11 <stdatomic.h> functions? Maybe you saw it, `<libkern/OSAtomicDeprecated.h>` has inline helpers implementing the old functions using the C11 ones. Also I think it would be best to transition to new functions and then relax barriers in separate commits. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10463#note_134116
On Fri Mar 27 10:54:25 2026 +0000, Marc-Aurel Zent wrote:
Maybe I misunderstanding the question, but `deliver` seems to be used for the event to be consumed at most by N queues (and the lock is actually per-queue). Nonetheless this is still behaving equivalently to the previous version. I'm claiming that ordering is needed so that, after the function returns with the event, it should only observe the state that the event has completely delivered to the N queues. Relaxed ordering doesn't guarantee that.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10463#note_134121
participants (4)
-
Brendan Shanks (@bshanks) -
Jinoh Kang (@iamahuman) -
Marc-Aurel Zent -
Marc-Aurel Zent (@mzent)