Repository navigation
[rocjitsu] Match gfx1201/gfx1100 DOT2, FP8/BF8 DOT4 and integer DOT half selection - #13691
yashjha123 wants to merge 7 commits into
Conversation
…alf selection
- True16 V_DOT2_F16_F16 / V_DOT2_BF16_BF16 run one packed step of each
target's DOT/WMMA datapath; MODE rounding and denormal fields do not
apply and FP16_OVFL clamps F16 only. Remove fp_mode::dot2_f16/dot2_bf16.
- RDNA4 V_DOT4_F32_{FP8,BF8}_{FP8,BF8} run through the GFX12 DOT datapath:
the four products align on one grid and an invalid product returns the
factor-NaN payload.
- NEG_HI on the F32 DOT accumulator is ABS, applied before NEG through the
shared source_modifier stage in the scalar and SIMD paths. On GFX12 a tiny
negative C subtracts one unit when the grid is at or below 2^-126.
- OP_SEL/OP_SEL_HI select the 16-bit source halves of integer DOT4/DOT8,
before signedness and CLAMP, in the scalar and SIMD paths.
- Add the BF16 layout to fp_format.h and a VOP3P_DOT_INT SIMD path id.
Shared generated bodies also change RDNA2 and CDNA1-4 (accumulator ABS,
integer selection); CDNA5 integer DOT changes directly. The GFX12
accumulator rule also reaches RDNA4 VOPD DOT2ACC and WMMA. Decoded cases
come from gfx1201 and gfx1100 captures; other targets are extrapolated.
✅ All Checks Passed — Ready for Review
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
|
🎉 All checks passed! This PR is ready for review. |
There was a problem hiding this comment.
🟡 Changes recommended
Clamped integer tests require SIMD on configurations where the existing AVX-512 workaround deliberately selects scalar execution.
1 open issue
What changed in this PR
Updates rocjitsu’s DOT emulation to match gfx1100/gfx1201 hardware behavior, extending the floating-point accuracy work in #12467.
Changes:
- Reuses architecture-specific arithmetic for packed DOT2 and FP8/BF8 DOT4.
- Corrects accumulator modifiers and integer source-half selection.
- Adds capture-based regressions covering arithmetic, modifiers, and execution paths.
| File | Description |
|---|---|
| emulation/rocjitsu/tests/valu_fp_mode_test.cpp | Adds capture-based DOT regressions. |
| emulation/rocjitsu/tests/source_modifier_test.cpp | Tests BF16 sign modifiers. |
| emulation/rocjitsu/tests/instruction_execution_harness_test.cpp | Updates BF16 and accumulator expectations. |
| emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/shared/simd_glue.h | Updates SIMD selection, modifiers, and tracking. |
| emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/shared/gfx12_dot.h | Extends GFX12 DOT arithmetic. |
| emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/shared/gfx11_dot2.h | Documents packed DOT2 reuse. |
| emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/shared/fp_mode.h | Removes superseded DOT2 helpers. |
| emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/shared/fp_format.h | Adds BF16 format traits. |
| emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/shared/execute_shared.h | Updates shared DOT executors. |
| emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/rdna4/vop3p_exec.cpp | Applies RDNA4 DOT fixes. |
| emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/rdna4/vop3_exec.cpp | Uses GFX12 packed DOT2 arithmetic. |
| emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/rdna3/vop3_exec.cpp | Uses GFX11 packed DOT2 arithmetic. |
| emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/rdna3_5/vop3_exec.cpp | Reuses GFX11 packed DOT2 arithmetic. |
| emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/cdna5/vop3p_exec.cpp | Applies integer half selection. |
| emulation/rocjitsu/lib/python/amdisa/tests/test_packed_codegen.py | Tests generated DOT behavior. |
| emulation/rocjitsu/lib/python/amdisa/tests/test_generator_profile_gates.py | Updates packed DOT2 assertions. |
| emulation/rocjitsu/lib/python/amdisa/codegen/execute/packed.py | Generates corrected DOT operations. |
| emulation/rocjitsu/lib/python/amdisa/codegen/execute/__init__.py | Passes architecture and selection settings. |
| emulation/rocjitsu/lib/python/amdisa/codegen/_generator.py | Updates dispatch and required includes. |
Review effort: Balanced
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
kuhar
left a comment
kuhar
left a comment
There was a problem hiding this comment.
I think it would help to use operand and instruction names that expose their roles in the new mixed-format code and capture tables:
- In
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/shared/gfx12_dot.h:144, useSrc0Format/Src1Formatfor theA/Btemplate types and the corresponding aliases ingfx12_dot4_f32_fp8. This makes it easier to track which input uses FP8 or BF8. - In
emulation/rocjitsu/tests/valu_fp_mode_test.cpp:2893, usesrc0/src1/accforLane::{a,b,c}and the matchingaddparameters. Around line 3130, names such asdot2_f32_f16anddot4_u8would distinguish opcode constants from format/type names likef16andu8.
The capture values can stay as they are; these names would make their interpretation easier to check.
We should record the new captures' provenance and measured scope in the repository. emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/shared/gfx12_dot.h:142 points readers to tests/fixtures/float_dot/README.md, but that README only documents the existing F16/BF16 DOT2/WMMA fixtures.
Please document the cards, SDK/driver and capture procedure for the new inline true16 DOT2, FP8/BF8 DOT4 and integer modifier cases in valu_fp_mode_test.cpp. Also distinguish measured gfx1100/gfx1201 behavior from the integer half-selection rule extrapolated to RDNA2/CDNA (lib/python/amdisa/codegen/execute/packed.py:1411), and record which callers exercise the changed accumulator-floor rule. The new gfx1201 BF16 WMMA probe supplies evidence for that caller; VOPD should remain identified as extrapolated unless separately measured.
For the older NEG_HI/NEG rows (tests/valu_fp_mode_test.cpp:3113), describe the capture batch and its joint rounding-field sweep instead of the unexplained “before 10-17” reference. This would make the evidence and its limits recoverable without the PR discussion.
| static constexpr uint16_t infinity = Bf16 ? 0x7f80 : 0x7c00; | ||
| /// @brief Factor encoding: exponent and fraction widths. OCP E4M3 has no | ||
| /// infinity; S.1111.111 is its only NaN. | ||
| template <int ExponentBits, int FractionBits, bool OcpE4m3 = false> struct FactorFormat { |
There was a problem hiding this comment.
I think it would help to derive F16Factor and Bf16Factor from fp_format::F16 and fp_format::BF16's kExponentBits/kMantissaBits. This PR adds the BF16 format traits but also repeats both layouts here as <5, 10> and <8, 7>. Reusing those traits would keep the layout definitions in one place; matching their constant names would also make the relationship easier to follow.
| /// packing op_sel == 0 && op_sel_hi == 3); neg / neg_hi flip the src0/src1 | ||
| /// product-operand signs and neg bit 2 flips the accumulator. The encoded CLAMP | ||
| /// product-operand signs; on the accumulator neg_hi bit 2 is ABS, applied before | ||
| /// neg bit 2, as in the scalar body. The encoded CLAMP |
There was a problem hiding this comment.
Nit: could we put @tparam Fmt on its own Doxygen line and reflow the final paragraph? It is currently attached to the NaN-payload sentence, with pk_fma slices split awkwardly across lines. Separating the parameter documentation would make this helper's contract easier to scan.
| const uint32_t expected = arch == ROCJITSU_CODE_ARCH_RDNA4 ? lane.gfx1201 : lane.gfx1100; | ||
| for (const uint32_t mode : lane.modes) { | ||
| char suffix[16]; | ||
| std::snprintf(suffix, sizeof(suffix), "Mode%X", mode); |
There was a problem hiding this comment.
We should use std::format("Mode{:X}", mode) for the suffix here and in dot_family_cases() around line 3274. It preserves the case names while removing the two fixed-size buffers and the new <cstdio> dependency, and follows the project's formatting idiom.
| # Only RDNA4 has these; gfx1201 runs them through its DOT datapath. | ||
| if dot_accumulation is not FloatDotAccumulation.GFX12: | ||
| raise ValueError(f'{instruction} needs the GFX12 DOT accumulation model') | ||
| formats = instruction.upper().removeprefix('V_DOT4_F32_').split('_') |
There was a problem hiding this comment.
This might be easier to maintain if the FP8/BF8 operand formats lived on InstructionSemantics, as they already do for packed conversions (semantics.py:889). All four DOT4 names currently map to one class, so both dispatch sites pass the mnemonic through just for gen_dot4 to parse it again. Carrying the format pair in operation would keep instruction classification in the semantic layer and remove the new instruction parameter and name parsing here.
| using namespace gfx12_dot_detail; | ||
| // 8-bit factors align all products on one grid, and an invalid product | ||
| // returns the factor-NaN payload. 16-bit factors align products in pairs. | ||
| constexpr bool kByteFactors = A::exponent_bits + A::fraction_bits < 8; |
There was a problem hiding this comment.
Optional: make product grouping and the invalid-product payload explicit arithmetic-policy inputs to gfx12_dot_terms. kByteFactors currently chooses both from the left operand's width, although the documented evidence is specifically for gfx1201 FP8/BF8 DOT4. Having the DOT4 and half-format wrappers select those two policies would distinguish instruction behavior from encoding layout. Current callers are correct; this is about making the shared DOT/WMMA core's assumptions visible.
| // that tiny C. | ||
| const int c_floor = Packed && !Bf16 ? -14 : -126; | ||
| const int grid = std::max(product_grid, std::max(c_floor, c.leading_exponent()) - 26); | ||
| int64_t total = align(c, grid, grid > c_floor); |
There was a problem hiding this comment.
It would be good to retain one F32-output and one BF16-output WMMA witness for this accumulator-floor change. The existing WMMA fixtures pass with either rule, while the gfx1201 probe matched the new rule in all 16,384 cases per opcode and distinguished it in 56 F32 and 14 BF16 cases.
For wave32, use A with only k=0 nonzero, uniform B and uniform C:
| Instruction | A[k=0] | B | C | Hardware/new | Old rule |
|---|---|---|---|---|---|
v_wmma_f32_16x16x16_bf16 |
0x2900 |
0x2280 |
0x8000082d |
0x0bffffff |
0x0c000000 |
v_wmma_bf16_16x16x16_bf16 |
0x3800 |
0x1400 |
0x8051 |
0x0c7f |
0x0c80 |
Adding these to the decoded WMMA tests would protect the changed behavior. The measurements support the implementation; this is a regression-coverage request.
…oduct policy explicitly - F16Factor and Bf16Factor come from fp_format::F16/BF16; factor constants use fp_format's kExponentBits/kMantissaBits names. - gfx12_dot_terms takes Src0Format/Src1Format and a ProductPolicy naming the product grouping and invalid-product NaN. The half-format and FP8/BF8 DOT4 wrappers choose them, instead of inferring both from the factor width. No behaviour change: DOT replay outputs are byte-identical.
…th std::format - The captured DOT lanes use src0/src1/acc, and the opcode constants read as instructions (dot2_f32_f16, dot4_u8, ...) rather than formats. - Build the "Mode<hex>" case suffix with std::format and drop <cstdio>. - Give @tparam its own line in try_execute_vop3p_dot_f16_simd's comment. Test names and values are unchanged.
V_DOT4_F32_{FP8,BF8}_{FP8,BF8} now record their src0/src1 formats in the
semantic operation, as the packed FP8/BF8 conversions do. gen_dot4 reads the
operation instead of re-parsing the mnemonic, so both dispatch sites drop the
instruction-name argument.
Regenerated sources are unchanged.
Add F32-output and BF16-output V_WMMA_*_16X16X16_BF16 cases, captured on gfx1201 wave32 with only k=0 of A nonzero: 0x0bffffff and 0x0c7f, where the old rule gave 0x0c000000 and 0x0c80. Restoring the old rule fails only this test. The probe matched the new rule on all 16,384 cases per opcode. Rename Bf8Bf8TinyNegativeAccumulator to Bf8Bf8NegativeSubnormalAccumulator: FP8/BF8 products never reach the -126 grid, so that lane shows C read unflushed, not the floor unit. Point the DOT capture comments at the fixtures README instead of a capture date.
Extend tests/fixtures/float_dot/README.md with the cards, ROCm and driver versions, procedure and MODE coverage of the inline DOT captures; separate the behaviour measured on gfx1100/gfx1201 from what RDNA2, CDNA1-4 and CDNA5 inherit through shared generated bodies; and list which callers of the negative-C floor rule were measured (VOP3P DOT2_F32_BF16, BF16 WMMA) and which are extrapolated (true16 DOT2_BF16_BF16, VOPD DOT2ACC_F32_BF16).
Motivation
The emulator used CPU floating-point accumulation for F16/BF16-output DOT2 and FP8/BF8 DOT4, which did not match the GPU’s accumulation order and precision. DOT4 also decoded BF8 operands as FP8. This PR reuses and extends the existing DOT/WMMA arithmetic to match hardware rounding, special values and subnormals.
What else was fixed
Some encoded operand controls were also ignored: floating DOT missed accumulator ABS, and integer DOT4/DOT8 ignored non-default source-half selection. These now use shared modifier and selection helpers. Default integer arithmetic and saturation were already correct. The existing F32 DOT arithmetic is retained, with a correction for tiny negative accumulators on GFX12.
Regression tests use gfx1100/gfx1201 captures and cover independent MODE fields, overflow, NaNs, subnormals, combined modifiers and half selection. They exercise SIMD enabled and forced scalar execution, check inactive lanes, and require eligible integer cases to use SIMD. Full test and replay results are pending.
Note: the shared generated code also changes RDNA2 and CDNA1–4 (accumulator ABS, integer half selection), and CDNA5 integer DOT. The gfx12 accumulator rule also reaches RDNA4 VOPD DOT2ACC and WMMA. Only gfx1201 and gfx1100 were measured; other targets follow by extrapolation.
###How it was tested
Issue tracking
#12467