fix(hrr): regenerate the generated files and API matrix, and test they stay current by danielbolsson · Pull Request #13500 · ROCm/rocm-systems

danielbolsson · 2026-10-14T20:05:35Z

JIRA ID : AIRUNTIME-2937

Generated HRR files on develop were stale, which made Unit_HRR_ApiMatrix_T2_Roundtrip fail on a GPU (hipMallocHost/hipFreeHost expected NOOP, replay runs the real handler). The same drift left hipKernelSetAttributeForDevice uncaptured.

Regenerate hrr_api_args.h, the playback and capture shims, the matrix header and its baseline.
Add test_generated_files_current.py to hrr-generator-rules: it regenerates each file into a scratch dir and fails if the checked-in copy differs. The matrix tests that caught this are hidden, so the default suite did not.

Tested on gbt350 (MI350X, GPUs 4,5) against develop 461c501 plus this change: ApiMatrix_T0..T4 and ManifestWellFormed pass; default integration run 133 passed, 1 skipped; hrr-generator-rules 18/18. The new test fails on unmodified develop.

🤖 Generated with Claude Code

…y stay current The matrix header still listed hipMallocHost and hipFreeHost as NOOP after they became real handlers, so Unit_HRR_ApiMatrix_T2_Roundtrip failed on a GPU. CLR also gained hipKernelSetAttributeForDevice without the HRR generated files being rerun, so it was never captured, and the matrix baseline had drifted. Regenerate hrr_api_args.h, both generated shims, the matrix header and its baseline. A new unit test in hrr-generator-rules regenerates each into a scratch directory and fails when a checked-in copy differs, so the default suite now catches this drift; the matrix tests that did were hidden. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Signed-off-by: Daniel Olsson <daniel.olsson@amd.com>

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

therock-pr-bot · 2026-10-14T20:06:29Z

✅ All Policy Checks Passed

Check	Status	Details
📝 PR Description	✅ Pass	—
⛔ Forbidden Files	✅ Pass	—
🧪 Unit Test	✅ Pass	—
🚫 Draft PR	🔜 To Be Enabled	—
🚩 Feature Flag	🔜 To Be Enabled	—
📊 Code Coverage	🔜 To Be Enabled	—

🎉 All policy checks passed!

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

🙋 Wish to Override Policy?

andraghetti

The staleness test is a good guard. Two points inline; the first blocks. This also overlaps #13472 (same hipKernelSetAttributeForDevice slot, with the v9 bump), which I approved today. Could one land first and the other rebase on it?

andraghetti · 2026-10-14T20:07:47Z

-    HRR_API_HIPREGISTERVAR = 554,
-    HRR_API_HIPUNREGISTERFATBINARY = 555,
-    HRR_API_COUNT = 556
+    HRR_API_HIPKERNELSETATTRIBUTEFORDEVICE = 547,


This moves the nine compiler IDs up by one while HRR_VERSION stays 8, so a v8 archive recorded on develop decodes __hipRegisterFatBinary (549) as __hipPushCallConfiguration. I thought this needs the v9 bump, as in #13472?

andraghetti · 2026-10-14T20:07:47Z

  "observable_class.ERROR_STUB": 4,
-  "observable_class.NOOP": 200,
-  "observable_class.REAL": 340,
+  "observable_class.NOOP": 221,


I thought the 22 APIs #12785 refuses by name are unreplayable, not NOOP? classify_replay does not read PLAYBACK_ONLY_UNREPLAYABLE_APIS yet (the follow-up from #12785), so this baseline records them as NOOP. Fine if the follow-up updates it; could you confirm?

mangupta

🤖 AI-assisted pre-review (for @mangupta)

[SUGGESTION] Duplicates the hipKernelSetAttributeForDevice slot that the already-approved PR #13472 adds

This PR and #13472 both add hipKernelSetAttributeForDevice at ID 547 and regenerate the same generated files. Whichever merges second will conflict on hrr_api_args.h, both generated shims, the matrix header and the baseline. The regeneration may also be done twice with different version handling, since only #13472 includes the v9 bump.

Suggested fix: Land #13472 first, then rebase this PR so it only adds the staleness test and the matrix/baseline corrections. Rerun the generators on the rebased tree so the new test passes.

mangupta · 2026-10-15T03:08:25Z

+    HRR_API_HIPKERNELSETATTRIBUTEFORDEVICE = 547,
+    HRR_API_HIPPOPCALLCONFIGURATION = 548,
+    HRR_API_HIPPUSHCALLCONFIGURATION = 549,
+    HRR_API_HIPREGISTERFATBINARY = 550,
+    HRR_API_HIPREGISTERFUNCTION = 551,
+    HRR_API_HIPREGISTERMANAGEDVAR = 552,
+    HRR_API_HIPREGISTERSURFACE = 553,
+    HRR_API_HIPREGISTERTEXTURE = 554,
+    HRR_API_HIPREGISTERVAR = 555,
+    HRR_API_HIPUNREGISTERFATBINARY = 556,
+    HRR_API_COUNT = 557


🤖 AI-assisted pre-review (for @mangupta)

[BLOCKING] New API ID inserted before compiler IDs shifts on-disk IDs without a format version bump

The change inserts HRR_API_HIPKERNELSETATTRIBUTEFORDEVICE at 547. That moves all nine compiler-table IDs up by one: __hipPopCallConfiguration goes from 547 to 548, and so on through __hipUnregisterFatBinary, which goes from 555 to 556. HRR_API_COUNT goes from 556 to 557. The archive format version is not changed. These IDs are written to capture archives by hrr_cap::writer::write_event_raw and used to index hrr_playback_dispatch and hrr_api_min_payload_size during replay. An archive recorded before this change will therefore be decoded with the wrong handler and the wrong minimum payload size for every compiler-table event.

Suggested fix: Bump the HRR archive format version, for example HRR_VERSION 8 to 9 as in #13472, together with the regenerated enum, so old archives are rejected or translated instead of being misdecoded. The alternative is to have the generator give new runtime APIs IDs that do not renumber existing ones. Coordinate with #13472 so only one of the two PRs claims the slot and the bump, and rebase the other on it.

mangupta · 2026-10-15T03:08:25Z

  "observable_class.ERROR_STUB": 4,
-  "observable_class.NOOP": 200,
-  "observable_class.REAL": 340,
+  "observable_class.NOOP": 221,
+  "observable_class.REAL": 320,
  "observable_class.UNREPLAYABLE": 12,
  "payload_loss.BIG_STRUCT_BY_VALUE": 8,
  "payload_loss.CONST_STRUCT_PTR_IN": 44,
  "payload_loss.any": 52,
-  "payload_loss.in_faithful": 33,
+  "payload_loss.in_faithful": 24,
  "replay_class.COMPILER_NOOP": 7,
  "replay_class.CUSTOM": 1,
  "replay_class.ERROR_STUB": 4,
-  "replay_class.GENERATED": 228,
-  "replay_class.MANUAL": 98,
-  "replay_class.NOOP": 200,
+  "replay_class.GENERATED": 205,
+  "replay_class.MANUAL": 101,
+  "replay_class.NOOP": 221,
  "replay_class.SPECIAL": 6,


🤖 AI-assisted pre-review (for @mangupta)

[IMPORTANT] Regenerated matrix and baseline record about 20 APIs that playback refuses by name as NOOP

The regenerated baseline makes the matrix more permissive: NOOP rises from 200 to 221, REAL falls from 340 to 320, and in_faithful falls from 33 to 24. In hrr_api_matrix_expectations.h, about 20 existing APIs flip from 0 to 1 in the column that this PR uses as the NOOP flag (see the hipMallocHost/hipFreeHost fix). These include the hipExternalMemory and hipExternalSemaphore APIs, hipExecutionCtx*, hipGreenCtxCreate, hipLibraryGetGlobal, hipLibraryGetManaged and hipGraphicsMapped. These look like the APIs that #12785 made playback refuse as unreplayable. They are recorded as NOOP only because classify_replay does not read PLAYBACK_ONLY_UNREPLAYABLE_APIS yet. The new staleness test then locks this misclassification in as the expected output.

Suggested fix: Before regenerating, make classify_replay read PLAYBACK_ONLY_UNREPLAYABLE_APIS so these APIs are classified UNREPLAYABLE. Otherwise, keep their previous expectations and document that the follow-up to #12785 will update the baseline. Check Unit_HRR_ApiMatrix_* on a GPU against the regenerated header before merging.

mangupta · 2026-10-15T03:08:25Z

+    import yaml  # noqa: F401
+    HAVE_YAML = True
+except ImportError:
+    HAVE_YAML = False
+


🤖 AI-assisted pre-review (for @mangupta)

[SUGGESTION] Matrix staleness check silently skips when PyYAML is missing, so the drift this PR fixes can recur unnoticed

test_api_matrix_header_matches_the_derived_manifest is decorated with skipUnless(HAVE_YAML). The drift this PR fixes is the hipMallocHost/hipFreeHost NOOP mismatch in hrr_api_matrix_expectations.h. If the default CI environment has no PyYAML, that check is skipped and the suite still passes, so the same matrix drift would go undetected again. Only the gen_hrr_api_args.py half would still be enforced.

