llvm_passes: lower atomicrmw fmin and fmax to cmpxchg loops - #1405
Merged
Merged
Conversation
pvelesko
force-pushed
the
2026-07-29-github-1403-lower-fp-atomic-minmax
branch
from
July 30, 2026 08:31
ba13dcb to
1949d51
Compare
Collaborator
Author
|
/run-aurora-ci |
pvelesko
marked this pull request as draft
July 30, 2026 18:32
pvelesko
marked this pull request as ready for review
August 23, 2026 10:32
__hip_atomic_fetch_min and __hip_atomic_fetch_max on a floating point type lower to atomicrmw fmin and fmax, which llvm-spirv refuses without SPV_EXT_shader_atomic_float_min_max, so the test does not link today.
Clang lowers __hip_atomic_fetch_min and __hip_atomic_fetch_max on a floating point type to atomicrmw fmin and fmax. llvm-spirv can only translate those with SPV_EXT_shader_atomic_float_min_max, which is not in the extension allow list chipStar's driver hands the translator, so the link dies with RequiresExtension: Feature requires the following SPIR-V extension: SPV_EXT_shader_atomic_float_min_max Widening the allow list would only move the failure to program build time on drivers without cl_ext_float_atomics. Expand the operation to a cmpxchg loop instead, which every target chipStar supports can run, and which is what devicelib.cl already does for atomicMin and atomicMax. Fixes #1403
pvelesko
force-pushed
the
2026-07-29-github-1403-lower-fp-atomic-minmax
branch
from
August 24, 2026 07:56
48e7006 to
a517113
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1403
__hip_atomic_fetch_min and __hip_atomic_fetch_max on a floating point type lower to atomicrmw fmin and fmax, which llvm-spirv can only translate with SPV_EXT_shader_atomic_float_min_max. That extension is not in the allow list chipStar's driver hands the translator, so the link dies.
Widening the allow list was the other option, but it would only move the failure to program build time on drivers without cl_ext_float_atomics. The new pass expands the operation to a cmpxchg loop, which every target chipStar supports can run, and which is what devicelib.cl already does for atomicMin and atomicMax.
New test tests/devicelib/TestAtomicFPMinMaxBuiltins.cpp, verified on Intel Arc B570 (OpenCL): does not link before, PASSED after. It covers float and double, a contended reduction, and the returned previous value.
Found while getting the Kokkos HIP backend running on chipStar; desul uses these builtins, which took out Kokkos_ContainersUnitTest_Serial and Kokkos_ContainersUnitTest_HIP.