ci(gfx11): bundle librocprofiler-sdk for roofline; add roofline input#44
Merged
Conversation
Author
|
Counter part PR: https://gitenterprise.xilinx.com/FaaSApps/rocm-scripts/pull/678 |
The per-op roofline profiler dlopens librocprofiler-sdk.so.1 at runtime (ggml-cuda-roofline.cpp), but the release only bundled librocprofiler-register.so, so the profiler silently no-op'd and wrote no artifact. Pack librocprofiler-sdk.so* plus its one bundle-missing dep librocm_sysdeps_dw.so.1 (libdw, a hard DT_NEEDED of the sdk lib); the existing $ORIGIN RPATH step lets the dlopen resolve both locally with no LD_LIBRARY_PATH. Add a `roofline` workflow_dispatch input (default true, preserving the prior always-on behavior) gating both -DGGML_HIP_ROOFLINE and the extra lib packing, so lean builds can opt out. Co-Authored-By: Claude Opus 4 (1M context) <noreply@anthropic.com>
The comment pointed at utils/gather_required_libs.py to regenerate the ROCm lib copy list, but that script does not exist in the repo; the list is maintained by hand. Remove the dead reference. Co-Authored-By: Claude Opus 4 (1M context) <noreply@anthropic.com>
roberteg16
force-pushed
the
rogarcia.roofline-bundle-libs
branch
from
caeace5 to
e9236fd
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
2 participants
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.
Summary
The per-op roofline profiler
dlopenslibrocprofiler-sdk.so.1at runtime (ggml/src/ggml-cuda/ggml-cuda-roofline.cpp), but the release only bundledlibrocprofiler-register.so. Without the-sdklib on the path the profiler silently no-ops and writes no artifact — which is why the downstreamttft_msroofline KPIs never populated.This PR makes the gfx11 release self-contained for profiling:
librocprofiler-sdk.so*plus its one bundle-missing transitive deplibrocm_sysdeps_dw.so.1(elfutils libdw, a hardDT_NEEDEDof the sdk lib). Its own deps (elf/z/zstd/liblzma/bz2) are already bundled. The existing$ORIGINRPATH step lets the runtimedlopenresolve both locally with noLD_LIBRARY_PATH.rooflineworkflow_dispatchinput (defaulttrue, preserving the prior always-on behavior) gating both-DGGML_HIP_ROOFLINEand the extra lib packing, so lean builds can opt out.Why
This replaces a runtime workaround on the consumer side (rocm-scripts installed
rocm-sdk-coreinto a venv and appended it toLD_LIBRARY_PATHjust to obtain the-sdklib). A companion rocm-scripts PR removes that workaround once a release built from this change is published.