Skip to content

fix(shader): round the f16 fused multiply-adds once - #2000

Merged
adriadam10 merged 1 commit into
boykopovar:mainfrom
hansjm10:fix/shader-f16-fma-single-rounding
Oct 10, 2026
Merged

adriadam10 merged 1 commit into
boykopovar:mainfrom
hansjm10:fix/shader-f16-fma-single-rounding

Conversation

@hansjm10

@hansjm10 hansjm10 commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

What

v_fma_f16, v_fmac_f16, v_fmamk_f16, v_fmaak_f16, v_pk_fma_f16 and v_pk_fmac_f16 were translated as an f32 FMA rounded to f16, which rounds twice: 0x62e9 * 0xd159 + 0x23ec gave 0xf89e, the hardware 0xf89f (TechnicalDebt).

The product of two f16 values is exact in f32. The translation now adds it to the addend in f32, recovers the exact rounding error with TwoSum, rounds the sum to odd and converts it with the existing round-to-nearest-even f16 conversion. Rounding to odd with at least p + 2 bits and then to nearest with p bits is one rounding (Boldo and Melquiond, "Emulation of FMA and Correctly Rounded Sums: Proved Algorithms Using Rounding to Odd", IEEE Transactions on Computers 57(4), 2008); f32 has 24 bits, f16 11. The f32 add and sub are already emitted with NoContraction, so drivers keep them exact. No f64 is needed. v_fma_mix* (f32 sources) is unchanged.

Source of the semantics: measured on RDNA2, gfx1036 (Ryzen iGPU, RADV, Mesa 22.3.6), through a Vulkan compute shader whose f16 fma() ACO compiled to v_fmac_f16_e32, v_pk_fmac_f16, v_fma_f16 and v_pk_fma_f16 (checked in the RADV_DEBUG=shaders disassembly). For each of the four, 2,200,256 inputs (the test rows, 1M uniform random, 1M near-cancellation, 200k with subnormal results) all match exact single rounding to nearest even, with f16 denormals kept and overflow to infinity. Not run with the hardware oracle: the float mode was the driver's default, and no NaN inputs were used. The PS5's gfx1013 is not measured.

Removes the TechnicalDebt entry.

Tested

Linux (Debian 12, GCC 12.2), Release.

  • New agc_driver_float16_fma_rounding: 64 lanes, 7 instruction forms. 116 of its 128 v_fma_f16/v_pk_fma_f16 triples and 37 of its v_fmaak_f16/v_fmamk_f16 rows round differently when rounded twice; they include results at the f16 overflow boundary (252 * 260 - 2^-24: 0x7bff, twice rounded 0x7c00), subnormal results, signed zeros and infinities. Fails on main (lane 0 v_fma_f16 is 0xf89e, expected 0xf89f); passes on RADV gfx1036, Intel Arc A310 (ANV) and lavapipe, under 0.1 s.
  • agc_driver_float16_ternary_clamp, agc_driver_packed_alu, agc_driver_fma_rounding, agc_driver_float16_modifiers pass on all three devices.
  • ctest agc|shader|recompiler|spirv|fused|vop on lavapipe (Mesa 22.3.6): 6 failures that fail identically on main (missing device features), and agc_driver_flat_store timed out under -j12. On RADV gfx1036: 8 failures that also fail on main, and agc_driver_scalar_wide_loads failed once under -j8 and passes 5 of 5 alone.

Checklist

  • Based on current main; no other open PR implements the same functions
  • One topic per PR; follow-ups go in a new PR
  • No comments in code except technical debt
  • Unimplemented paths throw (NotImplemented_nid_no_patch); silent stubs are listed in TechnicalDebt
  • No notes, investigation .md files or images added to the repository (attach them to this PR)
  • New third-party code is a submodule built from source
  • Generic behaviour, not specific to one title
  • Depends on: none
  • AI-assisted: yes

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

The finite-value path has a sound basis: the f16 product is exact in f32, and the SPIR-V f32 add/sub/mul emitters apply NoContraction, which the TwoSum error recovery needs. The new execution test also exercises the conversion boundary and all six instruction families.

One scope note: the PR says NaNs were not measured or included in the hardware matrix, while the new non-mix path replaces one FMA with multiply/add operations. The existing TechnicalDebt entry still says arithmetic NaN bits are driver-dependent; please confirm that remains the intended documented limit for this path, especially invalid inf * 0 inputs to the non-packed instructions. I am not approving without upstream validation: Build and Conventions are still action_required on this head.

@hansjm10

hansjm10 commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor Author

