-
Notifications
You must be signed in to change notification settings - Fork 43
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add atomic intrinsics #308
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is complex enough that having it in one place seems sensible.
1.7 failed on the atomics test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not really familiar with atomics and their APIs, but superficially this looks OK (minor some small changes).
CI sees to fail with assertions enabled though.
@maleadt I'm fixing bugs locally with LLVM assertion enabled. I'll ping you once I fix all bugs locally. |
@maleadt OK, I think it's ready. Can you run the CI? |
This patch introduces
LLVM.Interop.atomic_pointer*
onLLVMPtr
mimickingCore.Intrinsics.atomic_pointer*
API onPtr
. This is a superset of JuliaGPU/CUDA.jl#1393. It extracts the "CUDA-unaware" part of the atomics previously partially implemented in CUDA.jl. My motivation for this is to implement GPU-interop of https://github.com/JuliaConcurrent/UnsafeAtomics.jl and https://github.com/JuliaConcurrent/Atomix.jl without explicitly dispatching on CUDA.jl and AMDGPU.jl types.As discussed in JuliaGPU/CUDA.jl#1393 the native code from this is not particularly good on CUDA. But that's probably fixable in LLVM (a long-term solution). As a short-term solution, I think CUDA.jl and AMDGPU.jl can use
@device_override
to override these intrinsics in their device context (e.g., use@llvm.nvvm.*
intrinsics in CUDA).cc @vchuravy @jpsamaroo