Conversation
The GPU classifiers in `examine` are pure functions over strings a host hands them, and the strings are messy in ways no example test enumerates: `lspci` spells a codename with a die number, Windows decorates names with "(TM)", `rocminfo` pads columns, and `pci.ids` sometimes has no name at all. This builds generators from real fixture shapes — actual `lspci -nn -D` lines, `rocminfo` agent blocks, `Win32_VideoController` rows, AMD marketing names — and perturbs them, because a uniform-random string generator never reaches the branches that matter. Four invariants to start, all of which the classifiers already satisfy: totality and determinism, exact round-tripping of an `lspci` line's device text, independence from how a name is spelled, and agreement between the two Windows probes that read the same adapter row. The properties that assert *correct* classification come with the fixes that make them pass, so no commit here is red. `generator_reach_is_measured_and_sufficient` asserts floors on how far the generators actually reach. A generator that never samples the interesting region passes every property vacuously, so the reach is measured rather than assumed. Adds `proptest` as a dev-dependency of `rocm-core`, with the eight transitive crates that brings recorded in MANIFEST.md. `THIRD_PARTY_NOTICES.txt` is unaffected: `about.toml` sets `ignore-dev-dependencies`, since dev-dependencies are not shipped in the distributed binaries. Failing-case seed files are gitignored — the classification corpora are swept exhaustively by the deterministic tests that follow, so no coverage rides on seed luck. Signed-off-by: Roman Inflianskas <Roman.Inflianskas@amd.com>
`gfx_is_apu_family` matched a gfx110x / gfx115x prefix, so every pre-RDNA3 APU read as a discrete card: gfx90c (Renoir, Cezanne, Lucienne, Barcelo), gfx1033 (Van Gogh), gfx1035 (Radeon 680M/660M) and gfx1036, which is the iGPU on every Ryzen 7000-and-later desktop part. Two consequences a user sees. `serve` compares reported VRAM against a low-memory floor unless the GPU is an APU, so on those machines it measured the BIOS carve-out and warned about low VRAM while the model was being served out of system RAM — the exact false warning that gate exists to withhold. And `diagnose` gates the iGPU+dGPU collision check on `has_apu && has_discrete_amd`, which never both held, so the check never fired on the host shape it was written for. Packaging is a property of the silicon, so it is now stated once, keyed by target, rather than inferred from a numeric prefix that two different families share. The table also carries gfx902, gfx909 and gfx1037, which no lookup table produces — `gfx_target_from_gc_version` synthesises them from the GC version in DRM ip-discovery — and gfx1250, whose family the CLI already installs. It is not every target AMD has shipped and cannot be; an unlisted target answers "unknown", not "discrete". `examine` also kept its own marketing-name table. It resolved a strict subset of what the crate's other table resolves — 28% of a generated corpus against 92% — and the two disagreed about Krackan Point, which it called gfx1150 where the PCI device-id table and the 860M/840M SKUs it ships as both say gfx1152. Those are separate TheRock build families, so the disagreement selected the wrong runtime build. Beside it sat a third list, of APU codename fragments, that answered "is this an APU?" without ever producing a target, so a Renoir iGPU came back with an empty gfx target. Both are gone: one table maps a name to a target, and the packaging follows from the target. Folding those codenames in makes gfx90c reachable from a name for the first time, and `normalize_therock_family` would have filed it under `gfx90X-dcgpu` — a datacenter family, which `preferred_serve_engine_for_ therock_family` also reads as "prefer vLLM". No published family covers a Renoir-class iGPU, so it now answers none and the user is asked to choose. Both GPU probes prefer the PCI device id to the marketing string, since the id names the part where the string only hints at it: on Windows out of the PNP id, on Linux out of the `[1002:xxxx]` tag `lspci -nn` already prints. An adapter named "AMD Radeon(TM) 840M Graphics" resolves gfx1152 rather than nothing. No Gherkin scenario accompanies this (AGENTS.md §3). All three observable behaviours need a host with a pre-RDNA3 AMD APU, and no lane has one — the Strix Halo lanes report gfx1151, which was already classified as integrated before this change, so a scenario there would pass either way. `examine` reads the real host with no injection seam, so the behaviour cannot be planted either. Coverage sits one level down instead: the collision check runs through the real diagnosis catalog on a synthesised Raphael + RX 7900 XTX host, and the Rembrandt laptop case drives the real post-PCI probe sequence against a planted KFD topology. Signed-off-by: Roman Inflianskas <Roman.Inflianskas@amd.com>
Folding a `rocminfo` agent into the report overwrote `is_apu` with the target-derived answer unconditionally, and that answer is `false` both for a discrete GPU and for a target this crate has never heard of. So a GPU the PCI scan had already identified as an APU became a discrete one the moment `rocminfo` named a target outside the packaging table: more evidence produced a worse answer, clearing `has_apu` and with it the iGPU+dGPU collision diagnosis. A target whose packaging is known still wins. The target names the silicon, where the PCI marketing string is a heuristic over vendor-chosen text, so when the two genuinely disagree the target is the better witness. An unknown target is not a disagreement, though — it is an absence of evidence, and it now leaves the earlier verdict standing. The branch that creates a GPU for an agent with no PCI row to pair with — the ordinary ROCm container, which ships `rocminfo` but not `pciutils` — followed the same rule only halfway: it had no earlier verdict to preserve, but it also had the agent's own marketing name, and it discarded it to write `Some(false)`. `summarise_gpu_categories` reads that as "this is a discrete GPU", so an iGPU-only laptop reported a discrete AMD GPU. It now takes the target when the packaging is known, the marketing name when that resolves a part, and leaves `is_apu` unset when neither answers — the same conclusion `probe_gpus_sysfs_fallback` reaches, for the same stated reason. The `hipInfo` fold on Windows had the same shape and follows the same rule. `folding_in_rocminfo_only_ever_adds_knowledge` and `an_agent_with_no_pci_row_is_classified_from_what_evidence_there_is` are the two tests that discriminate this change; the other rocminfo tests in that module pass with it reverted, because they exercise targets the packaging table knows. This was latent rather than live: `rocminfo` prints indented ISA `Name:` sub-entries that the agent parser lets overwrite the agent's own name, so the fold returns early on a real host (#393) and never reached the overwrite. Fixing that parser is what would have made this live, so the test drives both parser shapes and holds the verdict under either. Signed-off-by: Roman Inflianskas <Roman.Inflianskas@amd.com>
The previous commit series stopped `normalize_therock_family` filing a Renoir-class iGPU under `gfx90X-dcgpu`, a datacenter family that also makes vLLM the preferred serving engine, by answering `None`. That overcorrected into a dead end. With no family, `resolve_family` tells the user to re-run with an explicit `--family`; but `AggregateDeviceTarget::resolve` then requires the detected target to normalize to the family named, and `None` matches nothing, so every value the user can pass is refused as "belongs to no recognized package family". The CLI's own remediation advice could not be followed. gfx90c now normalizes to `gfx90c`, the single-target shape gfx900, gfx906, gfx908 and gfx90a already use, and is listed in `known_therock_families`. It is not `-dcgpu`, so no serving engine is preferred for it. Assumed rather than verified: that TheRock's index names this family `gfx90c`. Nothing in this repository records a gfx90c family under any name, and the index could not be queried from here. The assumption fails safely: `known_therock_families` is documented as recognition, not availability, and where a channel publishes no `rocm-sdk-device-gfx90c` payload the install refuses by naming the payload it looked for and points at the other channel — the same outcome as for any other recognised family with no published payload. Signed-off-by: Roman Inflianskas <Roman.Inflianskas@amd.com>
The previous commits made both PCI probes prefer the device id to the marketing name, and made the hipInfo fold leave an unrecognised target's verdict alone. None of the three was exercised by any test that could fail on it: no test reached `probe_gpus_lspci` or `probe_gpus_windows`, which shell out, and the tests that built PCI rows assembled them from the classifiers by hand, re-implementing the name-only logic the probes had just stopped using. Making the Linux scan read the name only, or restoring the unconditional overwrite in the hipInfo fold, left every test green. Each probe now keeps only its process launch; what it does with the output moves into a pure fold over caller-supplied text — `apply_lspci_gpus`, `apply_windows_display_rows`, `apply_hipinfo_gcn_arch_names` — the split `apply_rocminfo_gpu_agents` already has. Every test that needs a PCI-sourced GPU now runs a real row through the real fold. New tests sweep every id the device-id table maps through a row whose name carries nothing (`Device` on Linux; on Windows "AMD Radeon(TM) Graphics", which is what Renoir, Rembrandt and Raphael iGPUs actually report), so each probe's id preference fails for all of them at once if it regresses. The hipInfo rule cannot be shown to matter end to end today: the display probe only reaches an APU verdict by resolving a target, and the fold only touches rows without one, so an unknown target meets `Some(false)` either way. Its test says so, and pins the fold's contract on a constructed state instead, so the first new source of a target-less verdict does not inherit the downgrade silently. An earlier message here said an unlisted target answers "unknown", not "discrete". That is true of `gfx_target_packaging` and of the rocminfo fold, and not of the PCI probes: a row neither table identifies is reported discrete, on purpose. The only consumer that acts on the verdict is the iGPU+dGPU collision check, which needs `has_apu && has_discrete_amd`. Reporting an unidentifiable row as unknown would clear `has_discrete_amd` beside every discrete card the tables do not name — RX 5000, Vega, Radeon VII — on exactly the hybrid host that check exists for. Reporting it discrete costs a wrong `has_discrete_amd` on an iGPU-only host the tables do not name, Mendocino for one, where the check still cannot fire. That trade is now one function, `pci_row_is_apu`, documented there and pinned by a test that drives both hosts through the diagnosis catalog. Also: the cross-check of the Windows probes is restated as what it now is — they share a decoder, so it guards the row shape each one feeds it, not agreement between two tables; a test that no discrete corpus part is ever classified as an APU, which also puts the MI300X row to work; corrected provenance for gfx902/gfx909, which come from KFD's packed `gfx_target_version` rather than DRM ip-discovery; and a duplicated block of assertions removed. Signed-off-by: Roman Inflianskas <Roman.Inflianskas@amd.com>
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
1 participant
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.
tests/e2e-cucumber/expectations.tomlfor the fixed ticket ID and removed/narrowed any now-stale xfail rows. — no rows reference this area.docs/architecture.md. — N/A.Fixes #520
Summary
gfx_is_apu_familyrecognised onlygfx115xandgfx110[3-9], so every older AMD integrated GPU — Raven Ridge, Picasso, Renoir/Cezanne, Van Gogh (Steam Deck), Rembrandt, Raphael (every Ryzen 7000/8000/9000 desktop iGPU), Mendocino — was classified as discrete. On those machinesrocm servewarned about low VRAM against the BIOS carve-out for a model actually served from system RAM, which is the exact warningvram_capacity_is_meaningfulexists to suppress.rocm doctor's iGPU + dGPU check never fired on the host shape it was written for.Risk: medium. This changes what
rocm examinereports for integrated GPUs, whetherrocm servewarns about VRAM, whetherrocm doctorraisesfix-9-igpu-dgpu, and one package family. Every consumer ofis_apu,has_apuandhas_discrete_amdwas traced.What changed
gfx942is both MI300A (an APU) and MI300X (discrete); a target-keyed table cannot express that, and the doc states the exception rather than letting the next reader assume the table is complete.rocminforeading now revisesis_apuonly when the target's packaging is known; an unknown target is an absence of evidence, not a disagreement. The same rule now applies to the Windows hipInfo fold.APU_KEYWORDSis deleted. A name's verdict is nowpackaging(target(name)), so there is one source of truth.examine's marketing-name classifier now delegates to the crate's existing table, which also fixes the Krackan Point target (gfx1150→gfx1152, agreeing with the device-id table) and raises corpus resolution from 28% to 91%.lspci -nnprints[1002:xxxx]on every line; the probe discarded it. It is now preferred over the name, matching what the Windows probe already does with the PNP id, so a GPUpci.idsdoes not know (printed asDevice) can still be classified when its id is in the table.gfx90cpreviously fell through astarts_with("gfx90")catch-all intogfx90X-dcgpu, a datacenter family, which made vLLM the preferred serving engine on a laptop iGPU. It now maps to its own single-target familygfx90c, following the existinggfx900/gfx906/gfx90apattern.Assumptions, stated
device-gfx90cpayload — not a wrong install — becauseknown_therock_familiesis documented as recognition, not availability.gfx125X-dcgpufamily.Deliberate trade-off
A PCI row whose target cannot be resolved is still reported as discrete (
Some(false)), not unknown. The only consumer isfix-9-igpu-dgpu'shas_apu && has_discrete_amdcheck. ReportingNoneinstead would silence that check beside any discrete card the tables do not name (RX 5000, Vega, Radeon VII) — the host it exists for. The cost of keepingSome(false)is a wronghas_discrete_amdon an unrecognised iGPU-only machine, where the check still cannot fire becausehas_apuis false. A test runs both sides throughdiagnose().Test plan
Properties, built from real
lspcilines and marketing names rather than random strings, with measured generator reach asserted as floors so the harness cannot rot. The harness lands in its own commit and is green against the unfixed code, so it smuggles in no fix.Each fix is guarded by a test that fails when only that fix is removed, verified by mutation:
rocminfooverwriteNoneExact)One mutant is equivalent on the real pipeline: restoring the unconditional overwrite in the Windows hipInfo fold changes no output today, because that fold only touches rows the display probe already marked
Some(false). It is guarded by a test on a constructed state, and the test's doc says so.fix-9-igpu-dgpunow fires at score 80 on a Raphael + RX 7900 XTX host, and the test asserts the note names gfx1100 as the discrete GPU and gfx1036 as the APU — a diagnosis that fires but confuses the two would be no better than silence.cargo test -p rocm-core --lib— 478 passed, 0 failedcargo test -p rocm --bin rocm— 970 passed, 0 failedcargo clippy --locked --workspace --all-targets -- -D warnings,cargo clippy --locked -p e2e-cucumber --test e2e -- -D warnings,cargo fmt --all --check,cargo xtask manifest --check— all exit 0On the Gherkin requirement
No scenario is added. The user-visible behaviours here — the withheld low-VRAM warning and
fix-9-igpu-dgpufiring — depend on the host's GPU, and neitherrocm examinenorrocm diagnosehas an injection point for a synthetic GPU topology. No CI lane has the hardware either: the Strix Halo lanes report gfx1151, which was already classified as integrated before this change, so a scenario there passes either way. Coverage sits one level down instead: the collision check runs through the realdiagnose()catalog, and the probe tests drive the real row-classification functions with reallspciand Windows display rows.Related
Name:lines overwrite the agent name #393 — its proposed fix ("deriveis_apufrom the target") would have regressed APU detection on the Raphael iGPU in its own reproduction until the predicate learned the pre-RDNA3 targets. This PR is that prerequisite, and it is correct both before and after examine: gfx_target is empty for every GPU even though rocminfo lists them → ISAName:lines overwrite the agent name #393 lands.blacklistamdgpuread as a blacklist directive), examine: lspci cannot name a GPU that pci.ids does not know, though the device id is on the line #507 (device-id table entries for Strix Halo and the gfx90c parts), examine: rocminfo agents are paired to lspci entries by index, which can mislabel both GPUs #508 (rocminfoagents paired to PCI rows by position), therock: Raven Ridge and Picasso APUs (gfx902, gfx909) are filed under a datacenter package family #519 (Raven Ridge and Picasso still reach the datacenter family through the KFD path).