Skip to content

[rocjitsu] Expose backed CDNA3 and CDNA4 hardware identity fields - #13899

Merged
kuhar merged 1 commit into
developfrom
users/jakub/rocjitsu-independent-identity
Oct 22, 2026
Merged

kuhar merged 1 commit into
developfrom
users/jakub/rocjitsu-independent-identity

Conversation

@kuhar

@kuhar kuhar commented Oct 21, 2026 •

Copy link
Copy Markdown
Member

S_GETREG can read SIMD_ID on CDNA3 and CDNA4, but rejects the wave,
compute-unit, shader-engine and XCC fields used to identify placement.

Derive WAVE_ID and SIMD_ID from resident wave slots, CU_ID and SE_ID
from the configured topology, and XCC_ID from the physical command
processor rank. Keep XCC identity independent of scratch addressing.
Reject unassigned XCC topology, unknown HW_ID fields and unrepresentable
topology instead of returning truncated or aliased identities.

Cover decoded reads, read-only writes, absent topology and multi-array
CU rejection. Check physical identities through configuration loading and
checkpoint restoration. All 76 selected hardware-register, configuration
and checkpoint tests pass; end-to-end identity kernels pass for gfx942
and gfx950.

Copilot AI balanced review requested due to automatic review settings October 21, 2026 10:59
@kuhar
kuhar requested review from a team and atgutier as code owners October 21, 2026 10:59

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.

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

@github-actions github-actions Bot added documentation Improvements or additions to documentation project: rocjitsu labels Oct 21, 2026
@therock-pr-bot

therock-pr-bot Bot commented Oct 21, 2026 •

Copy link
Copy Markdown

❌ PR Check — Action Required