Suggested fix: Ensure the CI job that runs hrr-generator-rules installs PyYAML. Alternatively, make the skip fail when a CI environment variable is set, so it cannot silently pass there.

…ss guard Carry PR #13500's content over the #13472 tree: - tests/integration/hrr_api_matrix_expectations.h regenerated with derive_manifest.py + check_matrix.py --emit-cxx (48 corrected expectation lines; hipMallocHost/hipFreeHost expect=0 REAL since MANUAL_PLAYBACK_APIS replays them by hand) - api_classes_baseline.json re-derived with derive_manifest.py --update-baseline (observable NOOP 201->221, REAL 340->320, etc.) - tools/tests/test_generated_files_current.py from #13500: regenerates generated files into a scratch dir and fails if checked-in copies drift Verified: generators are byte-identical to both PRs' committed content; hrr_check_sources.py all --base-sha origin/develop passes.

danielbolsson · 2026-10-15T09:09:43Z

Superseded by #13472, which now carries this PR's content: the regenerated API matrix expectations and api_classes_baseline.json, plus the test_generated_files_current.py staleness guard, on top of the hipKernelSetAttributeForDevice regeneration and the HRR_VERSION 8 -> 9 bump. Nothing from this branch is left behind; closing it.

…ss guard Carry PR #13500's content over the #13472 tree: - tests/integration/hrr_api_matrix_expectations.h regenerated with derive_manifest.py + check_matrix.py --emit-cxx (48 corrected expectation lines; hipMallocHost/hipFreeHost expect=0 REAL since MANUAL_PLAYBACK_APIS replays them by hand) - api_classes_baseline.json re-derived with derive_manifest.py --update-baseline (observable NOOP 201->221, REAL 340->320, etc.) - tools/tests/test_generated_files_current.py from #13500: regenerates generated files into a scratch dir and fails if checked-in copies drift Verified: generators are byte-identical to both PRs' committed content; hrr_check_sources.py all --base-sha origin/develop passes.

Copilot AI balanced review requested due to automatic review settings October 14, 2026 20:05

danielbolsson requested review from a team as code owners October 14, 2026 20:05

Copilot AI reviewed Oct 14, 2026

github-actions Bot added project: clr project: hrr labels Oct 14, 2026

andraghetti reviewed Oct 14, 2026

View reviewed changes

systems-assistant Bot added the organization: ROCm label Oct 14, 2026

mangupta reviewed Oct 15, 2026

View reviewed changes

mangupta added the lrt-bot: ready label Oct 15, 2026

danielbolsson mentioned this pull request Oct 15, 2026

fix(hrr): regenerate sources for hipKernelSetAttributeForDevice #13472

Merged

1 task done

danielbolsson closed this Oct 15, 2026

mangupta removed the lrt-bot: ready label Oct 15, 2026

Navigation Menu

Repository navigation

fix(hrr): regenerate the generated files and API matrix, and test they stay current - #13500

fix(hrr): regenerate the generated files and API matrix, and test they stay current#13500
danielbolsson wants to merge 1 commit into
developfrom
users/danielbolsson/hrr-matrix-hostalloc-handlers

danielbolsson commented Oct 14, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

therock-pr-bot Bot commented Oct 14, 2026

Uh oh!

andraghetti left a comment

Uh oh!

andraghetti Oct 14, 2026

Uh oh!

andraghetti Oct 14, 2026

Uh oh!

mangupta left a comment

Uh oh!

mangupta Oct 15, 2026

Uh oh!

mangupta Oct 15, 2026

Uh oh!

mangupta Oct 15, 2026

Uh oh!

danielbolsson commented Oct 15, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

danielbolsson commented Oct 14, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

therock-pr-bot Bot commented Oct 14, 2026

✅ All Policy Checks Passed

Uh oh!

andraghetti left a comment

Choose a reason for hiding this comment

Uh oh!

andraghetti Oct 14, 2026

Choose a reason for hiding this comment

Uh oh!

andraghetti Oct 14, 2026

Choose a reason for hiding this comment

Uh oh!

mangupta left a comment

Choose a reason for hiding this comment

Uh oh!

mangupta Oct 15, 2026

Choose a reason for hiding this comment

Uh oh!

mangupta Oct 15, 2026

Choose a reason for hiding this comment

Uh oh!

mangupta Oct 15, 2026

Choose a reason for hiding this comment

Uh oh!

danielbolsson commented Oct 15, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants