On Fri, Dec 12, 2025, at 11:03 AM, Nathan Bossart wrote:
> +/*
> + * _InterlockedExchange() generates a full memory barrier (or release
> + * semantics that ensures all prior memory operations are visible to
> + * other cores before the lock is released.
> + */
> +#define S_UNLOCK(lock) (InterlockedExchange(lock, 0))
Nathan, thanks for looking at the patch!
> This seems to change the implementation from
>
> #define S_UNLOCK(lock) \
> do { _ReadWriteBarrier(); (*(lock)) = 0; } while (0)
>
> in some cases, but I am insufficiently caffeinated to figure out what
> platforms use which implementation. In any case, it looks like we are
> changing it for some currently-supported platforms, and I'm curious why.
This change is within _MSC_VER, but AFAICT this intrinsic is available across
their supported platforms. The previous implementation of S_UNLOCK() boils
down to a no-op because the _ReadWriteBarrier()[1] is a hint to the compiler
and does not emit any instruction on any platform and it's also deprecated.
So, on MSVC S_UNLOCK is an unguarded assignment and then a loop that will be
optimized out, not really what we wanted I'd imagine. My tests with godbolt
showed this to be true, no instruction barriers emitted. I think it was
Andres[2] who suggested replacing it with _InterlockedExchange()[3]. So, given
that _ReadWriteBarrier() is deprecated I decided not to specialize this change
to only the ARM64 platform, sorry for not making that clear in the commit or
email.
My buildfarm animal for this platform was just approved and is named "unicorn",
I'm not making that up. :)
best.
-greg
> Perhaps there's some way to make the #ifdefs a bit more readable, too
> (e.g., a prerequisite patch that rearranges things).
>
> The rest looks generally reasonable to me.
>
> --
> nathan
[1] https://learn.microsoft.com/en-us/cpp/intrinsics/readwritebarrier
[2]
https://www.postgresql.org/message-id/beirrgqo5n5e73dwa4dsdnlbtef3bsdv5sgarm6przdzxvifk5%40whyuhyemmhyr
[3]
https://learn.microsoft.com/en-us/cpp/intrinsics/interlockedexchange-intrinsic-functions
