Skip to content

Cherry-pick AMDSMI commits into ROCm 10.0 - #9734

Closed
marbre wants to merge 3 commits into
release/therock-10.0from
users/marbre/therock-10.0-AIRDEL-17
Closed

marbre wants to merge 3 commits into
release/therock-10.0from
users/marbre/therock-10.0-AIRDEL-17

Conversation

@marbre

@marbre marbre commented Aug 5, 2026

Copy link
Copy Markdown
Member

Motivation

Cherry-picks amdsmi commits into the ROCm 10.0 release branch.

Technical Details

Issue Tracking

JIRA ID: AIRDEL-17

@github-actions github-actions Bot added documentation Improvements or additions to documentation project: amdsmi github actions Pull requests that update GitHub Actions code labels Aug 5, 2026
@marbre marbre changed the title Users/marbre/therock 10.0 airdel 17 Cherry-pick AMDSMI commits into ROCm 10.0 Aug 5, 2026
@marbre
marbre marked this pull request as ready for review August 5, 2026 20:32
@marbre
marbre requested review from a team August 5, 2026 20:32
@marbre
marbre requested review from a team as code owners August 5, 2026 20:32
marifamd and others added 3 commits August 8, 2026 12:51
Relands the manylinux wheel CI job (#7913) and the Python packaging rework
(#7914) after they were reverted in #9339, together with the follow-up
amd-smi CLI shebang fixes so the rework and its corrections land as one
change.

Packaging rework (reintroduced):
- Install the amdsmi module into the system python's site-packages via
  install(DIRECTORY) instead of pip + a .pth file; drop the pip plumbing and
  python3-pip/setuptools/wheel deps from the DEBIAN/RPM scriptlets.
- Replace the ROCm-path-walking resolver in amdsmi_wrapper.py (and
  tools/generator.py, regenerated in lockstep) with a flat loader:
  AMDSMI_LIB_OVERRIDE, a bundled libamd_smi_python.so, then the system
  libamd_smi.so; a _MissingLibrary sentinel keeps import tolerant.
- Add BUILD_PYTHON_WHEEL (default OFF): build a SONAME-isolated
  libamd_smi_python.so and a wheel that disables the loader's system fallback.
- Stage the module under share/amd_smi for ROCM_PATH consumers and TheRock.
- Add test_abi_compat.py and the manylinux wheel CI job.

CLI shebang fixes:
- Use a portable "#!/usr/bin/env python3" shebang for the amd-smi CLI so it
  runs on relocatable installs with no /usr/bin/python3 (TheRock containers)
  and on wheel runners where /usr/bin/python3 lacks the amdsmi module.
- Stop pinning the CLI shebang to a fixed interpreter: remove pin_shebang.cmake
  and the AMDSMI_CLI_SHEBANG_INTERP derivation. The pinned absolute path became
  a hard RPM "Requires:" (e.g. /usr/libexec/platform-python3.6) that RHEL10 and
  SLES16 could not satisfy; the RPM now depends only on python(abi).
- Resolve the env-form shebang in the install-verify harness instead of running
  "/usr/bin/env -c".

Signed-off-by: Maisam Arif <Maisam.Arif@amd.com>
…7912)

Add tests/run_amdsmi_pkg_conflict_test.py, which asserts that two AMD SMI
packages built with different SOVERSIONs do not co-own the same
libamd_smi.so.<major>, so installing one over the other cannot silently
break the other's loader.

Wire the ABI-compat output into the build harness: summarize_results in
run_amdsmi_build.py now scans abi_compat_output.txt alongside the existing
integration/unit/perf test outputs so an ABI regression surfaces in the CI
summary.

Signed-off-by: Maisam Arif <Maisam.Arif@amd.com>
…rework (#9033)

## Description

Correctness and coverage follow-ups to the AMD SMI Python packaging
rework (relanded in #9340). This branch is the packaging-hardening layer
on top of the SONAME conflict test + build harness (#7912).

Rebased onto `develop` after #7912 and #9340 merged; the now-upstream
shebang and self-locate-loader commits were dropped, and the surviving
relocatable-loader change was reconciled with develop's version (keeps
develop's `len(parents) > 3` guard and adds a `try/except OSError`
fall-through so a present-but-unloadable relocatable library does not
shadow the system SONAME).

### Loader / wrapper
- Fall through to the system library when the relocatable TheRock
library exists but fails to load (missing deps).
- Dedupe the wrapper `_libraries` init (the ctypesgen preamble already
initializes it).
- Read and write the staged wrapper as UTF-8 in
`disable_system_fallback.py` so the wheel flag flip is deterministic
regardless of build locale; make the staging idempotent so a rebuild
that reuses the flipped copy succeeds.

### Packaging correctness
- Stop the RPM from co-owning system Python directories.
- Restrict package `preun`/`prerm` cleanup to a full erase (not
upgrades).
- Quote the install prefix when writing `ld.so.conf.d` on RPM.
- Pin permissions on the installed Python module; make the operational
log group-writable (not world-writable) and stop the logger from
restoring world-writable permissions.
- Only strip release libraries when a strip tool is available; fail the
build when the system `python3` cannot be detected.

### Wheel build
- Build one wheel per platform (not per interpreter), tag it
`manylinux_2_28`, and verify the bundled `.so`.
- Use a clean PyPI version and expose the commit as metadata.
- Reject regenerating the wrapper against the wheel library; skip the
system site-packages install for wheel builds.

### Tests
- Loader-contract tests in `test_abi_compat.py`: preferred bundled
`.so`, wheel refuses system fallback when the bundle is missing, the
relocatable OSError fall-through, and an ambiguous-wrapper hard failure.
Assert the idempotent no-op in the fallback double-run guard.
- Guard against drift between the two installed module copies; install
the dual-copy runner so its guard tests run in CI; run the wheel
install-location check without a GPU.

### Verification
- `cmake -DBUILD_TESTS=ON` + `make`: 0 errors, `libamd_smi.so.27` +
`amdsmitst` built.
- `test_abi_compat.py` (10) and `test_dual_copy_guard.py` (4): all pass.

## JIRA ID

ROCM-3941

---------

Signed-off-by: Maisam Arif <Maisam.Arif@amd.com>
@marifamd
marifamd force-pushed the users/marbre/therock-10.0-AIRDEL-17 branch from 36175f9 to 91aea1d Compare August 8, 2026 17:51
@marbre

marbre commented Aug 11, 2026 •

Copy link
Copy Markdown
Member Author
  • rocprofiler-compute test is failing with

    2:    INFO    |-> [rocprofiler-sdk] Finished executing kernel
    2:    INFO    |-> [rocprofiler-sdk] Finished executing kernel
    2:    INFO    |-> [rocprofiler-sdk] Finished executing kernel
    2:    INFO    |-> [rocprofiler-sdk] Finished copying the output vector from the GPU to the CPU
    2:    INFO    |-> [rocprofiler-sdk] Error at position i 1, Expected: 0.000000, Found: 0.000000
    2:    INFO    |-> [rocprofiler-sdk] Error at position i 2, Expected: 0.000000, Found: 0.000000
    2:    INFO    |-> [rocprofiler-sdk] Error at position i 3, Expected: 0.000000, Found: 0.000000
    2:    INFO    |-> [rocprofiler-sdk] Error at position i 4, Expected: 0.000000, Found: 0.000000
    2:    INFO    |-> [rocprofiler-sdk] Error at position i 5, Expected: 0.000000, Found: 0.000000
    2:    INFO    |-> [rocprofiler-sdk] Error at position i 6, Expected: 0.000000, Found: 0.000000
    2:    INFO    |-> [rocprofiler-sdk] Error at position i 7, Expected: 0.000000, Found: 0.000000
    2:    INFO    |-> [rocprofiler-sdk] Error at position i 8, Expected: 0.000000, Found: 0.000000
    2:    INFO    |-> [rocprofiler-sdk] Error at position i 9, Expected: 0.000000, Found: 0.000000
    2:    INFO    |-> [rocprofiler-sdk] Error at position i 10, Expected: 0.000000, Found: 0.000000
    2:    INFO    |-> [rocprofiler-sdk] Error at position i 11, Expected: 0.000000, Found: 0.000000
    [..]
    

    while this passes on the release branch in TheRock: https://github.com/ROCm/TheRock/actions/runs/31371289759/job/93605017563

  • rocprofiler-sdk test is timing out.

@marifamd

Copy link
Copy Markdown
Contributor

@marbre Is this issue related: ROCm/rocm-libraries#10056 I believe it was merged after 10.0 branch

@marbre

marbre commented Aug 11, 2026

Copy link
Copy Markdown
Member Author

@marbre Is this issue related: ROCm/rocm-libraries#10056 I believe it was merged after 10.0 branch

The error message is a complete different one and the test is passing on the release branch, so there is a chance it is related to the cherry-pick. Can't merge with that red signal.

@marbre

marbre commented Aug 12, 2026

Copy link
Copy Markdown
Member Author

@marifamd any update on this?

@marifamd

Copy link
Copy Markdown
Contributor

After reviewing with @dmitrii-galantsev there are some patches that haven't been in a nightly yet, they were merged last night. This change is too risky for 10.0 and I'm closing this out

@marifamd marifamd closed this Aug 12, 2026
marbre pushed a commit that referenced this pull request Aug 18, 2026
#9864)

## Motivation

Bring the unified ABI changes onto the `release/therock-10.0` release
branch. These already-merged `develop` commits change the amd_smi ABI
(64-bit metrics counters, driver-reload removal, unified struct sync)
and must land together so the 10.0 header, wrapper and interface stay
consistent with each other — and so 10.0 does not ship an ABI that
differs from `develop`.

The two earlier attempts are closed: #9852 stacked these changes on the
Python packaging series, and #9734 was that packaging series. With #9734
closed, the packaging rework is not coming to 10.0, so this PR applies
the ABI changes to the raw release branch.

## Technical Details

**Five commits** on the `release/therock-10.0` tip (`6ccde5dbc27`). Four
are cherry-picks of merged `develop` commits (`-x` provenance
preserved); one is a changelog retarget.

| # | Commit | Cherry-picked from | Notes |
|---|--------|--------------------|-------|
| 1 | `15432e923f` fix(amdsmi): Bump GPU metrics accumulator counters
(#8210) | `aeb9da985b` | byte-identical |
| 2 | `7ff1686439` refactor(amdsmi): remove driver reload API and
related functions (#8575) | `706209f4f5` | byte-identical |
| 3 | `23ec5edc37` refactor(amdsmi): Sync with unified with known abi
changes (#7844) | `923767a953` | 3 conflicts, resolved below |
| 4 | `9030ac095c` docs(amdsmi): set changelog version to 10.0 | — |
retargets the changelog heading from `7.15.0` to `10.0` |
| 5 | `94cec8fda6` fix(rccl): correct UALoE fabric detection and
diagnostics on the amdsmi path (#9464) | `0f674c16e8` | byte-identical —
**required**, see below |

### Why the RCCL commit is part of this PR

Commit 5 is not incidental: **#7844 does not build without it.** `#7844`
flattens `amdsmi_fabric_info_t`, deleting `amdsmi_fabric_info_ver_t` and
moving the version to a top-level `fabric_version`. RCCL on the release
branch hardcodes the old nested spelling:

```cpp
// projects/rccl/src/misc/amdsmi_wrap.cc @ 6ccde5d
fabricInfo.fabric_info.version                   // deleted by #7844
fabricInfo.fabric_info.fabric_version.v1         // deleted by #7844
```

Both identifiers cease to exist after commit 3, so RCCL fails to compile
against the new header. `#9464` replaces those accesses with a
`amdSmiFabricInfoIsFlat` SFINAE trait that compiles against **both** the
flat and nested layouts, and fixes the UALoE detection bugs described in
that PR. It is a large diff (8 files, +1140/−29, including new RCCL
tests), so it is called out explicitly here.

**These five commits must land as a unit.** Splitting the PR, or
reverting the RCCL commit on its own, breaks the RCCL build.

### Conflicts resolved in #7844

Three conflicts, all because the release branch lacks changes `#7844`
was written on top of:

- **`py-interface/CMakeLists.txt`** — kept the release version.
`#7844`'s only hunk here reworks the packaging site-packages install
block, which does not exist on this branch (it came from the packaging
series in the now-closed #9734).
- **`py-interface/amdsmi_interface.py`** — keep-both: added the
`AMDSMI_GPU_CUID_SIZE` alias (context on `develop`, which has #9273)
alongside the release branch's `_AMDSMI_STRING_LENGTH` constants.
- **`CHANGELOG.md`** — dropped the packaging-rework entry, which
describes work not on this branch. `#7844`'s ABI entries merged cleanly.

## Issue Tracking

JIRA ID: AILITOOLS-270, ROCM-1553, ROCM-26658
JIRA ID: AIDREL-13
JIRA ID: AIDREL-19

## Test Plan

- Cherry-picked #8210 → #8575 → #7844 → #9464 onto the
`release/therock-10.0` tip (`6ccde5dbc27`)
- Full amd_smi build (library + CLI + `amdsmitst`)
- Regenerated `amdsmi_wrapper.py` and diffed against the committed file
- Diffed every cherry-pick against its `develop` original
- Verified the `amdsmi_fabric_info_t` flattening is ABI-preserving, by
compiling both headers and comparing `sizeof`/`offsetof`
- Checked every in-repo consumer of the changed/removed types for
breakage

## Test Result

- **Build:** clean (library + CLI + tests, 0 errors)
- **Wrapper:** byte-identical after regeneration (0-line diff)
- **Cherry-pick fidelity:** commits 1, 2 and 5 diff byte-identically
against their `develop` originals (modulo hunk offsets); commit 3
differs only in the three conflicts documented above
- **Fabric ABI is preserved:** `sizeof(amdsmi_fabric_info_t) = 320`
before and after, with every offset unchanged — `bdf=0`, version `=8`,
`v1=12`, `v1.accel_state=252`, `reserved=256`
- **Removed symbols:** `amdsmi_gpu_driver_reload` has no remaining
references outside the CHANGELOG; none of the 16 removed/renamed symbols
has a stale consumer anywhere under `projects/`
- **In-repo consumers:**
- `projects/rdc` reads `amdsmi_gpu_metrics_t` and the widened
accumulators and is *not* modified here — a `-fsyntax-only` A/B of
`RdcMetricFetcherImpl.cc` against the old and new headers produces the
identical error set (pre-existing, ESMI-gated), so nothing regresses.
The fields only widen, and they feed a `uint64_t`-valued map.
- `projects/rocshmem` keeps a private dlopen mirror of
`amdsmi_fabric_info_t`; it does not include `amd_smi/amdsmi.h`, and the
layout is unchanged, so it is unaffected.
- `projects/rocprofiler-systems` uses `amdsmi_gpu_metrics_t` and builds
green in this PR's TheRock CI.
- `projects/rocm-smi-lib` has its own independent `rocm_smi.h`;
unaffected.

## Known issues inherited from `develop`

Faithful cherry-picks, so these come along unchanged; they are present
on `develop` today and are not introduced by this backport. Listed so
the release branch owners know what is shipping. Fixes belong on
`develop` first.

- **`gpu_metrics` accumulators lose their "unset" sentinel** (from
#8210). The counters widened to 64-bit but the amdgpu table still
reports 32, so the `0xFFFFFFFF` sentinel zero-extends and stops matching
the 64-bit sentinel consumers check. `amdsmi_get_gpu_metrics_info()`
reports `gfx_activity_acc`/`mem_activity_acc` as a literal `4294967295`
instead of `N/A` on hardware that does not report them (reproduced on
gfx1030). Unchanged on parts that do report real values.
- **`tests/python/cli/test_set.py` fails instead of skipping** (from
#7844). `cmds_need_permission` is assigned in `setUpClass` after the
`_initialized` early-return, so `TestSet` raises `AttributeError`; and
the filter that replaced the old blanket skip covers only 3 of the
privileged `set` subcommands, so an unprivileged run also fails on
`--perf-level`, `--profile`, `--perf-determinism`, `--clk-level` and
others.
- **`amd-smi set --json` and `amd-smi static --partition --json`
disagree on one key** (from #7844). `set` now emits
`accelerator_partition_mem_alloc_mode` while `static` and the
`--compute-partition-mem-alloc-mode` flag keep
`compute_partition_mem_alloc_mode`.
- **Dead constants**: the keep-both resolution leaves
`_AMDSMI_STRING_LENGTH` and `_AMDSMI_MAX_STRING_LENGTH` with zero
references (every call site moved to the public
`AMDSMI_MAX_STRING_LENGTH`).

## Submission Checklist

- [x] Look over the contributing guidelines at
https://github.com/ROCm/rocm-systems/blob/develop/CONTRIBUTING.md.

---------

Signed-off-by: Bindhiya Kanangot Balakrishnan <Bindhiya.KanangotBalakrishnan@amd.com>
Signed-off-by: Maisam Arif <Maisam.Arif@amd.com>
Signed-off-by: gabrpham <Gabriel.Pham@amd.com>
Signed-off-by: josnarlo <joseph.narlo@amd.com>
Signed-off-by: Galantsev, Dmitrii <dmitrii.galantsev@amd.com>
Co-authored-by: gabrpham <Gabriel.Pham@amd.com>
Co-authored-by: Joseph Narlo <joseph.narlo@amd.com>
Co-authored-by: Galantsev, Dmitrii <dmitrii.galantsev@amd.com>
Co-authored-by: Mika Laitio <mika.laitio@amd.com>
Co-authored-by: Nilesh M Negi <Nilesh.Negi@amd.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation github actions Pull requests that update GitHub Actions code organization: ROCm project: amdsmi

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants