Repository navigation
perf(shader): convert f16 operands with UnpackHalf2x16 where f16 denormals are kept - #3988
Conversation
|
Tested and verified on Linux (CachyOS x86_64) with an NVIDIA GeForce RTX 5060 Ti (Blackwell, driver 615.78.08, Vulkan 1.4):
Confirms support and correct behavior on Blackwell architecture and Linux. Great work! LGTM. |
Dextroid17
left a comment
Dextroid17
left a comment
There was a problem hiding this comment.
Verification of the new tests on two devices, and one correction to the merge-order note
What I built and ran. Your head 977116c3 in a clean worktree, Release with ANYPS5_ENABLE_SPIRV_TOOLS=ON, GCC 16. Ran on lavapipe (Mesa 26.2.4, LLVM 23.1.1 — the CI device) and on an RTX 3080 Ti (Ampere — a generation your body says you could not test).
lavapipe agc_driver_f16_to_f32_unpack_spirv PASS (GPU-free, so CI runs it)
lavapipe agc_driver_f16_to_f32_unpack PASS "… (65536 halves, integer decode only)"
lavapipe agc_shader_memory / agc_shader_disk_cache PASS
3080 Ti agc_driver_f16_to_f32_unpack PASS "… (65536 halves, both decodes)"
3080 Ti agc_driver_f16_to_f32_unpack_spirv PASS
The one line worth putting in the body is the 3080 Ti's: "both decodes" means the device reported float16DenormPreserve, so the UnpackHalf2x16 path ran with its NaN rebuild and every one of the 65,536 halves was compared against the exact decode, payloads included. That is the half you could not run yourself, and it holds on a second NVIDIA generation.
Negative control (mine). One token: force EmitF16BitsToF32 to always take EmitF16BitsToF32Decoded, tests kept.
agc_driver_f16_to_f32_unpack_spirv FAIL
AGC graphics: f16 to f32 unpack: v_cvt_f32_f16 lacks DenormPreserve 16 with float16DenormPreserve=true
git checkout 977116c3 -- core/shader/recompiler/SpirvBackend/src/SpirvModuleEmitter.cpp → PASS
That is the same failure your own DenormPreserve-off check reports — so this arm reproduces your control from an independent driver of the flag rather than adding a new failure mode. Worth saying plainly: what it establishes is that the SPIR-V test bites when the mode is dropped, not that it caught something new. In the same arm the device test still passes on lavapipe, which is correct rather than a gap: with the flag false the two forms are the same code there.
One correction: the version collision is a different PR. The body says "Request version 17 is also what #2440 used; whichever lands second takes 18." #2440 is closed and unmerged (2026-10-09) and never took 17 — its diff bumps the version 13 → 14, while its own body says 16. main is at 16 (version < 1u || version > 16u), so 17 is free today — but #1077 takes 17 as well, with the identical assertion string NVNQQREAAAA=. Naming #1077 instead would point a reviewer at the PR that can actually collide.
Series. #3557 is stacked on #3531 (its body says so: "Only the last commit is new"), and #4011 contains this PR's commit; #3590 merged earlier today. So one line in each body naming which of the others it overlaps would save a reviewer reconstructing the order — and #3531 was re-pushed during the day, so the shas a reader sees in an older review may no longer match.
What I could not verify. No AMD GPU here and no NVIDIA generation other than Ampere; and I verified equivalence, not your instruction counts — no APS5_PIPELINE_STATS before/after, so the 8–12 instructions per read and the runtime medians are your measurements, quoted rather than reproduced.
None of the above is a blocker on the devices I ran. The two things I could not close are the AMD gap (yours to state) and the version renumbering (#1077).
Hermes Agent 🤖 at your service — AI assistance: the build, the runs and this write-up were produced by an AI agent on the contributor's machine.
|
I read through this and tested it on Windows with an NVIDIA card and AMD's Windows driver, since the description mentions both as open points. Code
AMD Windows driver (the open question from #1463) Tested by reviewer
* On that AMD driver, on This adds Blackwell to the Ada results in the description. I didn't measure performance: neither game gets far enough on Looks good to me. AI-assisted: yes. Written with AI assistance (Claude Code), with the tests run on my machine; I reviewed it. |
|
Thanks @TideOPL, @Dextroid17 and @Kikoarl. Ampere and two Blackwell cards are now in the description. @TideOPL, the AMD Windows run settles the open point from #1463: amdvlk reports @Dextroid17 you're right, #2440 never reached 17. #1077 and #4031 both take 17 and change the same |
|
Rebase handoff — rebased onto current 8 files conflicted, all the same shape: Verified on Linux (CachyOS, kernel 7.2.9, GCC 16.2.1, Ninja, Release,
To pick it up: Head is |
…rmals are kept Since boykopovar#1463, every f16 operand read decodes sign, exponent and mantissa with integer operations, 8 to 12 instructions more per read than UnpackHalf2x16 on NVIDIA. The decode works around an AMD Windows driver that folds the mask of a byte-selected half into UnpackHalf2x16. A new target flag, float16DenormPreserve, is set when the device reports shaderDenormPreserveFloat16 and denormBehaviorIndependence ALL, the same rule as float64DenormPreserve. With it, EmitF16BitsToF32 uses UnpackHalf2x16 and declares DenormPreserve for 16-bit floats, under which SPV_KHR_float_controls keeps unpacked subnormal halves. NVIDIA returns the default NaN for every NaN half, so the NaN bits are rebuilt from the half: its sign at bit 31, its payload at bits 22:13. The DenormPreserve 16 mode is declared once, shared with the native f32 to f16 conversion. Without the flag (lavapipe; RADV reports 32_BIT_ONLY) the integer decode stays. Halves of fields narrower than 16 bits, such as SDWA byte selects, keep the integer decode on every device. The flag is serialized (request version 17) and part of the cache key. New test agc_driver_f16_to_f32_unpack converts all 65,536 halves, with nonzero high bits, through v_cvt_f32_f16, and byte-selected halves through v_add_f16 SDWA, with each decode the device supports, and compares the bits with an exact decode. agc_driver_f16_to_f32_unpack_spirv checks which decode the SPIR-V uses with and without the flag.
977116c to
e4f212b
Compare
|
Checked on RADV and CI's lavapipe, plus the HW oracle (gfx1035):
|
|
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.
|
|
Found a request-version collision during independent CPU-only public-source checks: #5132. All 12 fixtures emitted by original main075 as version21 throw Writer/max/gate-to-22 alone is incomplete: all 12 current21 inputs with F16=true then silently deserialize as false. The issue includes a synthetic reproduction; no title or GPU failure is claimed. Would you confirm this narrower compatibility rule: retain writer21, accept exactly zero remaining bytes as historical F16=false or one byte0/1 as the current F16 flag, and throw for other version-21 tails? It preserves both emitted layouts without taking #5097's version22. This is a proposal, not an implemented/tested repair. AI-assisted: yes. |
7 participants
What
On devices that can keep f16 denormals, f16 operands are converted to f32 with
UnpackHalf2x16again, instead of the integer decode #1463 introduced. Results are unchanged on every device, NaN payloads included.Background.
UnpackHalf2x16inEmitF16BitsToF32with an integer decode of sign, exponent and mantissa. The reason was an AMD Windows driver that folds the mask of a byte-selected (SDWABYTE_n) half into the conversion and converts the unmasked register.EmitF16BitsToF32runs for every f16 operand read (IRConvertF32F16), f16 image and format data, and f16 interpolation, on every GPU. On NVIDIA the integer decode costs 8 to 12 instructions more per read thanUnpackHalf2x16.UnpackHalf2x16alone isn't exact everywhere:DenormPreservefor 16-bit floats,SPV_KHR_float_controlslets a driver flush subnormal halves. None of the three drivers here does, but nothing stops one.0x7ffffffffor every NaN half (OpFConvertfrom a real f16 does the same), and the Intel UHD quiets them (0x7c01→0x7fc02000). The integer decode keeps sign and payload (0x7f802000).How.
float16DenormPreserve, set when the device reportsshaderDenormPreserveFloat16anddenormBehaviorIndependenceisALL: the same rulefloat64DenormPreserveuses since ebaa687. Like that flag, it is serialized (request version 17) and part of the cache key.EmitF16BitsToF32usesUnpackHalf2x16and declaresDenormPreserve 16.SPV_KHR_float_controlsrequires that mode to keep "denormalized values obtained via unpacking an integer". The mode is declared once per module, shared with the native f32 → f16 conversion of feat(recompiler): convert f32 to f16 natively when the device rounds 16-bit floats to nearest even #1252.((int)(half << 16) >> 3) | 0x7f800000puts the sign at bit 31 and the payload at bits 22:13, which is what the integer decode returns.32_BIT_ONLYbecause AMD sets f16 and f64 denormals with one register field.BitFieldUExtract/SExtractof fewer than 16 bits under theConvertF32F16, which is how SDWA byte selects arrive) keep the integer decode on every device. fix(shader): convert f16 bits to f32 without UnpackHalf2x16 #1463's case can't come back even if AMD's Windows driver reportsALL; I couldn't check what it reports.Which GPUs.
shaderDenormPreserveFloat16on Pascal through Blackwell, Pascal included even though it has noshaderFloat16, andINDEPENDENCE_ALL(NVK reportsALLfor every generation it supports). The Intel UHD here reports both too.agc_driver_f16_to_f32_unpackpasses with both decodes:32_BIT_ONLY, so it keeps the integer decode, and the test passes there with "integer decode only".Before / after on an RTX 4060 Laptop (Ada, driver 591.86). SASS instructions per instruction from
APS5_PIPELINE_STATSbinary size over a chain of 16,FLOAT_MODE0xc0:mainUnpackHalf2x16without the NaN rebuildv_cvt_f32_f16v_cmp_*_f16v_add/sub/mul_f16v_fma_f16v_max/min_f16v_med3_f16v_pk_add/mul_f16v_pk_fma_f16v_fma_mix_f32(f16 sources)v_cvt_u16_f16The f16 unary instructions (
v_floor/ceil/trunc/rndne/fract/rcp/sqrt/rsq/exp/log/sin/cos_f16) each lose 8. 55 opcodes change; every other opcode in the sweep compiles to the same size.Run time, dependent chains over 16,384 groups of 32 lanes, median of 15 interleaved dispatches in one process, flag off vs on:
v_cvt_f32_f16× 2048v_fma_mix_f32× 2048v_max_f16× 2048v_pk_add_f16× 2048v_mul_f16× 2048v_add_f16× 2048v_pk_fma_f16× 1024Why results don't change.
DenormPreserve 16,UnpackHalf2x16returns that exact value for every non-NaN half, subnormals included, and like the integer decode it reads only the low 16 bits.sign << 31 | 0x7f800000 | payload << 13, the integer decode's special-value branch.main.Tested
Windows 11, MinGW-w64 GCC 15.2.0, Release,
mainat a11c8f7 + #3291's fix (b54fe62) so that the BDA tests don't hang on driver 591.86. RTX 4060 Laptop (driver 591.86), Intel UHD Graphics (Raptor Lake, driver 101.6790), lavapipe from Mesa 25.2.8 (the version CI installs) and 26.2.4 (Windows builds,GALLIUM_OVERRIDE_CPU_CAPS=avx).agc_driver_f16_to_f32_unpack:v_cvt_f32_f16, and the byte-selected half of each lane throughv_add_f16_sdwa ... src0_sel:BYTE_1with −0.agc_driver_f16_to_f32_unpack_spirv(no GPU needed, so CI runs it). With the flag, thev_cvt_f32_f16program declaresDenormPreserve 16and has oneUnpackHalf2x16. Thev_add_f16SDWA program has one too, for its whole-registerv7, while the byte-selectedv4is decoded with integers. Without the flag, neither program has either.agc_driver_f16_to_f32_unpackfails on the RTX 4060 (v_cvt_f32_f16 of 0xd9a47c01 is 0x7fffffff, expected 0x7f802000) and on the Intel UHD (is 0x7fc02000).DenormPreserve 16mode,agc_driver_f16_to_f32_unpack_spirvfails (v_cvt_f32_f16 lacks DenormPreserve 16 with float16DenormPreserve=true). The device test still passes on both GPUs, as neither driver flushes unpacked subnormal halves without the mode.UnpackHalf2x16makesagc_driver_f16_to_f32_unpack_spirvfail with oneUnpackHalf2x16more than expected.agc_shader_memoryandagc_shader_disk_cachecheck the flag's serialization, the version 17 prefix, legacy requests and the cache key.agc_driver_*,agc_shader_*,agc_spirv_*,vector_*,agc_masked_select,agc_validate_program,agc_denormal*,agc_float16_*; 325 tests) on a11c8f7 + b54fe62:agc_driver_emulated_color_compare(linux CI + NVIDIA:agc_driver_emulated_color_comparefails on main since #3438 (8-bit sRGB compare refused on the decode path) #3526, fixed onmainsince by fix(shader): allow constant-zero sRGB comparisons #3532).agc_driver_ds_swizzle,agc_driver_emulated_color_compare,agc_driver_f32_dot2_rounding(noFloat64; it throws instead of skipping) andagc_driver_vop1_float_unary(fract_f321 ulp off). The color compare is linux CI + NVIDIA:agc_driver_emulated_color_comparefails on main since #3438 (8-bit sRGB compare refused on the decode path) #3526; the other three fail the same way withmain's recompiler.agc_driver_emulated_color_compare,agc_driver_image_plane_address_on_line_implicit_lod(fixed onmainsince by test(agc): enter whole quad mode in the implicit-LOD 1D sample test #3922) and, on 25.2.8, the intermittentagc_driver_depth_fast_clear_submitsegfault.main0a8b1d3 (this branch): the new tests,agc_shader_memory,agc_shader_disk_cacheand the f16, SDWA, VOPC compare table, interpolation and mix tests (29) pass on the RTX 4060 and lavapipe 25.2.8, and on the Intel UHD all pass or are skipped exceptagc_driver_vop1_float_unary, as above.Limits.
readF16AsF32, on top of this conversion, so the two don't conflict.agc_shader_memoryassertions; whichever lands later takes the next version.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)