Skip to content

fix(hrr): Load COMGR before collecting version - #14133

Merged
LeoZhao-BJ merged 2 commits into
developfrom
users/liangzh/ROCM-33422
Oct 26, 2026
Merged

LeoZhao-BJ merged 2 commits into
developfrom
users/liangzh/ROCM-33422

Conversation

@LeoZhao-BJ

@LeoZhao-BJ LeoZhao-BJ commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Motivation

Fix HRR capture metadata collection to initialize COMGR before querying its version.

Technical Details

collect_comgr_version() now uses the existing amd::Comgr loader path and checks that COMGR is ready before calling amd::Comgr::get_version(). When COMGR cannot be loaded, capture metadata records an empty comgr_version instead of invoking an unresolved COMGR entry point.

Issue Tracking

JIRA ID: ROCM-33422

Test Plan

no change on tests

Test Result

pass all hrr basic cases

Submission Checklist

@LeoZhao-BJ
LeoZhao-BJ requested review from a team as code owners October 23, 2026 06:05
Copilot AI balanced review requested due to automatic review settings October 23, 2026 06:05
@therock-pr-bot

Copy link
Copy Markdown

✅ All Policy Checks Passed

Check Status Details
📝 PR Description ✅ Pass —
⛔ Forbidden Files ✅ Pass —
🧪 Unit Test ⚠️ Warning Error: Source/code files changed without an accompanying unit test.
Expected: add at least one test file named like test_<name>.py / test_<name>.cpp (or <name>_test.*).
Current: code file(s) changed: projects/clr/hipamd/src/hrr/hip_capture_metadata.cpp; no test file found
🚫 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 narrow change follows the existing COMGR initialization pattern and guards the version query, with no unresolved findings.

No open issues 🎉

What changed in this PR

This PR fixes COMGR version collection in HRR capture metadata by loading COMGR before querying its version.

Changes:

  • Uses the existing amd::Comgr loader and version-query wrapper.
  • Records an empty comgr_version when COMGR is unavailable, avoiding a call to an unresolved entry point.
File Description
projects/​clr/​hipamd/​src/​hrr/​hip_capture_metadata.cpp Initializes COMGR once and checks readiness before collecting its version.

Review effort: Balanced

@andraghetti andraghetti 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.

Ran on MI350: no new failures; hrr-generator-rules fails the same way on develop a08bc31.

@mangupta mangupta 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.

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

@LeoZhao-BJ
LeoZhao-BJ merged commit 7f7042a into develop Oct 26, 2026
602 of 616 checks passed
@LeoZhao-BJ
LeoZhao-BJ deleted the users/liangzh/ROCM-33422 branch October 26, 2026 04:55
@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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants