Add atomic float support

[michel2323]

JuliaGPU/oneAPI.jl#544 Adding the SPV_EXT_shader_atomic_float_add extension in oneAPI.jl seems to work.

[github-actions]

Your PR requires formatting changes to meet the project's style guidelines.
Please consider running Runic (git runic master) to apply these changes.

Click here to view the suggested changes.

diff --git a/lib/intrinsics/src/atomic.jl b/lib/intrinsics/src/atomic.jl
index a93f926..d627741 100644
--- a/lib/intrinsics/src/atomic.jl
+++ b/lib/intrinsics/src/atomic.jl
@@ -47,17 +47,21 @@ for gentype in atomic_types, as in atomic_memory_types
 @device_function atomic_xor!(p::LLVMPtr{$gentype,$as}, val::$gentype) =
     @builtin_ccall("atomic_xor", $gentype,
                    (LLVMPtr{$gentype,$as}, $gentype), p, val)
-end
-if gentype in atomic_integer_types
-    @eval begin
-    @device_function atomic_xchg!(p::LLVMPtr{$gentype,$as}, val::$gentype) =
-        @builtin_ccall("atomic_xchg", $gentype,
-                    (LLVMPtr{$gentype,$as}, $gentype), p, val)
-
-    @device_function atomic_cmpxchg!(p::LLVMPtr{$gentype,$as}, cmp::$gentype, val::$gentype) =
-        @builtin_ccall("atomic_cmpxchg", $gentype,
-                    (LLVMPtr{$gentype,$as}, $gentype, $gentype), p, cmp, val)
     end
+    if gentype in atomic_integer_types
+        @eval begin
+            @device_function atomic_xchg!(p::LLVMPtr{$gentype, $as}, val::$gentype) =
+                @builtin_ccall(
+                "atomic_xchg", $gentype,
+                (LLVMPtr{$gentype, $as}, $gentype), p, val
+            )
+
+            @device_function atomic_cmpxchg!(p::LLVMPtr{$gentype, $as}, cmp::$gentype, val::$gentype) =
+                @builtin_ccall(
+                "atomic_cmpxchg", $gentype,
+                (LLVMPtr{$gentype, $as}, $gentype, $gentype), p, cmp, val
+            )
+        end
 end
 end
 
diff --git a/test/atomics.jl b/test/atomics.jl
index bcfaa14..339d263 100644
--- a/test/atomics.jl
+++ b/test/atomics.jl
@@ -20,129 +20,129 @@ atomic_operations = [
     (:atomic_cas!, 0, 1),
 ]
 @testset "atomics" begin
-for (op, init_val, expected_val) in atomic_operations
-    for T in all_types
-        # Skip Int64/UInt64 if not supported
-        if sizeof(T) == 8 && T <: Integer && !("cl_khr_int64_extended_atomics" in dev.extensions)
-            continue
-        end
+    for (op, init_val, expected_val) in atomic_operations
+        for T in all_types
+            # Skip Int64/UInt64 if not supported
+            if sizeof(T) == 8 && T <: Integer && !("cl_khr_int64_extended_atomics" in dev.extensions)
+                continue
+            end
 
-        # Skip Float64 if not supported
-        if T == Float64 && !("cl_khr_fp64" in dev.extensions)
-            continue
-        end
+            # Skip Float64 if not supported
+            if T == Float64 && !("cl_khr_fp64" in dev.extensions)
+                continue
+            end
 
-        # Bitwise operations (only valid for integers)
-        if op in [:atomic_and!, :atomic_or!, :atomic_xor!] && T <: AbstractFloat
-            continue
-        end
+            # Bitwise operations (only valid for integers)
+            if op in [:atomic_and!, :atomic_or!, :atomic_xor!] && T <: AbstractFloat
+                continue
+            end
 
-        # Min/max operations (only supported for 32-bit integers in OpenCL)
-        if op in [:atomic_min!, :atomic_max!] && !(T in [Int32, UInt32])
-            continue
-        end
+            # Min/max operations (only supported for 32-bit integers in OpenCL)
+            if op in [:atomic_min!, :atomic_max!] && !(T in [Int32, UInt32])
+                continue
+            end
 
-        test_name = Symbol("test_", op, "_", T)
+            test_name = Symbol("test_", op, "_", T)
 
-        if op in [:atomic_add!, :atomic_sub!]
-            # Arithmetic operations
-            if op == :atomic_add!
-                @eval function $test_name(counter)
-                    OpenCL.@atomic counter[] += one($T)
-                    return
+            if op in [:atomic_add!, :atomic_sub!]
+                # Arithmetic operations
+                if op == :atomic_add!
+                    @eval function $test_name(counter)
+                        OpenCL.@atomic counter[] += one($T)
+                        return
+                    end
+                else
+                    @eval function $test_name(counter)
+                        OpenCL.@atomic counter[] -= one($T)
+                        return
+                    end
                 end
-            else
-                @eval function $test_name(counter)
-                    OpenCL.@atomic counter[] -= one($T)
-                    return
+            elseif op in [:atomic_and!, :atomic_or!, :atomic_xor!]
+                # Bitwise operations
+                if op == :atomic_and!
+                    @eval function $test_name(counter)
+                        OpenCL.@atomic counter[] &= one($T)
+                        return
+                    end
+                elseif op == :atomic_or!
+                    @eval function $test_name(counter)
+                        OpenCL.@atomic counter[] |= one($T)
+                        return
+                    end
+                else # xor
+                    @eval function $test_name(counter)
+                        OpenCL.@atomic counter[] ⊻= one($T)
+                        return
+                    end
                 end
-            end
-        elseif op in [:atomic_and!, :atomic_or!, :atomic_xor!]
-            # Bitwise operations
-            if op == :atomic_and!
-                @eval function $test_name(counter)
-                    OpenCL.@atomic counter[] &= one($T)
-                    return
-                end
-            elseif op == :atomic_or!
-                @eval function $test_name(counter)
-                    OpenCL.@atomic counter[] |= one($T)
-                    return
+            elseif op in [:atomic_max!, :atomic_min!]
+                # Min/max operations - use low-level API directly
+                if op == :atomic_max!
+                    @eval function $test_name(counter)
+                        ptr = OpenCL.pointer(counter, 1)
+                        OpenCL.atomic_max!(ptr, one($T))
+                        return
+                    end
+                else
+                    @eval function $test_name(counter)
+                        ptr = OpenCL.pointer(counter, 1)
+                        OpenCL.atomic_min!(ptr, one($T))
+                        return
+                    end
                 end
-            else # xor
-                @eval function $test_name(counter)
-                    OpenCL.@atomic counter[] ⊻= one($T)
-                    return
-                end
-            end
-        elseif op in [:atomic_max!, :atomic_min!]
-            # Min/max operations - use low-level API directly
-            if op == :atomic_max!
+            elseif op == :atomic_xchg!
+                # Exchange operation - use low-level API directly
                 @eval function $test_name(counter)
                     ptr = OpenCL.pointer(counter, 1)
-                    OpenCL.atomic_max!(ptr, one($T))
+                    OpenCL.atomic_xchg!(ptr, one($T))
                     return
                 end
-            else
+            elseif op == :atomic_cas!
+                # CAS operation - use low-level API directly (it's called atomic_cmpxchg!)
                 @eval function $test_name(counter)
                     ptr = OpenCL.pointer(counter, 1)
-                    OpenCL.atomic_min!(ptr, one($T))
+                    OpenCL.atomic_cmpxchg!(ptr, $T(0), one($T))
                     return
                 end
+            else
+                error("Unknown operation: $op")
             end
-        elseif op == :atomic_xchg!
-            # Exchange operation - use low-level API directly
-            @eval function $test_name(counter)
-                ptr = OpenCL.pointer(counter, 1)
-                OpenCL.atomic_xchg!(ptr, one($T))
-                return
-            end
-        elseif op == :atomic_cas!
-            # CAS operation - use low-level API directly (it's called atomic_cmpxchg!)
-            @eval function $test_name(counter)
-                ptr = OpenCL.pointer(counter, 1)
-                OpenCL.atomic_cmpxchg!(ptr, $T(0), one($T))
-                return
-            end
-        else
-            error("Unknown operation: $op")
-        end
 
 
-        # Try to compile the kernel - this is the key test
+            # Try to compile the kernel - this is the key test
         a = OpenCL.zeros(T)
