Add floating-point atomic add/sub and min/max intrinsics#453
Add floating-point atomic add/sub and min/max intrinsics#453michel2323 wants to merge 6 commits into
Conversation
Float32 atomic_add!/atomic_sub! emit OpenCL-style builtins that lower to the SPV_EXT_shader_atomic_float_add EXT instructions. Float64 atomic add, and float min/max for both types, use cmpxchg fallbacks since the corresponding native extensions aren't available in the LTS set.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #453 +/- ##
==========================================
+ Coverage 80.62% 80.93% +0.31%
==========================================
Files 13 13
Lines 898 918 +20
==========================================
+ Hits 724 743 +19
- Misses 174 175 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
The errors come from #452 |
|
Could we use an approach like in #447 to query whether the device supports SPV_EXT_shader_atomic_float_add and use the fallback cmpxchg loop only when necessary? |
The compare-and-swap loops compared floats with `==`: a stored NaN never compares equal, so the loop spins forever, and a failed exchange is mistaken for a successful one when -0.0 compares equal to 0.0. Compare the raw bits instead.
The OpenCL-style atomic_add/atomic_sub builtins with float arguments are only lowered to the EXT atomic-float instructions by the LLVM SPIR-V backend; the Khronos translator keeps them as external calls, which fail to link. Emit a __spirv_AtomicFAddEXT wrapper builtin instead, like the integer atomics (SPIR-V has no float subtraction, so atomic_sub! adds the negated value).
Native floating-point atomics are an optional device capability (cl_ext_float_atomics), so select between the EXT instructions and a compare-and-swap fallback at compile time, based on what the target device reports: - register fp32/fp64 atomic add and min/max features, detected through the cl_ext_float_atomics capability bitfields (requiring both the global- and local-memory bits, since the intrinsics cover both address spaces) - in SPIRVIntrinsics, provide native (`atomic_add_native!`, ...) and CAS-loop (`atomic_add_fallback!`, ...) versions of the float atomics, defaulting the public functions to the fallback; OpenCL.jl overrides them to pick the native instruction where supported via `has_feature`, which folds at compile time - allow the SPV_EXT_shader_atomic_float_add/_min_max SPIR-V extensions by default on devices that support them, and mask the min/max features on the OpenCL C program backend, where spirv2clc does not implement the corresponding capabilities This makes Float64 add and float min/max use native atomics on devices that support them (previously hardwired to the CAS loop), and gives Float32 add/sub a fallback on devices that don't (previously they required native support unconditionally).
`@atomic` on float arrays went through the generic value-comparing CAS loop; route +, -, min and max through the atomic intrinsics instead, so they use native instructions where the device supports them. Extend the atomics testsuite to cover float min/max.
The package gained new atomic intrinsics that OpenCL.jl relies on.
|
I registered fp32/fp64 atomic_add and atomic_min_max features, detected through the cl_ext_float_atomics capability bitfields (requiring both the global- and local-memory bits, since the intrinsics cover both address spaces). SPIRVIntrinsics now carries a _native! and a _fallback! version of each op, with the public functions defaulting to the compare-and-swap loop and OpenCL.jl overriding them to pick the native instruction via Making the selection device-driven shook out a few issues, split into separate commits:
|
|
Overall, this looks great to me! Pinging @vchuravy regarding what the story for |
| # the loops compare raw bits, not values: `==` on floats would spin forever on a stored NaN, | ||
| # and would treat a failed exchange as successful when -0.0 compares equal to 0.0. |
There was a problem hiding this comment.
Can we not just use === for the comparison instead of ==?
Float32 atomic_add!/atomic_sub! emit OpenCL-style builtins that lower to the SPV_EXT_shader_atomic_float_add EXT instructions. Float64 atomic add, and float min/max for both types, use cmpxchg fallbacks since the corresponding native extensions aren't available in the oneAPI set.