fix(examine): encode GC version minor/revision as hex digits - #152
fix(examine): encode GC version minor/revision as hex digits#152volen-silo wants to merge 1 commit into
Conversation
An LLVM gfx target is `gfx` + major in decimal + minor and revision as single
hex digits — `gfx90a` is GC 9.0.10 (10 == 0xa). `gfx_target_from_gc_version`
concatenated the three components as decimal, which agrees with hex only while
every component is below 10. Checked against every gfx target this repo already
knows (gfx900/906/908/90a/942/950, gfx1030-1036, gfx1100-1103, gfx1150-1153,
gfx1200/1201/1250); none contradicts the hex convention.
The concrete victim is gfx90a (MI210/MI250/MI250X), on the primary detection
path: KFD packs it as `90010`, `parse_linux_kfd_gfx_target` decodes 9/0/10
correctly, and the helper then produced "gfx9010". That token misses every
specific arm of `normalize_therock_family` and falls through to the loose
`starts_with("gfx90")` one, yielding "gfx90X-dcgpu" instead of "gfx90a" — the
wrong TheRock runtime wheel, and outside VLLM_PREFERRED_THEROCK_FAMILIES, so
engine selection changed too. The ip_discovery fallback fed the same helper and
had the same defect.
A minor or revision that cannot be a single hex digit now yields None rather
than a fabricated target, so detection tries the next source and otherwise
reports the target as unknown. For the same reason the 5/6-digit branch of
`parse_linux_kfd_gfx_target` no longer falls back to `gfx{digits}`: that value
is KFD's packed version, not a target name, and passing it through was how
out-of-range decodes still reached the loose family arm. An unknown target is
recoverable with `--family`; note this turns a previously-succeeding (but
wrong-wheel) `rocm install sdk` on such a host into a hard error from
`resolve_family`.
Sub-10 components are unchanged, so every existing expectation holds.
Refs EAI-7346. This does not appear to close the reported MI455X symptom
(gfx1250 read as gfx1210). GC 12.5.0 packs to 120500 and decodes to gfx1250
both before and after this change. Under the old encoding `gfx1210` was also
reachable from GC 1.2.10 and 1.21.0 (packed 10210 / 12100), which this change
maps to gfx12a and None respectively — but neither is a plausible GC IP
version. I could not obtain the MI455X sysfs values, so this is reasoning about
the code, not an observation of the hardware: closing EAI-7346 needs that data.
If the cause is the GC IP version differing from the LLVM target, the fix is an
override consulted before or inside `detect_linux_sysfs_gfx_target` — the
existing PCI/marketing table cannot serve, since it is only reached when sysfs
returns None and, on Linux, only under WSL.
Signed-off-by: Eugene Volen <Eugene.Volen@amd.com>
ReviewApprove — no blocking issues. The fix lands in the shared Verified locally: 1. The stated justification for removing the
|
tests/e2e-cucumber/expectations.tomlfor the fixed ticket ID and removed/narrowed any now-stale xfail rows. (No EAI-7346 rows exist.)Problem
An LLVM gfx target is
gfx+ major in decimal + minor and revision as single hex digits —gfx90ais GC 9.0.10 (10 == 0xa),gfx1250is GC 12.5.0.gfx_target_from_gc_versionconcatenated all three components as decimal, which agrees with hex only while every component is below 10.Checked against every gfx target this repo already knows — gfx900/906/908/90a/942/950, gfx1030–1036, gfx1100–1103, gfx1150–1153, gfx1200/1201/1250 — and none contradicts the hex convention.
The concrete victim is gfx90a (MI210/MI250/MI250X), on the primary detection path (
detect_linux_gfx_targettries KFD first,ip_discoverysecond — both feed the same helper):90010(major·10000 + minor·100 + step).parse_linux_kfd_gfx_targetdecodes 9 / 0 / 10 — correctly."gfx9010"instead of"gfx90a".normalize_therock_family("gfx9010")misses every specific arm and falls through to the loosestarts_with("gfx90")one →"gfx90X-dcgpu"instead of"gfx90a".VLLM_PREFERRED_THEROCK_FAMILIES, so engine selection changes too.Approach
majorstays decimal (gfx1030,gfx1250need both digits).Nonerather than a fabricated target, so detection tries the next source and otherwise reports the target as unknown.parse_linux_kfd_gfx_targetno longer falls back togfx{digits}. That value is KFD's packed version, not a target name, and passing it through was how out-of-range decodes still reached the loose family arm. The 3/4-digit branch keepsgfx{digits}, which is correct there.Sub-10 components are unchanged, so every existing expectation holds.
Behavior change
An unknown target is recoverable with
--family, but this does turn a previously-succeeding (and wrong-wheel)rocm install sdkon an affected host into a hard error fromresolve_family, which lists the recognized families. That is the intended direction — failing loudly beats silently installing the wrong runtime — but it is a visible change.What this does not close
Not
Fixes EAI-7346, deliberately. The reported MI455X symptom (gfx1250 read as gfx1210) does not appear to follow from this code: GC 12.5.0 packs to120500and decodes togfx1250both before and after. Under the old encodinggfx1210was also reachable from GC 1.2.10 and 1.21.0 (packed10210/12100), which this change maps togfx12aandNone— but neither is a plausible GC IP version.I could not obtain the MI455X sysfs values, so that is reasoning about the code, not an observation of the hardware. Closing EAI-7346 needs
gfx_target_versionandip_discovery/die/*/GC/*/{major,minor,revision}from the machine. If the cause is the GC IP version differing from the LLVM target, the fix is an override consulted before or insidedetect_linux_sysfs_gfx_target— the existing PCI/marketing table cannot serve, since it is only reached when sysfs returnsNoneand, on Linux, only under WSL.Related gap, not addressed here:
normalize_therock_family("gfx1250")returnsNone, so even a correctly detected gfx1250 maps to no family. Naming MI455X's TheRock family would be a guess, and guessing reintroduces the silent-wrong-family class this PR fixes.Testing
cargo fmt --all -- --checkandcargo clippy --workspace --all-targets -- -D warningsclean.cargo test --workspace --no-fail-fastpasses except two pre-existingrocm-coreproc_lifecycle::tests::tree_*failures, which fail on WSL2 for process-tree signalling reasons unrelated to this change (untouched file).New coverage: the
gfx90aregression at the helper, throughparse_linux_kfd_gfx_target("90010"), through anip_discoveryfixture, and end to end vianormalize_therock_family— plus out-of-range rejection. Every new assertion except themajor == 0case fails against the old implementation.Not run locally: no GPU on this host, so
rocm examinewas not exercised against real gfx90a hardware; the KFD andip_discoverypaths are covered by fixtures only.Refs EAI-7346.