[Atomics] Migrate metal to use native float atomics#788
Conversation
OpAtomicFAddEXT lowers to atomic_float, which requires Metal Shading Language 3.0. Compiling with the default language version rejected the shader and then hard-aborted on a nil MTLFunction. When QD_METAL_NATIVE_FLOAT_ATOMICS=1, target SPIRV-Cross and MTLCompileOptions at MSL 3.0, and nil-check before pipeline creation. Add a headless FEM99 regression test for the historical FIXME path.
Drop the QD_METAL_NATIVE_FLOAT_ATOMICS opt-in; advertise float-atomic caps whenever the device supports them.
Apple7+/Mac2+ now advertise spirv_has_atomic_float_add; shared / f16 / f64 / float min-max remain on the CAS path.
Em/en dashes to single hyphen, arrow to ->, superscript 3 to ASCII.
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5d0dab0cbb
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| caps.set(DeviceCapability::spirv_has_atomic_float, 1); | ||
| caps.set(DeviceCapability::spirv_has_atomic_float_add, 1); |
There was a problem hiding this comment.
On Apple7/Mac2 hardware running macOS 11/12 (or iOS before 16), this advertises native float atomics even though get_mtl_library() only selects MTLLanguageVersion3_0 inside the macOS 13/iOS 16 availability check. Float-add kernels are consequently emitted with MSL 3.0 atomic_float syntax and rejected by the older runtime instead of using the previously working CAS fallback; gate these capabilities on the same OS availability condition.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
we are only supporitng mac 14+, https://github.com/Genesis-Embodied-AI/quadrants#installation , so I think this isnt an issue.
…fem99 test docstring at 120 Addresses the "Check doc quality" and "Check line wrapping" CI checks on PR #788. atomics.md (doc quality): - Drop internal source-path references (frontend_ir.cpp, spirv_codegen.cpp, metal_device.mm) and internal build-config details (MTLLanguageVersion3_0 / SPIRV-Cross set_msl_version) from the user-facing notes. - Define "MSL" at first use, replace the undefined "RHI" abbreviation with "Quadrants", and replace "snodes" with "quantized fields". - Move the volatile-load lowering table + suppressed-optimization notes under an "Under the hood" heading, and mark the atomic-visibility-scope section "(advanced)", so the IR/backend-mechanics tables sit in clearly-internal sections. - Remove the redundant duplicate i64/u64 footnote. test_fem99_headless.py (line wrapping): - Reflow the module docstring prose and a comment from ~68-93c to 120c.
…-atomics-pr # Conflicts: # docs/source/user_guide/atomics.md
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 796aa091d6
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| caps.set(DeviceCapability::spirv_has_atomic_float, 1); | ||
| caps.set(DeviceCapability::spirv_has_atomic_float_add, 1); |
There was a problem hiding this comment.
On macOS 11/12 or iOS <16 devices that still report Apple7/Mac2 support, these caps make f32 atomic_add lower to OpAtomicFAddEXT/MSL atomic_float, but get_mtl_library() only sets MTLLanguageVersion3_0 inside @available(macOS 13.0, iOS 16.0, *) and otherwise compiles with the default language version, so those kernels fail instead of using the existing CAS fallback. This regresses installs the project still tags as macosx_11_0_arm64; Apple documents MSL 3.0 as macOS 13+/iOS 16+ at https://developer.apple.com/documentation/metal/mtllanguageversion/version3_0?language=objc.
Useful? React with 👍 / 👎.
1 participant
OpAtomicFAddEXT lowers to atomic_float, which requires Metal Shading Language 3.0. Compiling with the default language version rejected the shader and then hard-aborted on a nil MTLFunction. When QD_METAL_NATIVE_FLOAT_ATOMICS=1, target SPIRV-Cross and MTLCompileOptions at MSL 3.0, and nil-check before pipeline creation. Add a headless FEM99 regression test for the historical FIXME path.
Issue: #
Brief Summary
copilot:summary
Walkthrough
copilot:walkthrough