Skip to content

[rocjitsu] Add support for packed V_PK_LSHL_ADD_U64 execution. - #10665

Merged
hanhanW merged 3 commits into
ROCm:developfrom
hanhanW:users/hanhanW/gfx1251-execution
Sep 2, 2026
Merged

hanhanW merged 3 commits into
ROCm:developfrom
hanhanW:users/hanhanW/gfx1251-execution

Conversation

@hanhanW

@hanhanW hanhanW commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Teach the semantics derivation and packed code generator that V_PK_LSHL_ADD_U64 operates on two independent U64 values and addends with two U32 shift counts. Generate operand helpers for the consecutive register layout, generate operand helpers for consecutive SGPR/VGPR layouts and replicate non-register selectors across both packed elements, and install the CDNA5 execution callback.

This fills the instruction's execution gap while following the public LLVM operand profile and ThreeOpFrag<shl_0_to_4, add> selection pattern. The instruction does not consume clamp or source modifiers. Public sources only define shift counts from 0 through 4; lshl_masked keeps unsupported host inputs well-defined but does not establish gfx1251 semantics outside that range.

Add generator and instruction-level integration tests covering public source forms, wraparound, EXEC masking, operand order, register layout, and legal source/destination overlap. Keep LLVM's literal-101 vector as decode-only coverage because the available public sources do not define its arithmetic result.

This enables only V_PK_LSHL_ADD_U64. The gfx1251 execution capability remains unavailable, normal simulator configuration still rejects gfx1251, and the tests do not validate behavior on physical gfx1251 hardware.

Submission Checklist

@hanhanW
hanhanW requested a review from kuhar August 24, 2026 22:07
@hanhanW

hanhanW commented Aug 24, 2026

Copy link
Copy Markdown
Contributor Author

It depends on #10661

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

Added some comments

@hanhanW
hanhanW force-pushed the users/hanhanW/diff-cdna5-isa-variants branch from c6800f3 to 4e0c0de Compare August 25, 2026 22:44
@hanhanW
hanhanW force-pushed the users/hanhanW/gfx1251-execution branch from bf032dc to b5c63c3 Compare August 25, 2026 22:50
@hanhanW
hanhanW force-pushed the users/hanhanW/diff-cdna5-isa-variants branch from 4e0c0de to 5968875 Compare August 27, 2026 00:47
@hanhanW
hanhanW force-pushed the users/hanhanW/gfx1251-execution branch from b5c63c3 to 5fa58d1 Compare August 27, 2026 00:47
@hanhanW
hanhanW requested a review from kuhar August 27, 2026 00:57
@hanhanW
hanhanW marked this pull request as ready for review August 27, 2026 00:58
@hanhanW
hanhanW requested review from a team and atgutier as code owners August 27, 2026 00:58
@hanhanW
hanhanW requested a review from amd-eochoalo August 27, 2026 16:19
@hanhanW

hanhanW commented Aug 27, 2026

Copy link
Copy Markdown
Contributor Author

@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 % one comment

Teach the semantics derivation and packed code generator that
`V_PK_LSHL_ADD_U64` operates on two U64 values and two U32 shift counts.
Generate operand helpers that read two U64 elements from four consecutive
VGPRs, broadcast the low U64 value for SGPR and non-register sources, read two
U32 shift counts, and write two U64 results. Install the CDNA5 execution
callback for this instruction.

This fills the instruction's execution gap while following the public LLVM
operand profile and `ThreeOpFrag<shl_0_to_4, add>` selection pattern. The
instruction does not consume clamp or source modifiers. Public sources only
define shift counts from 0 through 4; `lshl_masked` keeps unsupported host
inputs well-defined but does not establish gfx1251 semantics outside that
range.

Add generator and instruction-level integration tests covering public source
forms, SGPR broadcast behavior, wraparound, EXEC masking, operand order,
register layout, and legal source/destination overlap. Keep LLVM's literal-101
vector as decode-only coverage because the available public sources do not
define its arithmetic result.

This enables only `V_PK_LSHL_ADD_U64`. The gfx1251 execution capability remains
unavailable, normal simulator configuration still rejects gfx1251, and the
tests do not validate behavior on physical gfx1251 hardware.

- [x] Look over the contributing guidelines at
  https://github.com/ROCm/rocm-systems/blob/develop/CONTRIBUTING.md.

