Skip to content

fix(rocprofiler-sdk): handle HSA execute permissions in formatter - #13286

Merged
venkat1361 merged 2 commits into
developfrom
users/venkat1361/hsa-formatter-fix
Oct 12, 2026
Merged

venkat1361 merged 2 commits into
developfrom
users/venkat1361/hsa-formatter-fix

Conversation

@venkat1361

Copy link
Copy Markdown
Contributor

Motivation

https://github.com/ROCm/rocm-systems/actions/runs/38259799910/job/114847245719?pr=12889

ROCr PR #13129 added HSA_ACCESS_PERMISSION_EX to hsa_access_permission_t. The rocprofiler-sdk permission formatter does not handle this value, causing builds with newer HSA headers to fail when warnings are treated as errors:

error: enumeration value ‘HSA_ACCESS_PERMISSION_EX’ not handled in switch [-Werror=switch]

Technical Details

Update the HSA access-permission formatter to:

  • Handle execute permission and its combinations: EXECUTE, READ_EXECUTE, WRITE_EXECUTE, and READ_WRITE_EXECUTE.
  • Switch on the integer bitmask and define the execute bit locally, preserving compatibility with older HSA headers that lack HSA_ACCESS_PERMISSION_EX.
  • Preserve existing labels for none, read-only, write-only, and read/write access.
  • Change the fallback label from NONE to UNKNOWN.

Issue Tracking

Test Plan

CI passes.

Test Result

Submission Checklist

Copilot AI balanced review requested due to automatic review settings October 12, 2026 16:48
@venkat1361
venkat1361 requested review from a team as code owners October 12, 2026 16:48
@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/rocprofiler-sdk/source/lib/rocprofiler-sdk/hsa/utils.hpp; 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 formatter handles all valid permission combinations while remaining compatible with older HSA headers.

0 open findings

What changed in this PR

Updates rocprofiler-sdk’s HSA permission formatter for newer ROCr headers while retaining compatibility with older headers.

Changes:

  • Formats execute permission and all read/write combinations.
  • Reports unsupported bitmasks as UNKNOWN.
File Description
projects/​rocprofiler-sdk/​source/​lib/​rocprofiler-sdk/​hsa/​utils.hpp Adds execute-bit formatting through an integer bitmask switch.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@venkat1361
venkat1361 merged commit b4b7790 into develop Oct 12, 2026
249 of 259 checks passed
@venkat1361
venkat1361 deleted the users/venkat1361/hsa-formatter-fix branch October 12, 2026 19:43
@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.

4 participants