Skip to content

build: re-enable rocm-smi-lib build (#8907) - #9574

Merged
HereThereBeDragons merged 1 commit into
release/therock-10.2from
users/lpromber/therock-10.2_8907
Oct 29, 2026
Merged

HereThereBeDragons merged 1 commit into
release/therock-10.2from
users/lpromber/therock-10.2_8907

Conversation

@HereThereBeDragons

Copy link
Copy Markdown
Contributor

Cherry pick of 37ca1c5
JIRA ID: AIRDEL-111

Motivation

ROCm SMI now ships with deprecation notices (ROCm/rocm-systems#13131), so build it again instead of skipping it. Partially reverts #6852.

This is intended to add rocm-smi back in for 10.2, but only for external users who didn't get the deprecation notice. This is meant to be a final build with a warning in the output for users to move over to amd-smi.

Technical Details

File Change
base/CMakeLists.txt Drop -DROCM_SMI_DISABLE=True
rocm-sdk-core setup.py, _cli.py Restore the rocm-smi console script
rocm_sdk/_dist_info.py Restore the rocm_smi64 core library entry
rocm_sdk/tests/core_test.py Restore the rocm-smi console-script test
amdrocm-core / amdrocm-developer-tools postinst + prerm Add rocm-smi next to amd-smi in the update-alternatives lists (dropped by #7672 while rocm-smi-lib was disabled)

Not restored: rocm_smi64 in the PyTorch wheel preload list (external-builds/pytorch/build_prod_wheels.py).

Test Plan

  • CI build + rocm-sdk core tests
  • python3 -m pytest build_tools/tests/sdk_targets_test.py
  • Render the 4 postinst/prerm templates (deb), bash -n, run against a fake prefix with a stub update-alternatives

Test Result

  • sdk_targets_test.py: 5 passed
  • _dist_info registers rocm_smi64 in core
  • All 4 scripts pass bash -n; postinst installs and prerm removes /usr/bin/rocm-smi alongside amd-smi

Issue Tracking

JIRA ID: ROCM-21935

## Motivation

ROCm SMI now ships with deprecation notices (ROCm/rocm-systems#13131),
so build it again instead of skipping it. Partially reverts #6852.

This is intended to add rocm-smi back in for 10.2, but only for external
users who didn't get the deprecation notice. This is meant to be a final
build with a warning in the output for users to move over to amd-smi.

## Technical Details

| File | Change |
|------|--------|
| `base/CMakeLists.txt` | Drop `-DROCM_SMI_DISABLE=True` |
| `base/post_hook_rocm_smi_lib.cmake` | Restore libdrm install-interface
include dirs for `rocm_smi64` / `oam` |
| `rocm-sdk-core` `setup.py`, `_cli.py` | Restore the `rocm-smi` console
script |
| `rocm_sdk/_dist_info.py` | Restore the `rocm_smi64` core library entry
|
| `rocm_sdk/tests/core_test.py` | Restore the `rocm-smi` console-script
test |
| `amdrocm-core` / `amdrocm-developer-tools` postinst + prerm | Add
`rocm-smi` next to `amd-smi` in the update-alternatives lists (dropped
by #7672 while rocm-smi-lib was disabled) |

Not restored: `rocm_smi64` in the PyTorch wheel preload list
(`external-builds/pytorch/build_prod_wheels.py`).

## Test Plan

- CI build + `rocm-sdk` core tests
- `python3 -m pytest build_tools/tests/sdk_targets_test.py`
- Render the 4 postinst/prerm templates (deb), `bash -n`, run against a
fake prefix with a stub `update-alternatives`

## Test Result

- `sdk_targets_test.py`: 5 passed
- `_dist_info` registers `rocm_smi64` in `core`
- All 4 scripts pass `bash -n`; postinst installs and prerm removes
`/usr/bin/rocm-smi` alongside `amd-smi`

## Issue Tracking

JIRA ID: ROCM-21935

---------

Signed-off-by: Maisam Arif <Maisam.Arif@amd.com>
Co-authored-by: gabrpham <Gabriel.Pham@amd.com>
@peymanr

peymanr commented Oct 28, 2026

Copy link
Copy Markdown

PR Pundit review

(testing PR Pundit Code Review agent. Not all comments may be relevant. Please reach out to @peymanr for suggestions and feedback.)

# Finding Where
1 🟠 rocm_smi64 not restored to LINUX_LIBRARY_PRELOADS external-builds/pytorch/build_prod_wheels.py:175
2 🔵 Shared rocm-smi alternative removed when either owning package uninstalled build_tools/packaging/linux/template/scripts/amdrocm-developer-tools-prerm.j2:20
3 🔵 rocm_smi64 entry misplaced and required rather than optional build_tools/packaging/python/templates/rocm/src/rocm_sdk/_dist_info.py:295

1. 🟠 Should fix: rocm_smi64 not restored to LINUX_LIBRARY_PRELOADS

external-builds/pytorch/build_prod_wheels.py:175

This PR undoes #6852 except for one change: 50dc7263a also removed "rocm_smi64" from LINUX_LIBRARY_PRELOADS in external-builds/pytorch/build_prod_wheels.py, and that entry is not restored. Now that librocm_smi64 is built and listed again (LibraryEntry("rocm_smi64", ...)), the generated torch/_rocm_init.py will not preload it with RTLD_GLOBAL. The preload was originally added in #4440 because PyTorch smoke tests failed with librocm_smi64.so.1: cannot open shared object file. Without it, wheel installs risk that failure again, or a fallback to a system /opt/rocm copy. Re-add "rocm_smi64" to LINUX_LIBRARY_PRELOADS to complete the revert.

Who is affected: Linux PyTorch wheels built with build_prod_wheels.py against ROCm Python packages from this PR, where torch or its ROCm libraries load librocm_smi64 at runtime.

Verified by running it.

Related past reviews: #898 on build_tools/packaging/python/templates/rocm/src/rocm_sdk/_dist_info.py, #2779 on build_tools/packaging/python/templates/rocm/src/rocm_sdk/tests/core_test.py

2. 🔵 Nit: Shared rocm-smi alternative removed when either owning package uninstalled

build_tools/packaging/linux/template/scripts/amdrocm-developer-tools-prerm.j2:20

rocm-smi is added to the update-alternatives lists of both amdrocm-core and amdrocm-developer-tools, but the binary ships in amdrocm-base. Both packages register the same path, and update-alternatives does not count registrations. So removing either metapackage runs --remove rocm-smi and deletes /usr/bin/rocm-smi, even though the binary and the other metapackage are still installed. The base scripts already do exactly this for amd-smi, rocminfo and rocm_agent_enumerator, so this PR adds one more instance of an existing pattern. A proper fix (register shared tools from one package, or from amdrocm-base) belongs in a separate cleanup.

Who is affected: Native deb/rpm users who install both amdrocm-core and amdrocm-developer-tools and then remove one of them lose the /usr/bin/rocm-smi link (as they already do for amd-smi/rocminfo).

Verified by running it.

Related past reviews: #7672 on build_tools/packaging/linux/template/scripts/amdrocm-developer-tools-postinst.j2

3. 🔵 Nit: rocm_smi64 entry misplaced and required rather than optional

build_tools/packaging/python/templates/rocm/src/rocm_sdk/_dist_info.py:295

LibraryEntry("rocm_smi64", ...) is inserted between rocm_kpack and rocdecode. Its sibling amd_smi is three lines below, so grouping the two SMI libraries would read better. Making the entry optional=True is not needed: on Linux rocm_smi_lib is now always built as part of base, which every core package includes. On Windows the empty DLL pattern already makes find_libraries skip it.

Who is affected: Readers and maintainers of _dist_info.py; this has no functional effect on any build.

Related past reviews: #5283 on build_tools/packaging/python/templates/rocm/src/rocm_sdk/_dist_info.py, #1191 on build_tools/packaging/python/templates/rocm/src/rocm_sdk/_dist_info.py, #5708 on build_tools/packaging/python/templates/rocm/src/rocm_sdk/_dist_info.py

Shared from a pilot of PR Pundit, an agentic code reviewer that builds and tests the change and reads the project's past reviews. Reviewed at 2f89a764; findings checked and posted by @peymanr; points already raised on this PR are left out.

@HereThereBeDragons
HereThereBeDragons merged commit 7a3842c into release/therock-10.2 Oct 29, 2026
372 of 384 checks passed
@HereThereBeDragons
HereThereBeDragons deleted the users/lpromber/therock-10.2_8907 branch October 29, 2026 09:26
@github-project-automation github-project-automation Bot moved this from TODO to Done in TheRock Triage Oct 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

7 participants