atomic: Fix memory barriers on MSVC ARM64#15447
Conversation
| #define SDL_MemoryBarrierRelease() __asm__ __volatile__ ("dmb ish" : : : "memory") | ||
| #define SDL_MemoryBarrierAcquire() __asm__ __volatile__ ("dmb ishld" : : : "memory") | ||
| #elif defined(_MSC_VER) && defined(_M_ARM64) | ||
| #include <arm64intr.h> |
There was a problem hiding this comment.
I included arm64intr.h here for the definitions of _ARM64_BARRIER_ISH and _ARM64_BARRIER_ISHLD. Another option would be to hardcode those constants and avoid having to pull in that header.
Anyone have strong opinions either way?
There was a problem hiding this comment.
Including that header seems fine as long as it's available for all the supported build environments.
| #elif defined(__GNUC__) && defined(__aarch64__) | ||
| #define SDL_MemoryBarrierRelease() __asm__ __volatile__ ("dmb ish" : : : "memory") | ||
| #define SDL_MemoryBarrierAcquire() __asm__ __volatile__ ("dmb ishld" : : : "memory") | ||
| #elif defined(_MSC_VER) && defined(_M_ARM64) |
There was a problem hiding this comment.
Would ARM64EC toolchains need this as well?
There was a problem hiding this comment.
I wasn't able to find any specific guidance on memory order guarantees for ARM64EC code, but it seems logical that we should use this for ARM64EC too. I'd imagine there's a DSB executed during transitions from ARM64EC to x64, but I doubt there's anything for ARM64EC code touching memory used by other ARM64EC code.
On that note, our spinlock code also needs an ARM64EC check too.
cgutman
force-pushed
the
msvc_arm64_barriers
branch
from
965d059 to
53aa470
Compare
3 participants
Description
MSVC ARM64 lacked an implementation of
SDL_MemoryBarrierAcquire()andSDL_MemoryBarrierRelease(), so it fell back toSDL_CompilerBarrier()(_ReadWriteBarrier()) which isn't sufficient on ARM64.Existing Issue(s)