[sjain-stanford]

Context and provenance

Fixes #10760. Addresses the same clamp omission reported for reverse subtraction in #10762.

Issue #10760 reported that a PyTorch float8_e4m3fn to float32 cast produced severely compressed values under rocJITsu's gfx1250 model even though the FP8 bytes were correct. The first incorrect intermediate was:

encoding: d5268004 02010904
v_sub_nc_u32 v4, v4, 4 clamp

For the raw E4M3 byte 0xc8, representing -4, rocJITsu wrapped 1 - 4 to 0xfffffffd instead of saturating the unsigned result to zero. That value corrupted the later shift and exponent reconstruction, producing -0.0625 instead of -4.0.

Root cause

rocJITsu decoded the VOP3 clamp field, but integer semantic lowering narrowed ordinary wrapping arithmetic without consulting it. Several SIMD paths made the same assumption. Because clamp is shared by the VOP3 encoding but is not legal for every opcode, deciding support from the encoding field or broad semantic class alone is also insufficient.

Implementation

The fix is implemented in the ISA profile and semantic generator rather than special-casing PyTorch, FP8, or gfx1250:

• An instruction-level policy identifies assembler-legal integer CLAMP operations. Unsupported combinations such as V_ADD3_U32 clamp do not acquire invented semantics.
• Shared typed helpers use sufficiently wide exact intermediates, saturate to the signed or unsigned destination range when clamp=1, and retain architectural modulo behavior when clamp=0.
• V_ADD_MIN/MAX_{I,U}32 always saturate their internal addition, as required by their instruction semantics, independently of the encoded CLAMP bit.
• The policy covers signed and unsigned 16- and 32-bit add/subtract and reverse-subtract, unsigned 64-bit add/subtract, packed 16-bit add/subtract, and carry/borrow forms.
• Multiply and multiply-add coverage includes signed and unsigned 16- and 24-bit source forms, packed I16/U16 MAD, widening MAD operations, and 32x32-to-64 MAD. The 64-bit CO forms derive carry/overflow from the unsaturated exact result while independently saturating VDST.
• SAD/MSAD coverage includes V_SAD_U8/U16/U32, V_SAD_HI_U8, and V_MSAD_U8; shifted SAD accumulation is performed exactly before saturation.
• Carry and borrow instructions saturate VDST while deriving and updating VCC from the original full-width arithmetic, including clearing stale result bits.
• Clamped generic, packed, carry/borrow, ternary, true16, and wide-MAD SIMD paths fall back to the corrected scalar implementation. Unclamped operations retain their existing SIMD paths.
• Shared legacy MAD64 lowering now applies the same semantics across CDNA1-4 and RDNA1-3.5; CDNA5's split generated paths use the same exact-intermediate rules.
• Generated execution sources were regenerated for the affected shared, CDNA1-5, RDNA1-4, and RDNA3.5 profiles; generated implementations were not patched by hand.

Regression coverage

The gfx1250 execution regression decodes and executes the exact instruction words from #10760 in both normal and forced-scalar modes. Additional coverage exercises:

• unsigned overflow/underflow and signed upper/lower saturation at 16, 32, and 64 bits;
• unsigned 64-bit saturation through normal and DPP execution;
• intrinsic ADD_MIN/MAX saturation with both encoded CLAMP values;
• packed I16/U16 add, subtract, and MAD with default and non-default selectors;
• 64-bit MAD signed positive overflow, signed underflow, unsigned overflow, unclamped wrapping, and both carry transitions;
• CDNA4 shared MAD64 behavior under forced-scalar and SIMD-enabled execution;
• reverse subtraction and unclamped modulo behavior;
• carry-in, borrow-in, carry/borrow set and clear outcomes, and stale VCC/SDST bits;
• signed and unsigned 16-/24-bit multiply and MAD boundaries;
• SAD/MSAD overflow, including shifted V_SAD_HI_U8 accumulation;
• normal versus forced-scalar execution for the affected SIMD-selected paths.

Generator tests use real MR ISA definitions to verify positive instruction-policy routing, unsupported-opcode rejection, helper selection, and consumption of inst_.clamp.

Validation performed

• Ran official amdisa regeneration and built the full rocjitsu_tests target across the generated architecture set.

• Passed all 107 Gfx1250ExecutionTest tests.

• Passed the focused amdisa generator and semantic suites: 1,012 passed, 1 skipped.

• Passed focused CDNA4 and CDNA5 native regressions, including clamped and unclamped shared MAD64 scalar/SIMD behavior.

• Re-ran the original PyTorch FP8 reproducer and restored the expected sequence:

  [-4.0, -2.0, -1.0, -0.5, -0.25, 0.0,
    0.25, 0.5, 1.0, 2.0, 4.0, 8.0]
  

• Passed all configured pre-commit hooks and git diff --check.

• Completed iterative multi-agent review with seven reviewers plus a curator; all curated findings were addressed and the final pass was clean.

Co-authored-by: GPT-5.6 Sol codex@openai.com

🤖 Generated with Codex

[therock-pr-bot]

✅ 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]

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

[Copilot]

Pull request overview

This PR fixes rocJITsu’s semantic lowering for VOP3 integer add/sub (including reverse subtract) when the instruction encoding sets CLAMP=1, ensuring results saturate (rather than wrap) before narrowing to 16/32-bit signed/unsigned destinations. This addresses the clamp-omission root cause behind incorrect PyTorch FP8 E4M3→FP32 casts on the gfx1250 (CDNA5) model.

Changes:

• Introduces shared typed helpers (vop3_integer_add / vop3_integer_sub) that implement correct saturating semantics when CLAMP is set, while preserving modulo wraparound when CLAMP is clear.
• Updates generated execute paths across shared + multiple arch profiles to route relevant VOP3 integer ops through the helpers, and disables the VOP3 integer SIMD fast-path when CLAMP is set (fallback to scalar for correctness).
• Adds targeted C++ execution regression tests (including forced-scalar mode) and Python generator/lowering tests to ensure inst_.clamp is consumed via the shared helpers.

Reviewed changes

Copilot reviewed 6 out of 15 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
emulation/rocjitsu/tests/cdna5_execution_test.cpp	Adds a gfx1250/CDNA5 execution regression covering the exact reported instruction encoding and boundary saturation cases (plus forced-scalar).
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/shared/simd_glue.h	Adds vop3_integer_add/sub saturation helpers and gates VOP3 integer SIMD fast-path on CLAMP==0.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/shared/execute_shared.h	Updates generated shared execute bodies for relevant VOP3 integer ops to call the new saturation helpers.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/rdna4/vop3_exec.cpp	Regenerates RDNA4 VOP3 execution to use the saturation helpers for relevant integer ops.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/rdna3/vop3_exec.cpp	Regenerates RDNA3 VOP3 execution to use the saturation helpers for relevant integer ops.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/rdna3_5/vop3_exec.cpp	Regenerates RDNA3.5 VOP3 execution to use the saturation helpers for relevant integer ops.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/cdna5/vop3_exec_alu.cpp	Regenerates CDNA5 VOP3 ALU execution to use the saturation helpers for relevant integer ops.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/cdna4/vop3_exec.cpp	Regenerates CDNA4 VOP3 execution to use the saturation helpers for relevant integer ops.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/cdna3/vop3_exec.cpp	Regenerates CDNA3 VOP3 execution to use the saturation helpers for relevant integer ops.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/cdna2/vop3_exec.cpp	Regenerates CDNA2 VOP3 execution to use the saturation helpers for relevant integer ops.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/cdna1/vop3_exec.cpp	Regenerates CDNA1 VOP3 execution to use the saturation helpers for relevant integer ops.
emulation/rocjitsu/lib/python/amdisa/tests/test_sema_derive.py	Adds generator/lowering assertions that VOP3 integer add/sub variants consume inst_.clamp via the shared helpers.
emulation/rocjitsu/lib/python/amdisa/codegen/execute/simd_codegen.py	Updates SIMD codegen commentary to reflect scalar fallback for saturating CLAMP behavior.
emulation/rocjitsu/lib/python/amdisa/codegen/execute/sema_lower.py	Adds lowering support to emit vop3_integer_add/sub(..., inst_.clamp) for targeted integer add/sub nodes.
emulation/rocjitsu/lib/python/amdisa/codegen/_generator.py	Marks applicable VOP3 integer vector_binop ops as saturation-capable when clamp is present, enabling the new lowering path.

---

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

[sjain-stanford]

We should handle CLAMP for the packed integer add/sub path as part of this fix. gen_pk_binop still narrows each result directly (emulation/rocjitsu/lib/python/amdisa/codegen/execute/packed.py:180), and the SIMD path does not gate on inst_.clamp (emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/shared/simd_glue.h:3623). As a result, V_PK_ADD/SUB_[IU]16 still wrap each half; for example, UINT16_MAX + 1 writes 0 instead of UINT16_MAX. Please saturate both selected halves independently, gate or implement SIMD, and test signed/unsigned bounds with non-default op_sel/op_sel_hi.

[sjain-stanford]

Just one comment

[atgutier]

Here are some agent findings:

Major findings

1. V_ADD_{MIN,MAX}_{I,U}32 incorrectly makes the instruction’s intrinsic saturating add depend on the encoded CLAMP bit.

At emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/shared/simd_glue.h:129, vop3_integer_add_minmax() calls: vop3_integer_add(lhs, rhs, clamp)

Therefore CLAMP=0 wraps the intermediate addition. For example, V_ADD_MAX_I32(INT_MAX, 1, INT_MIN) returns INT_MIN.

That conflicts with:

• CDNA5 MR ISA descriptions at shared/machine-readable-isa/isa/amdgpu_isa_cdna5.xml:126948, :127277, :127606, and :127935, which explicitly say the addition is performed “with clamping enabled”.

• LLVM’s VOP3Instructions.td:996-999

(https://github.com/llvm/llvm-project/blob/551d5172dd3902efbce5f4720b75bfc4e6441dc8/llvm/lib/Target/AMDGPU/VOP3Instructions.td#L996-L999), which selects these instructions from saddsat/uaddsat followed by min/max without setting the encoded clamp modifier.

The new tests at tests/cdna5_execution_test.cpp:268-277 codify the incorrect wrapping behavior for CLAMP=0.

Fix: always saturate the internal addition for these four opcodes; treat the encoded destination clamp separately if it has any additional defined effect. Update the unclamped boundary expectations.

Confidence: high.

2. The legal CLAMP modifier is ignored for all four 64-bit integer MAD forms.

The policy at lib/python/amdisa/isa_profile.py:584-642 omits:

• V_MAD_NC_U64_U32
• V_MAD_NC_I64_I32
• V_MAD_CO_U64_U32
• V_MAD_CO_I64_I32

Consequently, gen_vector_mad_64_32() is called without clamp capability at codegen/execute/init.py:220-222, and the generated implementations never read inst_.clamp:

• generated/cdna5/vop3_exec_ternary.cpp:3967 and :4013
• generated/shared/execute_shared.h:13193 and :13215

The public AMD definition explicitly enables clamping for both CO and NC forms, including an explicit “Saturation support” note for NC

I also confirmed that LLVM accepts all four gfx1250 forms with clamp.

For example, unsigned 0xffffffff * 0xffffffff + UINT64_MAX currently wraps instead of saturating VDST to UINT64_MAX.

Fix: add all four instructions to the policy, use a 65-bit-or-wider exact intermediate, saturate VDST, and preserve CO carry/overflow from the unsaturated result. Cover signed/unsigned, NC/CO, and normal/DPP execution.

Confidence: high.

3. V_PK_MAD_I16 and V_PK_MAD_U16 still wrap when CLAMP=1.

The scalar generated bodies at generated/shared/execute_shared.h:17452 and :17493 directly narrow the multiply-add to 16 bits without consulting inst_.clamp. The SIMD path at shared/simd_glue.h:3770 also does not reject clamped execution, so both paths can return the wrapped result.

The public AMD definition enables clamp for packed MAD

LLVM also accepts both gfx1250 forms with the modifier.

Examples:

• Unsigned 0xffff * 2 + 0 returns 0xfffe instead of 0xffff.
• Signed 32767 * 2 + 0 returns -2 instead of 32767.

The added packed regression at tests/cdna5_execution_test.cpp:374-401 covers only packed ADD/SUB.

Fix: use an exact per-half MAD intermediate and signed/unsigned saturation. Until the SIMD implementation supports that operation, reject CLAMP in try_execute_vop3p_pk_ternary_int_simd(). Add signed/unsigned, selector, normal/SIMD, forced-scalar, and unclamped-wrap tests.

Confidence: high.

[sjain-stanford]

Validated all three GitHub review findings against the CDNA5 ISA metadata and generated paths. Addressed in commits affb385 and later, with intrinsic ADD_MIN/MAX saturation, exact 64-bit MAD saturation while preserving CO from the unsaturated result, and per-half packed MAD saturation with clamped SIMD fallback. Added normal, DPP, selector, forced-scalar, SIMD, and unclamped-wrap regression coverage; focused Python and native tests pass.

[atgutier]

I think this issue will break gcc under current flags.

[atgutier]

Thanks for fixing the issues! LGTM now

[systems-assistant]

TheRock Submodule Bump Activity

Newest first

• 2026-09-08 — Included in TheRock via ROCm/TheRock#7974.