-
Notifications
You must be signed in to change notification settings - Fork 41
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_rmw!
with syncscope
#431
Conversation
76ac5f6
to
597fb37
Compare
You need to manually re-generate the wrappers by running |
Done! |
bb6907f
to
192e8a6
Compare
@maleadt, kind ping for review. |
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.
LGTM, but needs tests.
@maleadt, I've updated tests for LLVM < 16. Hopefully now they will pass :) |
LGTM. CI failure is EnzymeAD/Enzyme.jl#1688 |
Thanks! Can we tag a new release so that further packages can use it? |
First needs a JLL bump. I'll create a PR. |
…104775) Another upstreaming of C API extensions we have in Julia/LLVM.jl. Although [we went](maleadt/LLVM.jl#431) with a string-based API there, here I'm proposing something that's similar to existing metadata/attribute APIs: - explicit functions to map syncscope names to IDs, and back - `LLVM*SyncScope` versions of builder APIs that already take a `SingleThread` argument: atomic rmw, atomic xchg, fence - `LLVMGetAtomicSyncScopeID` and `LLVMSetAtomicSyncScopeID` for other atomic instructions - testing through `llvm-c-test`'s `--echo` functionality
…lvm#104775) Another upstreaming of C API extensions we have in Julia/LLVM.jl. Although [we went](maleadt/LLVM.jl#431) with a string-based API there, here I'm proposing something that's similar to existing metadata/attribute APIs: - explicit functions to map syncscope names to IDs, and back - `LLVM*SyncScope` versions of builder APIs that already take a `SingleThread` argument: atomic rmw, atomic xchg, fence - `LLVMGetAtomicSyncScopeID` and `LLVMSetAtomicSyncScopeID` for other atomic instructions - testing through `llvm-c-test`'s `--echo` functionality
Add ability to specify syncscope for
atomic_rmw!
op.This is needed to properly enable hardware floating-point atomics for AMDGPU (Ref: JuliaGPU/AMDGPU.jl#569) to avoid CAS loop.
Basically, expose
syncscope
argument here:https://github.com/llvm/llvm-project/blob/8ebe499e07760b0d18b5721b298efc9e4a241916/llvm/lib/IR/Core.cpp#L4299-L4308
Before:
%15 = atomicrmw fadd float addrspace(1)* %14, float 1.000000e+00 seq_cst, align 4, !dbg !148
global_atomic_cmpswap_b32 v2, v[0:1], v[2:3], off offset:-4 glc
With syncscope
"agent"
:%15 = atomicrmw fadd float addrspace(1)* %14, float 1.000000e+00 syncscope("agent") seq_cst, align 4, !dbg !148
global_atomic_add_f32 v[0:1], v2, off offset:-4
@maleadt, I suppose wrappers in
lib/
will be auto-updated or do I need to do this manually? I did it for a local build.