-        OpenCL.fill!(a, init_val)
-        kernel_func = @eval $test_name
-        OpenCL.@opencl global_size=1000 kernel_func(a)
-        result_val = Array(a)[1]
-        @test typeof(result_val) == T
-        @test result_val == T(expected_val)
+            OpenCL.fill!(a, init_val)
+            kernel_func = @eval $test_name
+            OpenCL.@opencl global_size = 1000 kernel_func(a)
+            result_val = Array(a)[1]
+            @test typeof(result_val) == T
+            @test result_val == T(expected_val)
     end
 end
 
 
-@testset "atomic_add! ($T)" for T in [Float32, Float64]
-    # Float64 requires cl_khr_fp64 extension
-    if T == Float64 && !("cl_khr_fp64" in cl.device().extensions)
-        continue
-    end
+    @testset "atomic_add! ($T)" for T in [Float32, Float64]
+        # Float64 requires cl_khr_fp64 extension
+        if T == Float64 && !("cl_khr_fp64" in cl.device().extensions)
+            continue
+        end
     if "cl_ext_float_atomics" in cl.device().extensions
-        @eval function atomic_float_add(counter, val::$T)
+            @eval function atomic_float_add(counter, val::$T)
             @builtin_ccall(
-                "atomic_add", $T,
-                (LLVMPtr{$T, AS.CrossWorkgroup}, $T),
+                    "atomic_add", $T,
+                    (LLVMPtr{$T, AS.CrossWorkgroup}, $T),
                 pointer(counter), val,
             )
             return
         end
 
         @testset "SPV_EXT_shader_atomic_float_add extension" begin
-            a = OpenCL.zeros(T)
-            @opencl global_size = 1000 extensions = ["SPV_EXT_shader_atomic_float_add"] atomic_float_add(a, one(T))
-            @test OpenCL.@allowscalar a[] == T(1000.0)
+                a = OpenCL.zeros(T)
+                @opencl global_size = 1000 extensions = ["SPV_EXT_shader_atomic_float_add"] atomic_float_add(a, one(T))
+                @test OpenCL.@allowscalar a[] == T(1000.0)
 
             spv = sprint() do io
-                OpenCL.code_native(io, atomic_float_add, Tuple{CLDeviceArray{T, 0, 1}, T}; extensions = ["SPV_EXT_shader_atomic_float_add"])
+                    OpenCL.code_native(io, atomic_float_add, Tuple{CLDeviceArray{T, 0, 1}, T}; extensions = ["SPV_EXT_shader_atomic_float_add"])
             end
             @test occursin("OpExtension \"SPV_EXT_shader_atomic_float_add\"", spv)
             @test occursin("OpAtomicFAddEXT", spv)

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.68%. Comparing base (71cf159) to head (58f3b27).

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #399      +/-   ##
==========================================
+ Coverage   80.27%   80.68%   +0.41%     
==========================================
  Files          12       12              
  Lines         730      730              
==========================================
+ Hits          586      589       +3     
+ Misses        144      141       -3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[michel2323]

Bump. Can someone take a look? @vchuravy ? I don't see how the failing tests are connected to the changes here.

[maleadt]

Thanks. Can you add a test that works here too?
CI failures look unrelated.

[michel2323]

@maleadt Added tests and rebased.

[simeonschaub]

The test can now be modified to actually call the atomic_add! method from SPIRVIntrinsics instead of defining its own function. It is also not exercising the Float64 path ATM, since Float32 is hard coded inside the test.

Ideally, we would also be testing the other atomic intrinsics that have been defined for Float32 and Float64 now. (Are the bitwise intrinsics like atomic_or! and friends even defined for floats?)

[michel2323]

@simeonschaub I tried to add more tests, but except for atomic_add, they all seem to fail, and I don't understand why. I see something about a fallback in the definition of @atomic. I've added atomic_sub that fails.

Can we remove the @builtin_ccall based tests?

[michel2323]

A restful night resolved the issues. This should pass now. A few comments:

• OpenCL only supports Int32 for min, max. Feels weird.
• Bitwise atomics only for integers

[michel2323]

@simeonschaub @maleadt @vchuravy Bump. I don't think the errors are related to my code changes.