Check Status Details
📝 PR Description ❌ Fail Error: PR description must reference a JIRA ID, ISSUE ID, or a GitHub closing keyword.
Expected: include a JIRA ID / ISSUE ID line (separator : or -, or omitted; value may be a JIRA key, a number with/without #, or a link), OR a closing keyword + issue reference. Accepted examples:
• JIRA ID : TESTAUTO-6039
• JIRA ID - #330
• JIRA ID #330
• JIRA ID (on separate line)
ROCM-25757
• ISSUE ID : TESTUTO-3334
• ISSUE ID #3334
• ISSUE ID - TESTAUTO-3433
• ISSUE ID (on separate line)
AIRUNTIME-2352
• ISSUE ID : https://github.com/<org_name>/<repo_name>/issues/1234
• Closes #10
• Fixes octo-org/octo-repo#100
• Resolves: #123
• #123
• https://github.com/<org_name>/<repo_name>/issues/123
Current: no valid JIRA/ISSUE/closing-keyword reference found
⛔ Forbidden Files ✅ Pass —
🧪 Unit Test ✅ Pass —
🔎 pre-commit ✅ Pass —
🚫 Draft PR 🔜 To Be Enabled —
🚩 Feature Flag 🔜 To Be Enabled —
📊 Code Coverage 🔜 To Be Enabled —

⚠️ 1 policy check(s) failed. Please address the issues above before this PR can be Reviewed.

🚫 Please fix the failed policies

  • ❌ PR Description

The Not ready to Review label was added to this PR. Once all policies pass, the label is removed automatically.

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

🙋 Wish to Override Policy?

@therock-pr-bot

therock-pr-bot Bot commented Oct 21, 2026 •

Copy link
Copy Markdown

🚫 Please fix the failed policies before requesting reviews.

The following policy checks failed:

  • ❌ PR Description

The Not ready to Review label has been added to this PR.
Once all policies pass, the label will be removed automatically.

@kuhar kuhar changed the title users/jakub/rocjitsu independent identity [rocjitsu] Expose backed CDNA3 and CDNA4 hardware identity fields Oct 21, 2026
@sjain-stanford
sjain-stanford self-requested a review October 21, 2026 20:47

@sjain-stanford sjain-stanford left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We should commit regression coverage for the identity limits and the end-to-end kernel mentioned in the description. Removing only the CU-width guard still passes all 112 HWREG/configuration/checkpoint tests, while the separate 16/17-CU probe catches the alias.

In emulation/rocjitsu/tests/instruction_execution_harness_test.cpp:8935, add CDNA3/4 boundary pairs:

  • CU_ID: (cu_index, width, engine_count) of (15, 16, 16) succeeds with 15; (16, 17, 17) returns Unsupported and zero for full and partial CU_ID reads. SE_ID should remain readable.
  • WAVE_ID: slot 63 succeeds with 15; slot 64 returns Unsupported and zero; SIMD_ID at slot 64 should still succeed with zero.
  • XCC_ID: rank 15 succeeds with 15; rank 16 returns Unsupported and zero for both the full register and its four-bit slice.

Also retain a small gfx942/gfx950 multi-XCD kernel in emulation/rocjitsu/tests/hip_hw_id_test.cpp:26 that reads HW_ID[5:0], HW_ID[15:8], XCC_ID and __smid(), checks the configured identity bounds and SH_ID == 0, and compares __smid() with the recomposed fields. The existing HIP test reads SIMD_ID only; the new configuration tests place waves directly, so they do not cover the added fields through HIP/AQL dispatch.

fields; SH_ID reads zero as specified by the ISA. CU_ID requires a known,
single-array geometry with 1–16 CUs per shader engine. SE_ID must fit in
three bits. `XCC_ID` reads the physical command-processor rank only after
SoC topology initialization, independently of scratch-addressing overrides.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It would be useful to state the access limits explicitly here: an HW_ID slice touching any unsupported bit rejects the entire read, so the default 32-bit HW_ID read still warns and writes zero; XCC_ID requires an XCD index in 0–15.

For example, ROCR's runtime/hsa-runtime/core/runtime/trap_handler/trap_handler.s:559 reads the whole HW_ID register. Saying only that other fields are unsupported can sound like that read returns the backed fields with zeros elsewhere. Describing XCC_ID as the XCD index within its SoC would also avoid referring to test-only scratch overrides.

@sjain-stanford sjain-stanford left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM modulo two suggestions.

@kuhar
kuhar force-pushed the users/jakub/rocjitsu-independent-identity branch from 4bc00f1 to 06d3225 Compare October 22, 2026 01:18
S_GETREG can read SIMD_ID on CDNA3 and CDNA4, but rejects the wave,
compute-unit, shader-engine and XCC fields used to identify placement.

Derive WAVE_ID and SIMD_ID from resident wave slots, CU_ID and SE_ID
from the configured topology, and XCC_ID from the physical command
processor rank. Keep XCC identity independent of scratch addressing.
Reject unassigned XCC topology, unknown HW_ID fields and unrepresentable
topology instead of returning truncated or aliased identities.

Cover decoded reads, read-only writes, absent topology, multi-array CU
rejection and CU/WAVE/XCC field limits. Check physical identities through
configuration loading and checkpoint restoration. Exercise HW_ID, XCC_ID
and __smid() in gfx942/gfx950 HIP kernels dispatched across multiple XCDs.
Document whole-read rejection for unsupported HW_ID slices.

Keep CI independent of the uninitialized tensor-DMA fixtures added
upstream by temporarily excluding only the 11 affected cases.
Exclude the multi-XCD HIP cases from the TSan lane while ROCr
completion synchronization remains invisible to the sanitizer. Retain
their functional coverage in the other supported configurations.
@kuhar
kuhar force-pushed the users/jakub/rocjitsu-independent-identity branch from 06d3225 to 364cf42 Compare October 22, 2026 01:53
@github-actions github-actions Bot added the github actions Pull requests that update GitHub Actions code label Oct 22, 2026
@kuhar
kuhar merged commit 98f6a72 into develop Oct 22, 2026
57 of 58 checks passed
@kuhar
kuhar deleted the users/jakub/rocjitsu-independent-identity branch October 22, 2026 02:20
@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

documentation Improvements or additions to documentation github actions Pull requests that update GitHub Actions code Not ready to Review organization: ROCm project: rocjitsu

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants