Skip to content

Metal: report unsupported atomics when lowering them - #943

Open
maleadt wants to merge 1 commit into
tb/atomicsfrom
tb/atomics-unsupported
Open

maleadt wants to merge 1 commit into
tb/atomicsfrom
tb/atomics-unsupported

Conversation

@maleadt

@maleadt maleadt commented Sep 24, 2026 •

Copy link
Copy Markdown
Member

validate_ir rejects the atomics that Metal can't express. When validation is disabled, e.g. by code_native and the other reflection functions, lower_atomics! used to select them anyway, to AIR intrinsics that don't exist (such as air.atomic.global.store.i64). Now it leaves unsupported operations alone and reports them after the lowering as an InvalidIRError, with the same reasons and backtraces as validate_ir.

Compilations that validate are unaffected. The only case that changes is reflection on a kernel that couldn't be compiled anyway: it now shows why, instead of printing an intrinsic that doesn't exist. One of our own tests depended on that, with a 64-bit atomic store in the fence lowering test; it now uses a 32-bit store.

@maleadt
maleadt added this pull request to stack #944 September 24, 2026 15:38
@maleadt
maleadt force-pushed the tb/atomics-unsupported branch from 6146418 to dfd9510 Compare September 24, 2026 16:15
@maleadt
maleadt removed this pull request from stack #944 September 25, 2026 06:14
@maleadt
maleadt force-pushed the tb/atomics-unsupported branch from dfd9510 to 0d0e759 Compare September 25, 2026 06:14
@maleadt
maleadt added this pull request to stack #951 September 25, 2026 06:14
@maleadt
maleadt force-pushed the tb/atomics-unsupported branch from 0d0e759 to 75dd53c Compare September 25, 2026 06:55
@codecov

codecov Bot commented Sep 25, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 87.15%. Comparing base (082d689) to head (e6cbee3).

Files with missing lines Patch % Lines
src/metal.jl 90.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           tb/atomics     #943   +/-   ##
===========================================
  Coverage       87.15%   87.15%           
===========================================
  Files              30       30           
  Lines            6277     6285    +8     
===========================================
+ Hits             5471     5478    +7     
- Misses            806      807    +1     

☔ View full report in Codecov by Harness.
📢 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.

`validate_ir` rejects the atomics that Metal cannot express, but when
validation is disabled (e.g., by `code_native` and the other reflection
macros), `lower_atomics!` selected them anyway, to AIR intrinsics that
don't exist (such as `air.atomic.global.store.i64`). Leave unsupported
operations alone instead, and report them after the lowering with the
same reasons and backtraces as `validate_ir`.

The fence lowering test relied on this, with a 64-bit atomic store.
@maleadt
maleadt force-pushed the tb/atomics-unsupported branch from 75dd53c to e6cbee3 Compare September 25, 2026 11:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant