Skip to content

docs(hrr): match api matrix workload comments to the expectations header - #14135

Merged
andraghetti merged 1 commit into
developfrom
users/andraghetti/hrr-api-matrix-comments
Oct 26, 2026
Merged

andraghetti merged 1 commit into
developfrom
users/andraghetti/hrr-api-matrix-comments

Conversation

@andraghetti

Copy link
Copy Markdown
Contributor

Motivation

Many comments in hrr_api_matrix_workload_test.cc contradict hrr_api_matrix_expectations.h and the capture code. They describe replay crashes the header no longer expects, calls the file never makes, and replay classes the header does not assign. A reader who uses the tests to learn how HRR handles an API is misled.

Technical Details

  • Comments only. No code, test or expectation line changes. git diff -U0 a08bc31f04 | grep '^[-+]' | grep -v '^[-+][-+]' | grep -v '^[-+] *//' prints nothing.
  • The crash comments on hipSetupArgument, hipGetProcAddress_spt, the _spt launches, hipCtx* and the shareable-handle exports now give the class from the header.
  • The hipLaunchByPtr, cooperative-barrier and "recorded either way" comments are corrected or removed. Capture records the IPC open and the external memory and semaphore imports only on hipSuccess.
  • The T1 and T2 headers name the classes their rows have, without counts.

Issue Tracking

JIRA ID: AIRUNTIME-3052

Test Plan

None. Only comments change, so the compiled code is the same as on develop. The PR carries ci:skip.

Test Result

Not applicable.

Submission Checklist

Many comments in hrr_api_matrix_workload_test.cc contradicted
hrr_api_matrix_expectations.h and the capture code: they described a
hipLaunchByPtr call the file never makes, replay segfaults on APIs the
header classes REAL, NOOP classes for the hipCtx* and shareable-handle
APIs that the header classes otherwise, a grid-wide barrier the
cooperative kernels do not have, and failed imports that capture does
not record. Rewrite each to state the header's class or the capture
behaviour, or delete it. Comments only.

Jira: AIRUNTIME-3052
@andraghetti
andraghetti requested a review from a team as a code owner October 23, 2026 09:10
Copilot AI balanced review requested due to automatic review settings October 23, 2026 09:10
@andraghetti andraghetti added the ci:skip Skip all CI builds/tests for this PR label Oct 23, 2026
@therock-pr-bot

Copy link
Copy Markdown

✅ All Policy Checks Passed

Check Status Details
📝 PR Description ✅ Pass —
⛔ Forbidden Files ✅ Pass —
🧪 Unit Test ✅ Pass PR does not contain code files — Unit Test auto-passed
🚫 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?

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The changes affect only comments, match the checked expectations and implementations, and have no unresolved findings.

No open issues 🎉

What changed in this PR

This comment-only PR aligns the HRR API matrix workload documentation with the expectations header and capture/replay behavior. Executable code and test expectations are unchanged.

Changes:

  • Replaces outdated replay-crash claims with current replay classifications.
  • Clarifies success-only capture behavior and removes descriptions of calls or barriers the workloads do not execute.
  • Revises the T1 and T2 headers to reflect their mixed replay classes.
File Description
projects/​hrr/​tests/​integration/​hrr_api_matrix_workload_test.cc Corrects workload comments to match API expectations and capture/replay behavior.

Review effort: Balanced

@danielbolsson danielbolsson left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checked 8606a18: comments only (the PR's own git diff -U0 | grep -v '//' check is empty) and the classes the new comments state match hrr_api_matrix_expectations.h. Nothing blocking.

@andraghetti
andraghetti merged commit 4d8c33e into develop Oct 26, 2026
53 of 61 checks passed
@andraghetti
andraghetti deleted the users/andraghetti/hrr-api-matrix-comments branch October 26, 2026 12:32
@systems-assistant

Copy link
Copy Markdown
Contributor

TheRock Submodule Bump Activity

Newest first

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci:skip Skip all CI builds/tests for this PR organization: ROCm project: hrr

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants