Skip to content

fix(rocjitsu): honor EXEC for lane-swap destinations - #12505

Merged
newling merged 2 commits into
developfrom
users/newling/fix-rocjitsu-masked-permlane
Oct 5, 2026
Merged

newling merged 2 commits into
developfrom
users/newling/fix-rocjitsu-masked-permlane

Conversation

@newling

@newling newling commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Motivation

rocJITsu currently lets lane-swap instructions overwrite inactive lanes. This corrupts address calculations in some hipBLASLt kernels and causes numerical failures during emulation.

In a wave64, each vector register holds 64 separate 32-bit values, one per lane. With all lanes enabled, v_permlane16_swap_b32_e32 v0, v1 exchanges these groups:

v1 lanes  0–15  ↔  v0 lanes 16–31
v1 lanes 32–47  ↔  v0 lanes 48–63

The lanes pair up in order: v1[0] swaps with v0[16], v1[1] with v0[17], and so on. Both registers are modified; the other halves retain their values. If both operands name the same register, this swaps neighboring 16-lane blocks within that register. The 32-lane variant exchanges v1[0–31] with v0[32–63].

EXEC has one bit per lane and controls which lanes may write results. This gfx950 example disables every lane before the swap:

s_mov_b64 exec, -1                  // Enable all 64 lanes.
v_mov_b32 v0, 1                     // Every lane: v0 = 1.
v_mov_b32 v1, 2                     // Every lane: v1 = 2.

s_mov_b64 exec, 0                   // Disable every lane.
v_permlane16_swap_b32_e32 v0, v1     // Must not write any lane.
s_mov_b64 exec, -1                  // Re-enable lanes for inspection.

Every lane should still have v0=1 and v1=2. Before this fix, the emulator changes v0 to 2 in lanes 16–31 and 48–63, and v1 to 1 in lanes 0–15 and 32–47, despite EXEC=0.

The CDNA4 ISA Reference Guide, 5 August 2025, §6.2.2, p. 33 states: “A thread only writes a result if the associated bit in the EXEC mask is set to 1.” The swap definitions in §12.8, pp. 218–219 and §12.11, pp. 337–338 specify both output writes without an EXEC override. Their pseudocode omits an explicit mask check; destination masking follows the general VALU output rule.

Technical Details

Add two checks in the shared lane-swap generator: each output write requires its destination lane's EXEC bit to be set. Preserve the snapshots of the original values so using the same register for both operands remains correct, and regenerate the affected CDNA4 and CDNA5 execution sources.

This repairs the hipBLASLt cases that exposed the bug: those kernels enable only some lanes while rearranging offsets into shared memory (LDS). The old emulator's swap also changed the disabled lanes' offsets. They now pass numerical validation with their original lane-swap instructions.

This change corrects destination writes. Inactive-source FI/BOUND_CTRL behavior and source-read observations remain unchanged; the regression does not prescribe the value of an active destination whose source peer is inactive.

Issue Tracking

JIRA ID: AIHPBLAS-3605

Test Plan

Check register values and observed writes for both encodings, supported widths, same and distinct register operands, and full, zero and partial EXEC masks. Run the standalone HIP probe, reduced hipBLASLt controls, adjacent native tests, generator tests and changed-file hooks.

Test Result

The standalone probe improves from 384 mismatched lane records out of 768 to zero. All 11 previously failing reduced hipBLASLt cases and four additional short-tail/edge cases pass. The separate known missing-wait reports remain detectable. Focused native tests, generator tests and hooks pass, with one generator test skipped. Validation used functional emulation; physical gfx950 validation has not been performed.

Submission Checklist

@therock-pr-bot

therock-pr-bot Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

✅ All Checks Passed — Ready for Review

Check Status Details
📝 PR Description ✅ Pass —
⛔ Forbidden Files ✅ Pass —
🧪 Unit Test ✅ Pass —
🔎 pre-commit ✅ Pass —
🚫 Draft PR 🔜 To Be Enabled —
🚩 Feature Flag 🔜 To Be Enabled —
📊 Code Coverage 🔜 To Be Enabled —
🤖 therock-pr-bot ✅ Pass —

🎉 All checks passed! This PR is ready for review.

📖 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 Sep 29, 2026 •

Copy link
Copy Markdown

🎉 All checks passed! This PR is ready for review.

@newling
newling marked this pull request as ready for review September 29, 2026 23:24
@newling
newling requested review from a team and atgutier as code owners September 29, 2026 23:24

@kuhar kuhar 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 apply the destination EXEC fix to the CDNA4-to-CDNA3 swap lowering too. In emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/code/dbt/semantic/cdna4_to_cdna3.cpp:767, it still explicitly treats swaps as EXEC-ignoring, and the final moves use full low/high group masks without intersecting saved_exec. For example, an e32 16-lane swap entered with only lane 0 enabled still overwrites inactive destination lane 16 after translation, unlike the corrected interpreter.

Please retain all-lane gathering, intersect each output mask with the saved EXEC, and add a partial-EXEC translation execution regression. BinaryTranslatorE2E.Cdna4ToCdna3Permlane16SwapWritesBothRowPairs currently checks only emitted mask literals, so it passes despite this mismatch.

@newling
newling force-pushed the users/newling/fix-rocjitsu-masked-permlane branch from 9ae62b6 to 2ba2585 Compare September 30, 2026 16:28
Masked lane swaps corrupted inactive VGPR lanes, including hipBLASLt LDS offset maps. Check each destination lane separately while retaining snapshots for aliased operands. Regenerate CDNA4/CDNA5 execution and cover masks, encodings, register aliases, and observed writes.
Intersect CDNA4-to-CDNA3 swap output groups with the saved EXEC mask. Use scalar pack and move instructions to preserve SCC without another temporary, retaining all-lane gathering and scratch restoration.

Replace mask-literal checks with translation execution coverage for both widths, register aliases, partial and zero EXEC, and preservation of EXEC, VCC and SCC.
@newling
newling force-pushed the users/newling/fix-rocjitsu-masked-permlane branch from 2ba2585 to 74f0751 Compare September 30, 2026 17:00
@newling

newling commented Sep 30, 2026

Copy link
Copy Markdown
Contributor Author

The CDNA4->CDNA3 lowering is a good spot, @kuhar I think it has been fixed in 2ba2585 now.

@newling
newling requested a review from kuhar September 30, 2026 17:02

@kuhar kuhar 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

@newling
newling merged commit 60a9e59 into develop Oct 5, 2026
91 of 92 checks passed
@newling
newling deleted the users/newling/fix-rocjitsu-masked-permlane branch October 5, 2026 17:51
@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.

2 participants