[rocjitsu] Execute packed FP64 basic arithmetic. by hanhanW · Pull Request #10871 · ROCm/rocm-systems

hanhanW · 2026-08-28T04:38:29Z

Enable instruction-level execution for gfx1251 V_PK_ADD_F64, V_PK_MUL_F64, V_PK_MAX_NUM_F64, and V_PK_MIN_NUM_F64. These instructions were previously model-only, so valid decoded instructions could not produce architectural register results.

Generate callbacks that route both packed elements through the shared MODE-aware F64 helpers. This honors rounding and denormal modes, EXEC masking, independent low/high source negation, clamp, packed-FP64 scalar propagation, literals, inline constants, and overlapping register tuples.

Validate the fixed packed layout and public LLVM width-specific source and register classes before installing each callback. Tests exercise the public LLVM encodings and source forms, boundary selectors and tuples, FP modes, NaN and signed-zero behavior, modifiers, inactive lanes, aliasing, and decoder rejection paths.

Keep V_PK_FMA_F64 and V_WMMA_F64_16X16X4_F64 model-only. gfx1251 still does not advertise full simulator execution, so this change provides decode-to-register execution rather than simulator or AQL end-to-end support.

hanhanW · 2026-08-28T04:39:04Z

It depends on

kuhar

Just 2 suggestions

therock-pr-bot · 2026-09-15T16:47:37Z

❌ 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 · 2026-09-15T16:47:39Z

🚫 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

🟡 Changes recommended

Unresolved decoder-generation validation issues and corresponding test coverage remain.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Enables gfx1251 decode-to-register execution for four packed FP64 arithmetic instructions while keeping FMA and WMMA model-only.

Changes:

Adds MODE-aware FP64 helpers and generated execution callbacks.
Updates CDNA5 metadata, decoder validation, and backend registration.
Expands execution, modifier, aliasing, and rejection-path tests.

Review findings:

Moderate (2 votes): _generator.py:10410 must restrict selector 104 handling to U64 semantic classes and add generator coverage.
Moderate (2 votes): vop3p.cpp:4261 must reject unused neg and neg_hi modifier bits.
Moderate (1 vote): vop3p.cpp:4287 must accept valid SGPR104_128 selectors.
Nit (1 vote): cdna5_execution_test.cpp:2536 should treat selector 104 as valid and cover it accordingly.

File summaries

File	Description
`shared/machine-readable-isa/isa/amdgpu_isa_cdna5_variants.json`	Updates model-only instruction metadata.
`emulation/rocjitsu/tests/cdna5_execution_test.cpp`	Adds packed FP64 execution and decoder validation tests.
`emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/shared/fp_mode.h`	Adds MODE-aware binary FP64 operations.
`emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/cdna5/vop3p.h`	Declares executable FP64 instruction classes.
`emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/cdna5/vop3p.cpp`	Adds FP64 decoding and validation.
`emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/cdna5/vop3p_exec.cpp`	Implements packed FP64 callbacks.
`emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/cdna5/execution_backend.h`	Adds callback identifiers.
`emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/cdna5/execution_backend_exec.cpp`	Registers execution callbacks.
`emulation/rocjitsu/lib/python/amdisa/tests/test_sema_derive.py`	Tests FP64 semantic derivation.
`emulation/rocjitsu/lib/python/amdisa/tests/test_packed_codegen.py`	Tests packed FP64 callback generation.
`emulation/rocjitsu/lib/python/amdisa/tests/test_generator_profile_gates.py`	Tests generated validation gates.
`emulation/rocjitsu/lib/python/amdisa/semantics.py`	Defines packed FP64 semantics.
`emulation/rocjitsu/lib/python/amdisa/codegen/execute/packed.py`	Generates packed FP64 execution code.
`emulation/rocjitsu/lib/python/amdisa/codegen/execute/__init__.py`	Registers the FP64 code generator.
`emulation/rocjitsu/lib/python/amdisa/codegen/_generator.py`	Generates instruction validation.

Review details

Suppressed comments (1)

emulation/rocjitsu/tests/cdna5_execution_test.cpp:2537

This newly added case treats src1 = 104 as invalid, but 104 is the valid SGPR104_128 member of the 128-bit public source class. Once the decoder is regenerated with the intended width-specific validation, this case will fail and the suite still will not exercise the legal form; remove it from the invalid cases and add 104 to the valid-selector coverage.

  constexpr std::array<uint16_t, 21> kInvalidV2F64Selectors{
      106, 107, 125, 126, 127, 209, 229, 230, 231, 232, 234,

Files reviewed: 10/15 changed files
Comments generated: 1
Review effort level: Lite

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

sjain-stanford

The existing thread has one consolidated remaining finding: packed FP64 validation still rejects selector 104 despite its public operand class, and the related acceptance/fresh-generation/modifier regression coverage needs updating. The earlier generator/output mismatch and missing generated negation-bit checks are addressed.

kuhar

LGTM % one comment

sjain-stanford

Thanks for addressing the comment. LGTM.

Enable instruction-level execution for gfx1251 V_PK_ADD_F64, V_PK_MUL_F64, V_PK_MAX_NUM_F64, and V_PK_MIN_NUM_F64. These instructions were previously model-only, so valid decoded instructions could not produce architectural register results. Generate callbacks that route both packed elements through the shared MODE-aware F64 helpers. This honors rounding and denormal modes, EXEC masking, independent low/high source negation, clamp, packed-FP64 scalar propagation, literals, inline constants, and overlapping register tuples. Validate the fixed packed layout and public LLVM width-specific source and register classes before installing each callback. Tests exercise the public LLVM encodings and source forms, boundary selectors and tuples, FP modes, NaN and signed-zero behavior, modifiers, inactive lanes, aliasing, and decoder rejection paths. Keep V_PK_FMA_F64 and V_WMMA_F64_16X16X4_F64 model-only. gfx1251 still does not advertise full simulator execution, so this change provides decode-to-register execution rather than simulator or AQL end-to-end support.

Signed-off-by: hanhanW <hanhan0912@gmail.com>

LLVM defines each packed FP64 source as VSrc_v2f64. Its VS_128 register class includes SGPR104_128, so rejecting selector 104 makes valid llvm-mc encodings fail to decode. Permit selector 104 in both source positions for V_PK_ADD_F64, V_PK_MUL_F64, V_PK_MAX_NUM_F64, and V_PK_MIN_NUM_F64, then regenerate the CDNA5 decoder. Add fresh-generation assertions for the special selector and unused modifier bits. Exercise each operation with SGPR104_128 in both source positions and verify that the VCC tuple components are not consumed. Tests: Gfx1251PackedF64ExecutionTest.*; related generator profile tests

Packed F64 sources are 128-bit operands whose 32-bit literal supplies a broadcast F64 element. The generator previously repeated FMT_NUM_PK2_F64 in both the widening table and a size predicate, while the surrounding names and diagnostic still described only 64-bit operands. Key widening policies by operand width and data format so the packed-F64 table entry is the single source of truth. Generalize the helper name and diagnostic, and add focused coverage showing that 128-bit packed F64 uses F64 high-bit widening while packed U64 retains the default immediate semantics. Tests: test_literal_codegen.py; relevant generator profile tests; generate-amdisa.sh

GCC 13 at -O2 diagnoses the single-element std::vector initializer-list assignment in DbiArgSimBase::SetUp as an out-of-bounds inlined memmove. Because rocJitsu treats warnings as errors, this prevents the ubsan-gcc CI lane from building after the DBI argument tests landed on develop. Append the sentinel to the newly constructed empty vector instead. This preserves the one-element probe argument list while avoiding the optimizer warning. Tests: GCC 13/UBSan full build; DBI argument simulator tests; packed-F64 execution tests

systems-assistant · 2026-09-18T07:06:12Z

TheRock Submodule Bump Activity

Newest first

2026-09-18 — Included in TheRock via ROCm/TheRock#8300.

hanhanW requested review from amd-eochoalo and kuhar August 28, 2026 04:38

hanhanW requested review from a team and atgutier as code owners August 28, 2026 04:38

github-actions Bot added the project: rocjitsu label Aug 28, 2026

hanhanW marked this pull request as draft August 28, 2026 04:39

systems-assistant Bot added the organization: ROCm label Aug 28, 2026

hanhanW force-pushed the users/hanhanW/diff-gfx1251-packed-u64-arithmetic branch from 44b2e76 to a96c48a Compare September 2, 2026 14:53

hanhanW force-pushed the users/hanhanW/gfx1251-packed-fp64-arithmetic branch from 4d89c57 to 66abb78 Compare September 2, 2026 15:01

hanhanW marked this pull request as ready for review September 2, 2026 15:01

This was referenced Sep 2, 2026

[rocjitsu] Execute packed FP64 fused multiply-add #10997

Merged

[rocjitsu] Implement gfx1251 F64 WMMA execution. #11124

Merged

[rocjitsu] Model target-specific CDNA5 setreg semantics. #11125

Merged

hanhanW force-pushed the users/hanhanW/gfx1251-packed-fp64-arithmetic branch from 66abb78 to e063d7a Compare September 2, 2026 15:33

hanhanW mentioned this pull request Sep 2, 2026

[rocjitsu] Enable functional gfx1251 simulation #11131

Merged

kuhar reviewed Sep 10, 2026

View reviewed changes

Comment thread emulation/rocjitsu/tests/cdna5_execution_test.cpp Resolved

Comment thread emulation/rocjitsu/lib/python/amdisa/codegen/execute/__init__.py Resolved

hanhanW changed the base branch from users/hanhanW/diff-gfx1251-packed-u64-arithmetic to develop September 15, 2026 16:46

Copilot AI lite review requested due to automatic review settings September 15, 2026 16:47

hanhanW force-pushed the users/hanhanW/gfx1251-packed-fp64-arithmetic branch from cbb9e4b to 4b0b0c0 Compare September 15, 2026 16:47

therock-pr-bot Bot added the Not ready to Review label Sep 15, 2026

Copilot started reviewing on behalf of hanhanW September 15, 2026 16:48

View session

hanhanW requested a review from sjain-stanford September 15, 2026 16:51

Copilot AI reviewed Sep 15, 2026

View reviewed changes

Comment thread emulation/rocjitsu/lib/python/amdisa/codegen/_generator.py Resolved

sjain-stanford requested changes Sep 15, 2026

View reviewed changes

hanhanW requested a review from sjain-stanford September 15, 2026 21:46

kuhar approved these changes Sep 16, 2026

View reviewed changes

Comment thread emulation/rocjitsu/lib/python/amdisa/codegen/_generator.py Resolved

sjain-stanford approved these changes Sep 16, 2026

View reviewed changes

hanhanW added 8 commits September 16, 2026 11:20

[rocjitsu] Use std::isnan for packed FP64 minmax

fa3cacc

[rocjitsu] Use standard quiet NaN for packed FP64 minmax

5c28e95

address comments

8ff2444

Signed-off-by: hanhanW <hanhan0912@gmail.com>

[rocjitsu] Keep SGPR104 selector U64-only

723279e

Signed-off-by: hanhanW <hanhan0912@gmail.com>

hanhanW force-pushed the users/hanhanW/gfx1251-packed-fp64-arithmetic branch from 65a00ef to afde462 Compare September 16, 2026 18:48

hanhanW merged commit 6feea34 into ROCm:develop Sep 16, 2026
37 of 39 checks passed

accauble reviewed Sep 17, 2026

View reviewed changes

Comment thread emulation/rocjitsu/tests/dbi/dbi_arg_sim_test.cpp Resolved

hanhanW deleted the users/hanhanW/gfx1251-packed-fp64-arithmetic branch September 21, 2026 18:50

Navigation Menu

Repository navigation

[rocjitsu] Execute packed FP64 basic arithmetic. - #10871

[rocjitsu] Execute packed FP64 basic arithmetic.#10871
hanhanW merged 8 commits into
ROCm:developfrom
hanhanW:users/hanhanW/gfx1251-packed-fp64-arithmetic

hanhanW commented Aug 28, 2026

Uh oh!

hanhanW commented Aug 28, 2026

Uh oh!

kuhar left a comment

Uh oh!

Uh oh!

Uh oh!

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

edited

Loading

Uh oh!

therock-pr-bot Bot commented Sep 15, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

sjain-stanford left a comment •

edited

Loading

Uh oh!

kuhar left a comment

Uh oh!

Uh oh!

sjain-stanford left a comment

Uh oh!

Uh oh!

Uh oh!

systems-assistant Bot commented Sep 18, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

hanhanW commented Aug 28, 2026

Uh oh!

hanhanW commented Aug 28, 2026

Uh oh!

kuhar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

therock-pr-bot Bot commented Sep 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

❌ PR Check — Action Required

Uh oh!

therock-pr-bot Bot commented Sep 15, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (1)

Uh oh!

Uh oh!

sjain-stanford left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kuhar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sjain-stanford left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

systems-assistant Bot commented Sep 18, 2026

TheRock Submodule Bump Activity

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

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

edited

Loading

sjain-stanford left a comment •

edited

Loading