Repository navigation
[Build] Find ROCm through rocm-sdk before falling back to /opt/rocm* - #1142
Conversation
`find_package(hip)` was pointed at a `/opt/rocm*` glob, so a pip-installed ROCm SDK inside a virtualenv was invisible and the build either picked up an unrelated system ROCm or failed outright. Order is now `ROCM_PATH`, then `rocm-sdk path --root` — the SDK's own query, whose root carries `lib/cmake/hip/hip-config.cmake` — then the original glob. `ERROR_QUIET` plus the empty-output check covers "rocm-sdk absent" and "rocm-sdk failed" the same way, since both fall through. Verified in a scratch project against the real file: resolves via `rocm-sdk`, resolves via `ROCM_PATH` and takes precedence, and with neither available falls back to the glob and then fails at `find_package` — which is right on a host with no system ROCm. This was the only `/opt/rocm` hardcode in the repo's CMake. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
coderfeli
left a comment
There was a problem hiding this comment.
Reviewed the updated HIP discovery change. The real pip ROCm HIP config reads ROCM_PATH when resolving dependencies, so an empty or stale value previously caused configure to fail at AMDDeviceLibs even after hip_DIR selected the SDK. Commit f39f41b scopes ROCM_PATH to the SDK during that lookup and restores it afterward. I also removed the mock-only discovery test.
I configured against the installed pip ROCm SDK with ROCM_PATH unset, empty, and stale; all three now select the SDK. A valid explicit prefix and the normal fallback after a failed rocm-sdk command also resolved as expected. The four existing CMake backend tests pass. I found no remaining blocking issue in this diff; CI for the updated head is still pending.
Motivation
Since ROCm 7.14, ROCm can be installed through pip. The HIP package used to build
FlyDSL must come from the selected ROCm installation. Passing a ROCm prefix in
find_package(hip ... PATHS ...)did not guarantee this: CMake searches itsnormal package locations before
PATHS, so a system HIP could win over the SDKin the active virtual environment.
Change
The ROCm runtime now tries HIP in this order:
ROCM_PATH, if it resolves a HIP package;rocm-sdk path --root, if that command succeeds andresolves a HIP package;
/opt/rocm*fallback.The first two searches use CMake's
NO_DEFAULT_PATHoption so defaults cannotprecede the selected prefix. An empty or stale
ROCM_PATHfalls through to theSDK. While loading HIP from the SDK,
ROCM_PATHtemporarily points to that SDKso HIP's own dependencies are found there too; its original value is restored
afterwards. If
rocm-sdkis missing or fails, the original fallback still works.As with other CMake packages, an explicit or previously cached
hip_DIRremainsauthoritative. Clear
hip_DIRor use a fresh build directory when switchingROCm installations.
Validation
flydsl-0.3.1.dev1129-cp313-cp313-linux_x86_64.whlin a Debian 13 container with a ROCm 7.14 virtual environment.
confirm that unset, empty, and stale
ROCM_PATHvalues select the SDK.A valid
ROCM_PATHwins, and a failedrocm-sdkcommand falls back tonormal CMake package discovery.
main, a local Sphinx build with warnings treated aserrors passed. This includes the docs change that removes the remote mapping
request which had hit a GitHub API rate limit on the older branch.
GPU tests and benchmarks passed. Navi passed on retry after its runner failed
to download the wheel artifact on the first attempt. Checks for the current
head are shown below.
Submission Checklist
https://github.com/ROCm/TheRock/blob/main/GOVERNANCE.md#pull-requests.