refactor discovery, ship therock tarballs as artifacts - #1024
refactor discovery, ship therock tarballs as artifacts#1024simeonschaub wants to merge 4 commits into
Conversation
…ry subpackages By default the ROCm runtime (HIP, HSA) and vendor libraries (rocBLAS, rocSPARSE, rocSOLVER, rocRAND, rocFFT, MIOpen) now come from the new ROCm_Runtime subpackage, which ships AMD's TheRock distribution tarballs directly as lazy artifacts, selected by GPU architecture and ROCm version through platform augmentation. The former src/discovery/ code moves into the ROCm_Runtime_Discovery subpackage, which replicates ROCm_Runtime's API for a local ROCm installation; it is only loaded when that is requested through ROCm_Runtime's "local" preference, settable via the new AMDGPU.set_rocm_version!([version]; local_rocm) (mirroring CUDA.set_runtime_version!). ld.lld now always comes from AMDGPU_LLVM_Backend_jll (it has to match the LLVM version device code is generated with), and the device bitcode libraries keep coming from ROCmDeviceLibs_jll until local ones can be downgraded. libMIOpen_path is renamed to libMIOpen for consistency with the provider packages. Also adds a manually-dispatched workflow to regenerate the Artifacts.toml hashes from repo.amd.com. Assisted-by: Claude Code (claude-fable-5)
Both ROCm_Runtime and ROCm_Runtime_Discovery export the same library path globals, so AMDGPU can simply `using` whichever the "local" preference selects: the imported bindings alias the provider's globals and pick up the values assigned by its `__init__`, which runs before AMDGPU loads. AMDGPU's `libhip`/`libhsaruntime` names are renamed to the providers' `libamdhip64`/`libhsa_runtime64` so no aliasing layer is needed, and `lld_path`/`libdevice_libs` are imported directly from their JLLs, which are assumed to always be available. The discovery package gains libhiprtc and libamd_comgr lookups for API parity with ROCm_Runtime. As a side effect the device libraries are now available while AMDGPU itself precompiles (the JLL's `__init__` runs then), so the warming compile links them for real; the DEVICE_LIBS reset stays to keep absolute artifact paths and bitcode blobs out of the package image. Assisted-by: Claude Code (claude-fable-5)
The subpackages are wired up through [sources], which only the active project's Project.toml provides and which Pkg supports only on Julia 1.11+. Steps running on Julia 1.10 (like the GPU-less environment check) or resolving from another project (docs, perf) therefore failed with "expected package ROCm_Runtime to be registered". Explicitly Pkg.develop'ing the subpackages records path entries in the manifest, which works everywhere. Verified by simulating the CI resolve in a fresh clone on Julia 1.10 and nightly, including a GPU-less load (arch = "none") on 1.10. Assisted-by: Claude Code (claude-fable-5)
There was a problem hiding this comment.
AMDGPU.jl Benchmarks
Details
| Benchmark suite | Current: 0162ceb | Previous: 39fe271 | Ratio |
|---|---|---|---|
amdgpu/synchronization/context/device |
527.5 ns |
560 ns |
0.94 |
amdgpu/synchronization/stream/blocking |
222.5 ns |
230 ns |
0.97 |
amdgpu/synchronization/stream/nonblocking |
297.5 ns |
315 ns |
0.94 |
array/accumulate/Float32/1d |
80553.5 ns |
70043.5 ns |
1.15 |
array/accumulate/Float32/dims=1 |
280381.5 ns |
284709 ns |
0.98 |
array/accumulate/Float32/dims=1L |
95523.75 ns |
80278.75 ns |
1.19 |
array/accumulate/Float32/dims=2 |
92016.25 ns |
72131 ns |
1.28 |
array/accumulate/Float32/dims=2L |
2754104 ns |
2614542.5 ns |
1.05 |
array/accumulate/Int64/1d |
81188.5 ns |
78386.25 ns |
1.04 |
array/accumulate/Int64/dims=1 |
248158.5 ns |
244018.5 ns |
1.02 |
array/accumulate/Int64/dims=1L |
101566.5 ns |
83741.25 ns |
1.21 |
array/accumulate/Int64/dims=2 |
93793.75 ns |
84758.75 ns |
1.11 |
array/accumulate/Int64/dims=2L |
2894465.75 ns |
2891796.75 ns |
1.00 |
array/broadcast |
56058.5 ns |
72998.75 ns |
0.77 |
array/construct |
2517.5 ns |
2182.5 ns |
1.15 |
array/copy |
36475.5 ns |
36628 ns |
1.00 |
array/copyto!/cpu_to_gpu |
111141.5 ns |
110909.25 ns |
1.00 |
array/copyto!/gpu_to_cpu |
111876.75 ns |
110519.25 ns |
1.01 |
array/copyto!/gpu_to_gpu |
59271 ns |
45368.25 ns |
1.31 |
array/iteration/findall/bool |
144777 ns |
132337 ns |
1.09 |
array/iteration/findall/int |
151544.75 ns |
151897.25 ns |
1.00 |
array/iteration/findfirst/bool |
151932 ns |
144392.25 ns |
1.05 |
array/iteration/findfirst/int |
150492 ns |
145337.25 ns |
1.04 |
array/iteration/findmin/1d |
132394.25 ns |
113169 ns |
1.17 |
array/iteration/findmin/2d |
122166.75 ns |
108861.75 ns |
1.12 |
array/iteration/logical |
243088.5 ns |
251833.5 ns |
0.97 |
array/iteration/scalar |
306136.75 ns |
295024.25 ns |
1.04 |
array/permutedims/2d |
71986 ns |
71828.5 ns |
1.00 |
array/permutedims/3d |
71476 ns |
69728.5 ns |
1.03 |
array/permutedims/4d |
74198.5 ns |
74073.75 ns |
1.00 |
array/random/rand/Float32 |
44495.5 ns |
45320.75 ns |
0.98 |
array/random/rand/Int64 |
54270.75 ns |
54208.25 ns |
1.00 |
array/random/rand!/Float32 |
65776 ns |
64303.25 ns |
1.02 |
array/random/rand!/Int64 |
55200.75 ns |
71623.5 ns |
0.77 |
array/random/randn/Float32 |
81006.25 ns |
79463.75 ns |
1.02 |
array/random/randn!/Float32 |
80341 ns |
80086.25 ns |
1.00 |
array/reductions/mapreduce/Float32/1d |
108146.75 ns |
97711.5 ns |
1.11 |
array/reductions/mapreduce/Float32/dims=1 |
94866.25 ns |
85676.25 ns |
1.11 |
array/reductions/mapreduce/Float32/dims=1L |
842147 ns |
832249.5 ns |
1.01 |
array/reductions/mapreduce/Float32/dims=2 |
99669 ns |
84791.25 ns |
1.18 |
array/reductions/mapreduce/Float32/dims=2L |
144384.5 ns |
144512 ns |
1.00 |
array/reductions/mapreduce/Int64/1d |
107486.5 ns |
97688.75 ns |
1.10 |
array/reductions/mapreduce/Int64/dims=1 |
94903.75 ns |
84121.25 ns |
1.13 |
array/reductions/mapreduce/Int64/dims=1L |
838259.25 ns |
836152 ns |
1.00 |
array/reductions/mapreduce/Int64/dims=2 |
96038.75 ns |
84931.25 ns |
1.13 |
array/reductions/mapreduce/Int64/dims=2L |
144549.5 ns |
142327 ns |
1.02 |
array/reductions/reduce/Float32/1d |
107706.5 ns |
97861.25 ns |
1.10 |
array/reductions/reduce/Float32/dims=1 |
94466.25 ns |
84426.25 ns |
1.12 |
array/reductions/reduce/Float32/dims=1L |
842487 ns |
835814.5 ns |
1.01 |
array/reductions/reduce/Float32/dims=2 |
97311.5 ns |
84691.25 ns |
1.15 |
array/reductions/reduce/Float32/dims=2L |
144412 ns |
143922.25 ns |
1.00 |
array/reductions/reduce/Int64/1d |
107349 ns |
97814 ns |
1.10 |
array/reductions/reduce/Int64/dims=1 |
92998.75 ns |
83881.25 ns |
1.11 |
array/reductions/reduce/Int64/dims=1L |
834579.25 ns |
827999.25 ns |
1.01 |
array/reductions/reduce/Int64/dims=2 |
97911.25 ns |
84931 ns |
1.15 |
array/reductions/reduce/Int64/dims=2L |
144567 ns |
144367 ns |
1.00 |
array/reverse/1d |
46175.5 ns |
45648.25 ns |
1.01 |
array/reverse/1dL |
75903.5 ns |
73036 ns |
1.04 |
array/reverse/1dL_inplace |
63353.25 ns |
80503.75 ns |
0.79 |
array/reverse/1d_inplace |
61746 ns |
52233.25 ns |
1.18 |
array/reverse/2d |
50685.75 ns |
50390.75 ns |
1.01 |
array/reverse/2dL |
91208.75 ns |
82971.25 ns |
1.10 |
array/reverse/2dL_inplace |
92803.75 ns |
92171.25 ns |
1.01 |
array/reverse/2d_inplace |
59695.75 ns |
55451 ns |
1.08 |
array/sorting/1d |
335577.25 ns |
335057.25 ns |
1.00 |
integration/byval/reference |
39521 ns |
39661 ns |
1.00 |
integration/byval/slices=1 |
40360 ns |
40411 ns |
1.00 |
integration/byval/slices=2 |
158202 ns |
147452 ns |
1.07 |
integration/byval/slices=3 |
238793 ns |
248133 ns |
0.96 |
integration/volumerhs |
4989270 ns |
5013333 ns |
1.00 |
kernel/indexing |
36878 ns |
57200.75 ns |
0.64 |
kernel/indexing_checked |
60208.25 ns |
54958.25 ns |
1.10 |
kernel/launch |
1320 ns |
1382.5 ns |
0.95 |
kernel/rand |
119299.25 ns |
98641.5 ns |
1.21 |
latency/import |
1998362070 ns |
1895221133 ns |
1.05 |
latency/precompile |
41803446362 ns |
38320507704 ns |
1.09 |
latency/ttfp |
6051840349 ns |
5930819959 ns |
1.02 |
This comment was automatically generated by workflow using github-action-benchmark.
The previous fix dev'ed the subpackages in the step command, but the julia-test plugin instantiates the project in its pre-command hook, which runs before the step's command -- so Julia 1.10 (no [sources] support) still failed with "expected package ROCm_Runtime to be registered". Follow CUDA.jl's approach instead: on 1.10 (tests, Enzyme, GPU-less check) drop the julia-test plugin and run Pkg.develop + Pkg.test manually; on 1.11+ the plugin works as-is since Pkg honors [sources] there, so those steps go back to their plain Pkg.update() command. Verified on a fresh clone with Julia 1.10: develop + update resolves, and Pkg.test constructs its sandbox with the dev'ed subpackages from the parent manifest. Assisted-by: Claude Code (claude-fable-5)
|
Cool! Do we also handle Instinct GPUs? I see they seem omitted in explicit cases which though relate mostly to Windows OS from the comments. |
|
Yes, instinct should work just fine on Linux, it's just Windows that doesn't support them because upstream doesn't provide tarballs for that combination. We have external CI on MI300, correct? Could you trigger a run, or can I even do it myself now? |
|
I see, the marketing name is Windows only indeed, so all seems fine and Instinct GPUs are included. Just tested this locally and all works fine. |
This would only be possible if the PR comes from an AMDGPU branch (limitation from external CI). Is there a way we could switch to a AMDGPU branch now that you have write access? |
Some of the AI comments still need to be cleaned up but the logic looks reasonable from a first review, so I thought I'd put this up as a draft for feedback.
It turns out MIOpen needs all of libllvm and libclang, so unlike discussed in #919 (comment), I went back to shipping therock artifacts directly as done in that PR, since we don't gain much by repackaging anymore.
Otherwise, do a split similar to CUDA.jl of having ROCm_Runtime be a pseudo-jll providing automatic downloads through artifacts and having manual discovery live in ROCm_Runtime_Discovery. Either one of those is then loaded depending on a preference