[rominf]

Follows up #558.

The review on #558 noted that GFX_TARGETS in examine_driver_proptests.rs had no gfx125x entry. So the gfx125* arm of normalize_therock_family, which maps to gfx125X-dcgpu, was reached only by its single example test, not by a_reported_target_normalizes_to_a_known_family.

This adds gfx1250, the gfx125x target the detection code already produces from a 12.5.0 GC version. The property now checks that a host reporting it, in any spelling the strategy generates (upper case, :xnack- suffix, trailing whitespace), normalizes to a known TheRock family, and that normalizing that family again changes nothing. The list's doc comment now says it keeps at least one target per raw-target arm.

Verification

• Deleting the gfx125* arm makes the property fail with minimal input "gfx1250". Without this entry the same mutation passes it.
• The property checks only that the family is a known one, not which one. Remapping the arm to another known family is caught by normalize_therock_family_maps_gfx1250_to_gfx125x_dcgpu and known_therock_families_all_round_trip instead.
• cargo fmt --all --check, cargo test --locked -p rocm-core --lib, cargo clippy --workspace --all-targets -- -D warnings (with and without --all-features), and a Windows-target cargo check -D warnings all pass locally.

No e2e scenario: this is a test-only change with nothing a CLI user can observe.

[siloteemu]

🔴 Automated review · pr-review-watcher · 7e8858d

This automation never files a GitHub approval, so no approving review will appear here whatever the outcome — the merge decision stays with a human reviewer.

Review — no blocking findings

Full review of the whole change: prw-base...7e8858d8, one commit, one file.
The description says this follows up #558 and adds a gfx1250 entry to the property-test target list. The change does exactly that.

Blocking

None

Non-blocking

None

Decisions for the author

• The new doc comment states an invariant that nothing enforces — non-blocking-improvement
  crates/rocm-core/src/examine_driver_proptests.rs:34-35 says the list holds "at least one per raw-target arm of normalize_therock_family, so the family property reaches every arm". That is true today: I checked every raw-target arm against the list. A future raw-target arm added without a matching list entry would make the comment false with no test failing. Reach is also probabilistic. Each entry is drawn in 256 cases by default, so a miss is very unlikely, but a low PROPTEST_CASES weakens it. inputs_reach_every_region (line 251 onward) checks only suffix and upper-case reach, not per-arm reach. Consider softening the wording, or leave it as is and treat it as a convention. It is not a defect in this change.

Positive signals

• The new entry is the gfx125x target the detection code already produces: gfx_target_from_gc_version(12, 5, 0) gives gfx1250, asserted at crates/rocm-core/src/host_gpu.rs:3463-3466. It is therefore a realistic input, not an invented one.
• The PR text states honestly what the property does not check. It asserts only that the family is a known one, and which family is covered by the example test and the round-trip test.

Deployment notes

None

What this covered

Every changed line was read, and so was the whole of crates/rocm-core/src/examine_driver_proptests.rs. I also read normalize_therock_family, known_therock_families and the gfx-target detection code in crates/rocm-core/src/host_gpu.rs, AGENTS.md, and the history of the touched region. The review was done by two independent read-only workers (code and adherence, history) plus my own targeted run.

Test verdicts for the one test the change affects, a_reported_target_normalizes_to_a_known_family:

• Would it pass with the production change reverted? This is a test-only change, so there is no production change to revert. The equivalent question is whether the new entry exercises the arm it names. I ran it. With the gfx125* arm deleted from normalize_therock_family, the property fails with minimal input "gfx1250". With that arm deleted and the entry also removed, the property passes. The new entry therefore covers the arm, and the PR text's claim holds.
• What stays green if one branch is broken alone? Remapping the gfx125* arm to another known family would not fail this property, because it checks only membership. As the description says, normalize_therock_family_maps_gfx1250_to_gfx125x_dcgpu (host_gpu.rs:3320) and known_therock_families_all_round_trip (host_gpu.rs:3328) catch that case. I did not run that mutation separately.

Ran: cargo test -p rocm-core --lib driver_proptests on the head (6 passed, 1 ignored, the pre-existing #506 defect marker), plus the mutation above. I did not run fmt, clippy or the full workspace, because the CI rollup covers them.

CI rollup on this head, queried once: every check that reported succeeded, including clippy, build-and-test, windows-build-and-test, prek, the commit signatures + sign-off check and the E2E lanes. "Skill checks (skillscope)" and "Sphinx docs build (-W)" show as skipped, not failed. I did not read their workflow definitions. A skip is plausible for a change that touches no skill or docs files, but I did not verify why.

Scenario requirement (AGENTS.md section 3): the change is test-only with no CLI-observable behaviour, and the PR text says so. No Gherkin scenario is owed.
Identity: the commit carries a Signed-off-by line and no internal references. Signature verification could not be done locally (no allowed-signers file in this checkout), so I rely on the passing CI signature check.

Did not run: the prior-changes pass, because I was told not to read PR comments and reviews. Not reconciled against any discussion here.

[jussielo-amd]

Reviewed the diff against origin/main (merge-base, three-dot): a single 4-line change in crates/rocm-core/src/examine_driver_proptests.rs adding gfx1250 to GFX_TARGETS (line 37) and a doc comment on the array (lines 33-34) stating the new invariant that the list carries at least one example per raw-target arm of normalize_therock_family.

I independently verified the claims rather than taking them on trust. gfx1250 is the real target the detection code produces for a 12.5.0 GC version (crates/rocm-core/src/host_gpu.rs:3463-3466), and it is the only example that reaches the gfx125* arm (host_gpu.rs:130), which maps to gfx125X-dcgpu. I ran the mutation locally: deleting that arm from normalize_therock_family makes a_reported_target_normalizes_to_a_known_family (examine_driver_proptests.rs:216) fail immediately with minimal input "gfx1250", confirming the new entry is what gives the property reach into that arm; with the arm restored, cargo test -p rocm-core --lib driver_proptests is green (6 passed, 1 pre-existing #506 ignore, unrelated). This is a test-only change with no CLI-observable behavior, so no Gherkin scenario is owed per AGENTS.md section 3. Ran a leak scan over the diff (origin/main..HEAD) — clean.

This PR already carries an automated pr-review-watcher review round (COMMENTED, current against this same head 7e8858d) that reached the same conclusion with no blocking findings; that automation never files an approval, so I'm adding one here since my own review, including the mutation re-verification above, independently reaches the same result. Approving.