-
Notifications
You must be signed in to change notification settings - Fork 44
Int64 atomics #347
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
Int64 atomics #347
Conversation
|
Your PR requires formatting changes to meet the project's style guidelines. Click here to view the suggested changes.diff --git a/lib/intrinsics/src/atomic.jl b/lib/intrinsics/src/atomic.jl
index 9bbbdbe..c3027bf 100644
--- a/lib/intrinsics/src/atomic.jl
+++ b/lib/intrinsics/src/atomic.jl
@@ -63,23 +63,35 @@ end
for as in atomic_memory_types
@eval begin
-# There is native support for atomic_xchg on Float32, but not for Float64,
-# so we always reinterpret for consistency.
+ # There is native support for atomic_xchg on Float32, but not for Float64,
+ # so we always reinterpret for consistency.
@device_function atomic_xchg!(p::LLVMPtr{Float32,$as}, val::Float32) =
- reinterpret(Float32, atomic_xchg!(reinterpret(LLVMPtr{UInt32,$as}, p),
- reinterpret(UInt32, val)))
-@device_function atomic_xchg!(p::LLVMPtr{Float64,$as}, val::Float64) =
- reinterpret(Float64, atomic_xchg!(reinterpret(LLVMPtr{UInt64,$as}, p),
- reinterpret(UInt64, val)))
+ reinterpret(
+ Float32, atomic_xchg!(
+ reinterpret(LLVMPtr{UInt32, $as}, p),
+ reinterpret(UInt32, val)
+ )
+ )
+ @device_function atomic_xchg!(p::LLVMPtr{Float64, $as}, val::Float64) =
+ reinterpret(
+ Float64, atomic_xchg!(
+ reinterpret(LLVMPtr{UInt64, $as}, p),
+ reinterpret(UInt64, val)
+ )
+ )
@device_function atomic_cmpxchg!(p::LLVMPtr{Float32,$as}, cmp::Float32, val::Float32) =
reinterpret(Float32, atomic_cmpxchg!(reinterpret(LLVMPtr{UInt32,$as}, p),
reinterpret(UInt32, cmp),
reinterpret(UInt32, val)))
-@device_function atomic_cmpxchg!(p::LLVMPtr{Float64,$as}, cmp::Float64, val::Float64) =
- reinterpret(Float64, atomic_cmpxchg!(reinterpret(LLVMPtr{UInt64,$as}, p),
- reinterpret(UInt64, cmp),
- reinterpret(UInt64, val)))
+ @device_function atomic_cmpxchg!(p::LLVMPtr{Float64, $as}, cmp::Float64, val::Float64) =
+ reinterpret(
+ Float64, atomic_cmpxchg!(
+ reinterpret(LLVMPtr{UInt64, $as}, p),
+ reinterpret(UInt64, cmp),
+ reinterpret(UInt64, val)
+ )
+ )
end
end
@@ -256,7 +268,8 @@ for (op,impl) in [(+) => atomic_add!,
Base.max => atomic_max!,
Base.min => atomic_min!]
@eval @inline atomic_arrayset(A::AbstractArray{T}, I::Integer, ::typeof($op),
- val::T) where {T <: Union{atomic_integer_types...}} =
+ val::T
+ ) where {T <: Union{atomic_integer_types...}} =
$impl(pointer(A, I), val)
end
diff --git a/test/atomics.jl b/test/atomics.jl
index 068e8cd..f1724bf 100644
--- a/test/atomics.jl
+++ b/test/atomics.jl
@@ -1,16 +1,16 @@
@testset "atomics" begin
-function atomic_count(counter)
- OpenCL.@atomic counter[] += 1
- return
-end
+ function atomic_count(counter)
+ OpenCL.@atomic counter[] += 1
+ return
+ end
-@testset "atomic_add! ($T)" for T in [Int32, UInt32, Int64, UInt64]
- if sizeof(T) == 4 || "cl_khr_int64_extended_atomics" in cl.device().extensions
- a = OpenCL.zeros(T)
- @opencl global_size=1000 atomic_count(a)
- @test OpenCL.@allowscalar a[] == 1000
+ @testset "atomic_add! ($T)" for T in [Int32, UInt32, Int64, UInt64]
+ if sizeof(T) == 4 || "cl_khr_int64_extended_atomics" in cl.device().extensions
+ a = OpenCL.zeros(T)
+ @opencl global_size = 1000 atomic_count(a)
+ @test OpenCL.@allowscalar a[] == 1000
+ end
end
-end
end |
|
Ok, it works now! I was just missing a dispatch so it previously was using the fallback CAS implementation for everything. Still, that shouldn't error, just be slow, so something else is going wrong here. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #347 +/- ##
=======================================
Coverage 78.86% 78.86%
=======================================
Files 12 12
Lines 672 672
=======================================
Hits 530 530
Misses 142 142 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Any ideas why this is now failing on Windows? (and Mac?) |
|
Windows is #317, macOS hangs seem related... |
|
Hm, the timeouts seem to happen on #348 too. So maybe unrelated? |
|
@maleadt it looks like you still need to manually approve the workflow run even if you're the one commiting 😅 |
Do you have that code around still? Would be good to have that as an issue; invalid SPIR-V codegen seems bad enough to investigate. |
|
It was simply using the fallback definition here with an Int64 input, so that code should still reproduce it. I can try reducing it to a standalone reproducer tomorrow |
This doesn't work as-is, when running the tests, I get: