Repository navigation
fix(shader): round v_fma_mixlo_f16 and v_fma_mixhi_f16 once to f16 - #2162
Conversation
|
Tested the five-file change from PR head Environment: Windows 11 Pro 10.0.26200, Ryzen 7 7700X, Radeon RX 7900 XT (RDNA 3, device 0x744c), driver 32.0.31041.1004, Vulkan 1.4.349. WinLibs GCC 15.2.0 UCRT/POSIX/SEH r7, Release, BUILD_TESTING=ON, SPIRV-Tools OFF. ANYPS5_GPU=7900 XT and ANYPS5_REQUIRE_VULKAN=1. Before applying the production fix, copied only the two updated regression test sources onto main and built their targets. Both fail: With the complete PR change on the same main revision:
This validates the translated shader execution on AMD's Windows Vulkan driver against the supplied regression expectations. No independent native-instruction hardware oracle or PS5/title testing was performed, and no changes to the PR implementation were made. |
The f16 forms of v_fma_mix computed an f32 FMA and converted it to f16, so a result that lands on an f16 tie after the f32 rounding was rounded twice. RDNA2 rounds the exact product-sum once. They now use the exact f64 path of v_interp_p2_f16 and flush f32 sources like v_fma_mix_f32. They follow the f16 round and denormal fields on hardware, so another f16 mode throws, as packed f16 does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Rebased onto current |
b97126e to
3a9da56
Compare
…16-fused fix(shader): round v_fma_mixlo_f16 and v_fma_mixhi_f16 once to f16
What
v_fma_mixlo_f16andv_fma_mixhi_f16can return a result one f16 step away from the hardware's:They were translated as an f32 FMA converted to f16. That rounds twice: here the f32 rounding lands exactly on an f16 tie, and the second rounding goes to even. The hardware rounds the exact product-sum once. #1878 made the f32 result fused; this does the same for the f16 results.
They now use
FPInterpolateF16, the pathv_interp_p2_f16already uses to round an f32 product-sum once to f16: exact f64 product, sum rounded to odd, one rounding to f16. Their f32 sources are also flushed when the shader flushes f32 denormals, asv_fma_mix_f32already does and as the hardware does. A product below 2^-126 is kept, as on hardware.v_fma_mix_f32is unchanged.v_fma_f16andv_pk_fma_f16are left to #2000 (no overlap: its new path is for non-mixv_fma_f16). #2025 adds theFP16_OVFLclamp to the sharedwriteF16call below the new mix path; it does not change the double rounding, and whichever of the two merges second should pass the sources to the mix path'swriteF16too.Measured on RDNA2 (gfx1036, Radeon 610M) with the hardware oracle in four modes: IEEE 0/1, DX10 clamp 0/1, f32 denormals kept/flushed, each with f16 denormals kept, f16 round to nearest even and
FP16_OVFLclear. The same values in all four modes unless noted:0xbfc00000,0x33800000,0x008000000x80010x80020x80010x62e9,0xd159,0x23ec0xf89f0xf89e0xf89f0x477ff000,0x3f800000,0xa6800000(65520 - tiny)0x7bff0x7c000x7bff0x00800000,0x3e800000,0x3f801000(product below 2^-126)0x3c010x3c000x3c010x7f7fffff,0x00000001,0(f32 denormals kept)0x00080x00000x0008They now need the Float64 capability, like
v_interp_p2_f16andv_dot2_f32_f16(TechnicalDebt).The hardware follows the f16 round and denormal fields of
FLOAT_MODEhere, not the f32 round field. The translation only does nearest even with f16 denormals kept, and so did the f32 FMA before, so another f16 mode now throws, as packed f16 already does, instead of giving a wrong result. Measured with IEEE=0 and f32 denormals flushed:FLOAT_MODEf16 fields0xbfc00000,0x33800000,0x008000000x80010x80020x80010xbfc00000,0x33800000,0x008000000x80010x80020x80010xbfc00000,0x33800000,0x008000000x80000x80020x3fc00000,0x33800000,00x00000x00020x3fc00000,0x33800000,00x00010x0002TechnicalDebt now also says that mixlo and mixhi always quiet a signaling NaN (
0x7f812345gives0x7e09at IEEE=0 and 1), unlikev_fma_mix_f32.Which title needs it
No specific title run. This completes #1878 for the f16 results in the 0xc0 mode titles use and the other measured modes above.
Tested
CachyOS Linux, GCC, Release; RTX 4070 Ti (NVIDIA) and Radeon 610M (RADV).
main3bd74c1 plus this change: 514/514 on NVIDIA, RADV and lavapipe (the upstream CI driver).vector_fused_multiply_addnow decodes and translatesv_fma_mixlo_f16,v_fma_mixhi_f16andop_sel_hi:[1,1,1], and requires no f32 FMA and one exact product-sum.agc_driver_mix_precisionadds 64 hardware rows for f32, f16 and mixed sources; 60 of them differ from the hardware before the fix, on both drivers. Both fail without the fix (0xabcdf89e, expected0xabcdf89f) and pass with it, under 1 s.vector_fused_multiply_addalso checks that f16 denormals flushed or f16 round toward -infinity throw, and that a set f32 round field does not.check_conventions.py,git diff --check.FP16_OVFLset. Other f16 modes were measured only to confirm they differ; they throw.Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed in TechnicalDebt.mdfiles or images added to the repository (attach them to this PR)🤖 Generated with Claude Code