Signed-off-by: hanhanW <hanhan0912@gmail.com>
Signed-off-by: hanhanW <hanhan0912@gmail.com>
@hanhanW
hanhanW changed the base branch from users/hanhanW/diff-cdna5-isa-variants to develop September 1, 2026 21:13
Copilot AI lite review requested due to automatic review settings September 1, 2026 21:14
@hanhanW
hanhanW force-pushed the users/hanhanW/gfx1251-execution branch from 5fa58d1 to bc62b32 Compare September 1, 2026 21:14
@therock-pr-bot

therock-pr-bot Bot commented Sep 1, 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 Sep 1, 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.

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.

Pull request overview

This PR adds execution support for the CDNA5 packed instruction V_PK_LSHL_ADD_U64 across the semantics derivation pipeline, generated execution backend, and simulator tests, while keeping unsupported shift-count semantics fail-closed and explicitly tracked as a non-architectural simulator error.

Changes:

  • Implements generated execution for VPkLshlAddU64Vop3p (including packed operand helpers) and wires it into the CDNA5 execution backend callback table.
  • Adds a wavefront-scoped InstructionExecutionError mechanism and propagates fail-closed execution errors to the CU scheduler (halt + warning).
  • Expands C++ and Python test coverage for semantics derivation, packed codegen, and instruction execution across public LLVM source forms.

Reviewed changes

Copilot reviewed 12 out of 17 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
shared/machine-readable-isa/isa/amdgpu_isa_cdna5_variants.json Removes V_PK_LSHL_ADD_U64 from the model_only_instructions list while keeping it feature-gated under packed_u64_single_sgpr_ops.
emulation/rocjitsu/tests/cdna5_execution_test.cpp Adds packed-U64 execution tests (active-lane behavior, shift range rejection, form coverage, overlap, VCC/EXEC shifts) plus test helpers.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/vm/amdgpu/wavefront.h Introduces InstructionExecutionError state on Wavefront with report/query/clear helpers and reset integration.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/vm/amdgpu/compute_unit.h Clears instruction_execution_error at the start of each direct instruction execution.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/vm/amdgpu/compute_unit.cpp Halts a wave (with warning) when an instruction reports an execution error.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/cdna5/vop3p.h Declares execute_impl for VPkLshlAddU64Vop3p.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/cdna5/vop3p.cpp Wires VPkLshlAddU64Vop3p to selected_exec_fn(...) rather than a null execute pointer.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/cdna5/vop3p_exec.cpp Adds packed U64/U32 operand helpers and implements lane-masked packed shift-left-add execution with fail-closed shift validation.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/cdna5/execution_backend.h Adds InstructionExecutionId::VPkLshlAddU64Vop3p.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/cdna5/execution_backend_exec.cpp Registers the callback entry for VPkLshlAddU64Vop3p.
emulation/rocjitsu/lib/python/amdisa/tests/test_sema_derive.py Adds semantics-derivation test for V_PK_LSHL_ADD_U64.
emulation/rocjitsu/lib/python/amdisa/tests/test_packed_codegen.py Adds codegen tests asserting correct helper usage, fail-closed ordering, and lack of clamp/modifier consumption.
emulation/rocjitsu/lib/python/amdisa/tests/test_generator_profile_gates.py Updates generator gating tests to expect VPkLshlAddU64Vop3p to be executable and to validate helper emission.
emulation/rocjitsu/lib/python/amdisa/semantics.py Adds _derive_vop3p mapping for V_PK_LSHL_ADD_U64 → pk_lshl_add_u64.
emulation/rocjitsu/lib/python/amdisa/codegen/execute/packed.py Adds gen_pk_lshl_add_u64 generator implementing per-lane packed U64 shift+add with validation.
emulation/rocjitsu/lib/python/amdisa/codegen/execute/init.py Registers the new packed-U64 generator under pk_lshl_add_u64.
emulation/rocjitsu/lib/python/amdisa/codegen/_generator.py Emits new CDNA5 packed-U64 helper structs/functions into generated execution support.

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

Record a nonzero SimulationEngine exit before resetting a wavefront that rejects an unsupported operand value. Retire the wave through the normal workgroup completion path so the command processor cannot retain the dispatch after its final wave is gone.

Exercise the scheduler path with the public gfx1251 literal-101 encoding and verify both the simulator failure and AQL completion signal. Keep gfx1251 full-simulation capability disabled.

Signed-off-by: hanhanW <hanhan0912@gmail.com>
@hanhanW
hanhanW merged commit 5bdcd35 into ROCm:develop Sep 2, 2026
28 of 29 checks passed
@hanhanW
hanhanW deleted the users/hanhanW/gfx1251-execution branch September 2, 2026 00:12
@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.

3 participants