Skip to content

Add 64-bit atomic min/max support#866

Open
tgymnich wants to merge 7 commits into
mainfrom
feature/issue-477-atomics-memory-ordering
Open

Add 64-bit atomic min/max support#866
tgymnich wants to merge 7 commits into
mainfrom
feature/issue-477-atomics-memory-ordering

Conversation

@tgymnich

Copy link
Copy Markdown
Member

Implement device-side / for by wiring Metal AIR i64 atomics, add compilation-time GPU family gating for Apple9+, and cover both supported and unsupported hardware paths in atomics tests.

fixes #477

Assisted by: Copilot

@christiangnrd

christiangnrd commented Jun 26, 2026

Copy link
Copy Markdown
Member

Min and max are supported on Apple8 on macOS. It’s footnote 7 in the first set of footnotes

@codecov

codecov Bot commented Jun 26, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.90909% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 86.66%. Comparing base (b8d46ce) to head (a291e76).

Files with missing lines Patch % Lines
src/compiler/compilation.jl 90.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #866      +/-   ##
==========================================
+ Coverage   86.62%   86.66%   +0.04%     
==========================================
  Files          76       76              
  Lines        5144     5152       +8     
==========================================
+ Hits         4456     4465       +9     
+ Misses        688      687       -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.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Metal Benchmarks

Details
Benchmark suite Current: a291e76 Previous: b8d46ce Ratio
array/accumulate/Float32/1d 525542 ns 515334 ns 1.02
array/accumulate/Float32/dims=1 517187.5 ns 506708 ns 1.02
array/accumulate/Float32/dims=1L 8947958 ns 8950000 ns 1.00
array/accumulate/Float32/dims=2 539000 ns 538958 ns 1.00
array/accumulate/Float32/dims=2L 2894416 ns 2913312.5 ns 0.99
array/accumulate/Int64/1d 930000 ns 931417 ns 1.00
array/accumulate/Int64/dims=1 1110125 ns 1114292 ns 1.00
array/accumulate/Int64/dims=1L 9839542 ns 9781937 ns 1.01
array/accumulate/Int64/dims=2 1447437.5 ns 1454062.5 ns 1.00
array/accumulate/Int64/dims=2L 6947125 ns 6944084 ns 1.00
array/broadcast 334792 ns 333167 ns 1.00
array/construct 3209 ns 3333 ns 0.96
array/permutedims/2d 619417 ns 619917 ns 1.00
array/permutedims/3d 1082625 ns 1099417 ns 0.98
array/permutedims/4d 1241875 ns 1231833 ns 1.01
array/private/copy 379500 ns 393104 ns 0.97
array/private/copyto!/cpu_to_gpu 366666 ns 368604.5 ns 0.99
array/private/copyto!/gpu_to_cpu 361145.5 ns 357542 ns 1.01
array/private/copyto!/gpu_to_gpu 324854.5 ns 325791 ns 1.00
array/private/iteration/findall/bool 1191583 ns 1192542 ns 1.00
array/private/iteration/findall/int 1322125 ns 1323145.5 ns 1.00
array/private/iteration/findfirst/bool 1325000 ns 1315083 ns 1.01
array/private/iteration/findfirst/int 1385708 ns 1375917 ns 1.01
array/private/iteration/findmin/1d 1517458 ns 1520792 ns 1.00
array/private/iteration/findmin/2d 1229062.5 ns 1229812.5 ns 1.00
array/private/iteration/logical 1984312.5 ns 1990958 ns 1.00
array/private/iteration/scalar 1731875 ns 1747333 ns 0.99
array/random/rand/Float32 598375 ns 603209 ns 0.99
array/random/rand/Int64 644084 ns 641812.5 ns 1.00
array/random/rand!/Float32 497333 ns 498917 ns 1.00
array/random/rand!/Int64 475458 ns 466875 ns 1.02
array/random/randn/Float32 557500 ns 561042 ns 0.99
array/random/randn!/Float32 452167 ns 454937.5 ns 0.99
array/reductions/mapreduce/Float32/1d 615208 ns 620042 ns 0.99
array/reductions/mapreduce/Float32/dims=1 447333.5 ns 442167 ns 1.01
array/reductions/mapreduce/Float32/dims=1L 699959 ns 701292 ns 1.00
array/reductions/mapreduce/Float32/dims=2 447354.5 ns 449542 ns 1.00
array/reductions/mapreduce/Float32/dims=2L 1001375 ns 1001125 ns 1.00
array/reductions/mapreduce/Int64/1d 798292 ns 808375 ns 0.99
array/reductions/mapreduce/Int64/dims=1 749541.5 ns 758375 ns 0.99
array/reductions/mapreduce/Int64/dims=1L 1100250 ns 1102250 ns 1.00
array/reductions/mapreduce/Int64/dims=2 765625 ns 812583 ns 0.94
array/reductions/mapreduce/Int64/dims=2L 2214167 ns 2213417 ns 1.00
array/reductions/reduce/Float32/1d 606354 ns 610625 ns 0.99
array/reductions/reduce/Float32/dims=1 448625 ns 446333.5 ns 1.01
array/reductions/reduce/Float32/dims=1L 702709 ns 702500 ns 1.00
array/reductions/reduce/Float32/dims=2 355416 ns 356917 ns 1.00
array/reductions/reduce/Float32/dims=2L 494666 ns 497708 ns 0.99
array/reductions/reduce/Int64/1d 802709 ns 798708 ns 1.01
array/reductions/reduce/Int64/dims=1 740208 ns 760000 ns 0.97
array/reductions/reduce/Int64/dims=1L 1097542 ns 1097542 ns 1
array/reductions/reduce/Int64/dims=2 298042 ns 293833.5 ns 1.01
array/reductions/reduce/Int64/dims=2L 685459 ns 689500 ns 0.99
array/shared/copy 159958 ns 158167 ns 1.01
array/shared/copyto!/cpu_to_gpu 38875 ns 39584 ns 0.98
array/shared/copyto!/gpu_to_cpu 40375 ns 39917 ns 1.01
array/shared/copyto!/gpu_to_gpu 40916 ns 40292 ns 1.02
array/shared/iteration/findall/bool 1199125 ns 1199792 ns 1.00
array/shared/iteration/findall/int 1328333 ns 1330625 ns 1.00
array/shared/iteration/findfirst/bool 1008959 ns 994708 ns 1.01
array/shared/iteration/findfirst/int 1000000 ns 1020625 ns 0.98
array/shared/iteration/findmin/1d 1283750 ns 1281750 ns 1.00
array/shared/iteration/findmin/2d 1231875 ns 1231125 ns 1.00
array/shared/iteration/logical 1793208 ns 1796688 ns 1.00
array/shared/iteration/scalar 3468.75 ns 3437.5 ns 1.01
array/sorting/1d 1987562.5 ns 1993833.5 ns 1.00
array/sorting/2d 8453250 ns 8450062.5 ns 1.00
integration/byval/reference 1150709 ns 1150062.5 ns 1.00
integration/byval/slices=1 1152167 ns 1151750 ns 1.00
integration/byval/slices=2 2080792 ns 2079167 ns 1.00
integration/byval/slices=3 18378292 ns 18009271 ns 1.02
integration/metaldevrt 438083.5 ns 439250 ns 1.00
kernel/indexing 329958 ns 331500 ns 1.00
kernel/indexing_checked 487583 ns 492500 ns 0.99
kernel/launch 1987.5 ns 1954.1 ns 1.02
kernel/rand 489750 ns 497209 ns 0.98
latency/import 1809613541 ns 1812323292 ns 1.00
latency/precompile 40243004874.5 ns 39925435375 ns 1.01
latency/ttfp 2147904042 ns 2150481396 ns 1.00
metal/synchronization/context 518.5647668393782 ns 534.3264248704663 ns 0.97
metal/synchronization/stream 276.37987012987014 ns 274.8666666666667 ns 1.01

This comment was automatically generated by workflow using github-action-benchmark.

Comment thread src/compiler/compilation.jl Outdated
haskey(functions(mod), "air.atomic.global.min.u.i64") ||
haskey(functions(mod), "air.atomic.global.max.u.i64")
if has_i64_atomic_modify &&
!MTL.supports_family(device(), MTL.MTLGPUFamilyApple9)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checking device() is wrong here, because it disallows even generating code (e.g. for tests, or debugging). Normally we check job.config.target as per above, but if we want a different compatibility axis we could encode those things in the job.config.params (which are controller by Metal.jl).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added gpufamily to params

@tgymnich

Copy link
Copy Markdown
Member Author

Min and max are supported on Apple8 on macOS. It’s footnote 7 in the first set of footnotes

@christiangnrd good catch!

@tgymnich tgymnich force-pushed the feature/issue-477-atomics-memory-ordering branch 5 times, most recently from 2bfa509 to c15493b Compare June 27, 2026 13:31
Comment thread src/compiler/compilation.jl Outdated
Comment thread src/compiler/compilation.jl Outdated
tgymnich and others added 4 commits June 30, 2026 14:07
Implement device-side / for  by wiring Metal AIR i64 atomics, add compilation-time GPU family gating for Apple9+, and cover both supported and unsupported hardware paths in atomics tests.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Christian Guinard <28689358+christiangnrd@users.noreply.github.com>
@christiangnrd christiangnrd force-pushed the feature/issue-477-atomics-memory-ordering branch 2 times, most recently from ba1938b to 50cc657 Compare June 30, 2026 17:26
@christiangnrd

Copy link
Copy Markdown
Member

The only question for me that remains is if the ::Union{Nothing, MTL.MTLGPUFamily} field affects performance and if we really need to guard against a GPUFamily query returning nothing

Co-Authored-By: Christian Guinard <28689358+christiangnrd@users.noreply.github.com>
@christiangnrd christiangnrd force-pushed the feature/issue-477-atomics-memory-ordering branch from 50cc657 to a86918b Compare June 30, 2026 17:30
@tgymnich

tgymnich commented Jun 30, 2026

Copy link
Copy Markdown
Member Author

The only question for me that remains is if the ::Union{Nothing, MTL.MTLGPUFamily} field affects performance and if we really need to guard against a GPUFamily query returning nothing

@christiangnrd nothing is for virtualized metal devices. Falling back to Apple1 does not make any sense here. We could maybe introduce a fake Apple0, AppleVirtual, or NONE family.

@christiangnrd

Copy link
Copy Markdown
Member

@christiangnrd nothing is for virtualized metal devices.

Don't they report Apple5?

@tgymnich

Copy link
Copy Markdown
Member Author

maybe newer ones do. the ones in GitHub action report nothing.

@christiangnrd

Copy link
Copy Markdown
Member

Maybe it's a bug in how you're checking? The Metal.versioninfo printout in the CI runs does show Apple5

@tgymnich

tgymnich commented Jul 4, 2026

Copy link
Copy Markdown
Member Author

@christiangnrd The problem seems to be with the intel runners

@christiangnrd

christiangnrd commented Jul 4, 2026

Copy link
Copy Markdown
Member

I guess we can keep the nothing, but I've disabled the precompilation workload for non-aarch64 platforms since it's a waste to precompile on a platform we know doesn't work

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.

Add Intrinsics for 64 Bit Atomics

3 participants