Yes, that stays the documented limit: the scalar f16 FMA NaN payload is the driver's, as TechnicalDebt says for v_add/mul/fma_f32/f16 and for scalar f16 arithmetic under the clamp modifier.

  • The set of NaN results is unchanged. With f16 sources the f32 product cannot overflow, so mul + add is NaN exactly when the fused op was: a NaN source, inf * 0 or inf - inf. A NaN sum makes the TwoSum error NaN, the ordered != 0 test is false, and the sum passes through unchanged.
  • Packed results don't depend on the driver: packedFloat16 still builds NaN lanes from the source bits (the gfx1035 rule, including inf * 0 giving the default NaN over a NaN src2).
  • Only the scalar payload can move, from the driver's fma to its mul/add.

Checked with a throwaway probe (12 lanes of NaN, sNaN, inf * 0, inf * 0 + NaN, inf - inf and all-NaN sources through the 7 forms of the new test), on main and on this branch:

  • RADV (gfx1036) and lavapipe: all outputs identical.
  • ANV (Arc A310): only scalar v_fma_f16/v_fmac_f16 changed, in two cases. inf * 0 + 0x7e01 was 0x7e01, now 0xfe00; sources 0x7e11, 0x7c22, 0x7e33 gave 0x7e33, now 0x7e11. Packed, v_fmaak_f16 and v_fmamk_f16 are unchanged. On this branch the three drivers return the same bits for every probe row; on main ANV differed in those two.
  • Signaling NaNs are still quieted on the scalar path by all three drivers (0x7c01 * 1 + 1 gives 0x7e01), where the packed path keeps 0x7c01 at IEEE=0, as already documented.

v_fma_f16, v_fmac_f16, v_fmamk_f16, v_fmaak_f16, v_pk_fma_f16 and
v_pk_fmac_f16 rounded an f32 fused multiply-add to f16, rounding twice.
The product of two f16 values is exact in f32, so the sum is now rounded
to odd in f32 with an exact error term and then rounded to nearest even
in f16, which rounds the exact result once.
@hansjm10
hansjm10 force-pushed the fix/shader-f16-fma-single-rounding branch from 888724c to 6de064f Compare October 9, 2026 09:31
@github-actions

github-actions Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

Other open pull requests touch the same code. Merging one will break the other, or both implement the same thing. If this one builds on them, list them in Depends on.

Pull request Conflicting files (lines on main) Same exports Same new files
#1281 docs/dev/TechnicalDebt.md L330-332
#1290 docs/dev/TechnicalDebt.md L330-332
#2025 core/shader/recompiler/Translation/src/FloatInstructions.cpp L21-22, L25-26, L216-217
#2035 docs/dev/TechnicalDebt.md L331-332
#2042 docs/dev/TechnicalDebt.md L331-332
#2043 docs/dev/TechnicalDebt.md L331-332
#2162 core/shader/recompiler/Translation/src/FloatInstructions.cpp L216
docs/dev/TechnicalDebt.md L331-332

@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 9, 2026
@popcoder565

Copy link
Copy Markdown
Contributor

Native Windows run, since no Windows results were posted. Setup: Windows 10 (19045), RTX 3060 Laptop (NVIDIA 616.92), MinGW-w64 GCC 15.2.0 (WinLibs r7), Release. Head 6de064f6, merged onto main d70b899 in one build with 33 other AGC/shader PRs that had no Windows run.

No title was run.

🤖 Addressed by Claude Code

@adriadam10

Copy link
Copy Markdown
Collaborator

Checked on the HW oracle (RDNA2 gfx1035) with the test's seven instruction forms: with f16 denormals kept, at IEEE=0 and IEEE=1, the hardware matches the Expected table bit for bit and matches this lowering on 4,820 more rows (random plus ±0, subnormals, the 0x7bff overflow boundary, infinities, NaNs), and every finite case equals exact single rounding. The test fails without the change and passes on RADV and on CI's lavapipe (Mesa 25.2.8); popcoder565's Windows/NVIDIA run covers the rest. Note for whichever of this and #2025 lands second: they touch the same lines of FloatInstructions.cpp, and clampF16Overflow wraps fmaF16RoundedToOdd without issue. Merging.

@adriadam10
adriadam10 merged commit 0f84fef into boykopovar:main Oct 10, 2026
6 checks passed
SleglJan pushed a commit to SleglJan/AnyPS5 that referenced this pull request Oct 10, 2026
…ingle-rounding

fix(shader): round the f16 fused multiply-adds once
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agc libSceAgc* docs shader core/shader

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants