Repository navigation
fix(shader): convert f16 bits to f32 without UnpackHalf2x16 - #1463
Conversation
On the AMD Windows driver (Radeon RDNA2 iGPU, 32.0.21030.2001) UnpackHalf2x16 of a byte-selected half converts the unmasked source register: an SDWA BYTE_0 source of 0xfff0fff0 reads as f16 0xfff0 instead of 0x00f0, so agc_driver_sdwa_float_selectors and agc_driver_sdwa_ldexp_f16 fail there. EmitF16BitsToF32 now decodes sign, exponent and mantissa with integer operations, scaling subnormals by 2^-24, which is exact for every half and leaves the driver nothing to fold into the conversion.
|
NaN rows for
The hardware keeps the sign and the payload ( |
|
Thanks for the hardware rows. That matches what the decode does now: sign and |
MarcZX10
left a comment
There was a problem hiding this comment.
Reviewed the complete diff and the full Linux and Windows Build checks. The integer sign/exponent/mantissa path avoids the AMD UnpackHalf2x16 behavior while preserving normal, subnormal, zero, infinity, and NaN payload representations; the reported device runs cover the affected tests and the full suite. AI-assisted review.
|
Full-range check on RDNA2 (gfx1036, Radeon 610M) with I then ran the same 65,536 halves through the recompiler (low half and shifted high half, guest wave64 and wave32, 262,144 results per device) and compared with the hardware rows, on Linux with this PR merged onto
So the new decode equals the hardware for every half at IEEE=0. The float16, |
|
Thanks for the full sweep. The unmasked-source result only reproduced with AMD's Windows driver (32.0.21030.2001) here, so matching RADV fits. |
|
Data point from a discrete RDNA 2 card on a newer driver: this fixes both tests on an AMD Radeon RX 6700 XT (Navi 22, 0x73df), Adrenalin 25.10.45.05 (driver 32.0.21045.5002), Windows 11 Pro 23H2, WinLibs GCC 15.2.0 r7, Release. Cherry-picked
Full AI-assisted (Claude Code). |
|
Another data point for this one, on Windows: Windows 11, WinLibs GCC 15.2.0 r7, Release, merged onto
AI-assisted (Claude Code); I ran this on my machine. |
…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.
…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. (cherry picked from commit e4f212b) Fork: request version 21 kept from upstream, on top of version 20 (paClVsOutCntl) from the AGC slice. The Vulkan device half (VulkanDevice.cpp) and the libSceAgcDriver tests are dropped; on Metal float16DenormPreserve stays false, so the integer f16 decode is kept. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* perf(shader): search a bindless image table once per access EmitImage selects the table slot of a bindless image before it branches over the image's runtime modes, and EmitImageMode then searched the table again in every mode arm. Each search is a binary search of dependent flattened_srt loads, so every bindless sample ran two. The mode arms now take the selection EmitImage already made; the modes copy the base image, so the mapping offset, root and iteration count the search reads are the same. (cherry picked from commit d759a43) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(agc,shader): prime the two host shared mutexes added after 48dcaac Commit 48dcaac introduced PrimedSharedMutex and converted every host std::shared_mutex that existed at the time, because winpthreads initializes a static rwlock on first use and a concurrent first lock returns EINVAL, which std::shared_mutex ignores (docs/dev/TechnicalDebt.md:10). Two host mutexes were added afterward and missed the conversion: - Recompiler.cpp registrationModule() static mutex (added by baf3437) - MultisampleTarget.cpp guardedMutex() static mutex (added by d109f39) Apply the same priming. Recompiler.cpp reuses the PrimedSharedMutex already defined in that translation unit; MultisampleTarget.cpp gets the identical local helper to match the existing inline precedent. Tested: Windows 11 x64, WinLibs GCC 15.2.0 (matches CI). Compiled Recompiler.cpp.obj and all three MultisampleTarget.cpp.obj targets with the project build flags; check_conventions.py passes. (cherry picked from commit 4d37c93) Fork: MultisampleTarget.cpp hunk left to the AGC slice. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * perf(shader): drop the input flush that rcp, rsq, sqrt, exp and log results never use The SPIR-V emitters of FPRecip32, FPRecipSqrt32, FPSqrt, FPExp2 and FPLog2 flushed a denormal input to its signed zero before the host operation. Since #537, unaryFloatSpecials replaces the host result for every input below 2^-126 with the hardware's result for a zero of the same sign: rcp and rsq give a signed infinity, sqrt the signed zero, log -inf and exp 1.0. floatUnary and float16Unary both go through it, so the flushed input could only change a value that is then discarded. v_rcp_iflag_f32 gets the same special values and already went without the flush. The pixel prologue computes PositionW as FPRecip32 of FragCoord.w without those special values, so it now flushes FragCoord.w in the IR first and gives the same result as before. (cherry picked from commit 1dc99ba) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(shader): split a BDA span read's in-place loads from its fallback On an RTX 3060 (Windows, driver 591.86) a dispatch never finishes when the lanes of read_bda_span split between its in-place dword loads and its per-dword byte fallback, which calls read_bda_dword_bytes: Windows resets the GPU and the device is lost. The two were the arms of one selection in the DontInline function. They are now two selections, the loads under the span's condition and the fallback under its negation, joined after both. Loads, lookups, faults and stops are unchanged for every lane; only the SPIR-V control-flow shape changes (SPIR-V specification 2.11, structured control flow). Every stage except mesh shaders is emitted as SPIR-V 1.3, and OpSelect chooses between composites with a scalar condition only from 1.4 on, so the join stays a selection. The same hang and fix were reported in boykopovar#3291 on an RTX 4060 Laptop GPU (Windows, driver 591.86) and an RTX 5060 (Linux, driver 590.48.01). An RTX 4070 Laptop GPU on driver 616.92 does not hang with either shape. The driver-side cause is unknown, so the shape is recorded in docs/dev/debt/shader.md. On main the hang fails nine tests on the RTX 3060, in two ways. agc_driver_{flat,global}_loads{,_x2_x3,_x4} and agc_driver_flat_apertures_{2,5} abort after 64 to 71 s: the lost batch keeps its lease, so teardown waits 60 s for the pinned guest allocation. agc_driver_pixel_interlock fails after about 4 to 7 s with vkDeviceWaitIdle returning VK_ERROR_DEVICE_LOST (-4) and never reaches the pin wait. All nine pass with the change. Fixes boykopovar#3291 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit f1b2e60) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(shader): load a BDA span in place from its own function read_bda_span chooses between the span's in-place dword loads and its per-dword byte fallback, which calls read_bda_dword_bytes. Each shape of that choice so far hangs a group of NVIDIA drivers: a dispatch never finishes and the device is lost. As the two arms of one selection, with the loads emitted in the arm, it hung drivers 591.86 and 590.48.01 when lanes took both arms (boykopovar#3291). As two selections, the loads under the span's condition and the fallback under its negation (boykopovar#4759), it hangs 595.97, 596.36, 610.62, 610.74 and 610.57.04 (boykopovar#4975). The loads now live in load_bda_span, a DontInline function that calls nothing, and read_bda_span is one selection again: one arm calls load_bda_span, the other is the fallback as before. Probe, loads, lookups, faults and stops are unchanged for every lane. On an RTX 4090 (Linux, driver 610.57.04) the two-selection shape hangs agc_driver_scalar_wide_loads and agc_driver_flat_apertures_0..13 in a build with SPIRV-Tools, and agc_driver_bda_span_reads and agc_driver_pixel_interlock in a build without. All pass with this change in both builds. Drivers 591.86, 590.48.01, 595.97, 596.36, 610.62 and 610.74 are not tested; a shape with the loads in their own DontInline function was reported passing on 591.86 in boykopovar#3291. agc_shader_memory now requires that a span read function holds no load and calls its load function once, and that the load function holds the four loads and no call. Refs boykopovar#4975 (cherry picked from commit 8c04efc) Fork: libSceAgc* and docs/dev/debt hunks dropped (AGC slice / file absent). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * perf(shader): convert f16 operands with UnpackHalf2x16 where f16 denormals 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. (cherry picked from commit e4f212b) Fork: request version 21 kept from upstream, on top of version 20 (paClVsOutCntl) from the AGC slice. The Vulkan device half (VulkanDevice.cpp) and the libSceAgcDriver tests are dropped; on Metal float16DenormPreserve stays false, so the integer f16 decode is kept. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * perf(shader): skip the f16 NaN rebuild for operands that are only compared With float16DenormPreserve, EmitF16BitsToF32 rebuilds the bits of a NaN half after UnpackHalf2x16, because NVIDIA returns the default NaN and Intel quiets it. When every use of the converted operand is an f32 comparison or a NaN test, directly or through abs and neg, only whether it is NaN matters, so the conversion returns UnpackHalf2x16's result as is. v_cmp_*_f16 goes from 11.5 to 4 SASS on an RTX 4060. agc_driver_f16_to_f32_unpack adds v_cmp_u_f16 and v_cmp_lt_f16 over all 65,536 halves with each decode the device supports. Its SPIR-V check requires the v_cvt_f32_f16 conversion to rebuild the NaN bits and the compared conversions not to. (cherry picked from commit 352bf30) Fork: libSceAgc* and docs/dev/debt hunks dropped (AGC slice / file absent). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(shader): keep cube image gradients out of the face coordinate bias image_sample_d and image_sample_cd on a cube pass their gradients through the same conversion as the face coordinates, which subtracts 1.0 to move AMD's [1, 2] face range to [0, 1]. A gradient is a difference of coordinates, so the bias turned a one-texel gradient into one of about -1 and the sample read the smallest mip. Gradients now skip the bias. agc_driver_image_sample_cube_derivatives samples every face of a 16x16 cube at five levels through image_sample_d and checks the level and face it read. TechnicalDebt records that the gradient handling follows Mesa and was not measured on hardware. (cherry picked from commit 73eaa1f) Fork: libSceAgc* and docs/dev/debt hunks dropped (AGC slice / file absent). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(shader): declare the GDS buffer coherent GDS is one memory for the whole GPU on AMD hardware: every work-group's ds_read and ds_write with gds see the others' writes, with no cache in between. The recompiler keeps it in a storage buffer that was not decorated Coherent, so on a GPU whose L1 caches storage buffer loads, NVIDIA's from Turing on among them, a plain GDS read could return a value cached before another work-group's GDS write. Atomics were not affected. The gds buffer is now declared Coherent. verifyCoherentGds in agc_shader_memory compiles a ds_read_b32 and a ds_write_b32 on GDS and checks that the gds buffer is Coherent. (cherry picked from commit 0998eb4) Fork: libSceAgc* and docs/dev/debt hunks dropped (AGC slice / file absent). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * perf(shader): reclaim unreachable cached source IR values (cherry picked from commit 57b2dc8) Fork: libSceAgc* and docs/dev/debt hunks dropped (AGC slice / file absent). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(shader): read the ancillary render target index from Layer when the draw exports it (cherry picked from commit 7015c04) Fork: the DrawDecode.cpp line that sets layerExported is in libSceAgcDriver and left to the AGC slice; until it lands layerExported stays false and the ancillary render target index reads as 0, as before. Fork ShaderDiskCache format kept (bumped once at the end of the slice); libSceAgcDriver tests and docs/dev/debt dropped. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(shader): serialize the pixel layer export flag from request version 23 main allocates request version 22 to the tessellation patches per group, so a version 22 request has no layer export byte. (cherry picked from commit de5279b) Fork: request version 23 kept to match upstream; version 22 (patchesPerGroup, from AGC commit 25a8372) is not in this tree yet, so the fork writes no version 22 field. libSceAgcDriver test hunk dropped. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(shader): bump the shader disk cache format for the sync-8 shader slice The ported recompiler changes add layerExported to the cached pixel info and change generated shader output, so cached entries from format 31 are not reused. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Karan Vora <52519664+karanvora2599@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Mohamed Aziz Hadjkacem <mohamedaziz.hadjkacem21@gmail.com> Co-authored-by: fdfawef324 <287205748+fdfawef324@users.noreply.github.com> Co-authored-by: Dean Galvin <dean.galvin@live.com> Co-authored-by: eljackjoestar-cmd <eljackjoestar@gmail.com> Co-authored-by: TideOPL <obinewman101@gmail.com>
What
On the AMD Windows driver,
UnpackHalf2x16of a byte-selected half converts the unmasked source register.agc_driver_sdwa_float_selectorsandagc_driver_sdwa_ldexp_f16fail on an AMD RDNA2 iGPU (both pass on NVIDIA, and CI has no AMD device):Probing the ldexp shader's SPIR-V on both GPUs (lane 2, input 0xfff0fff0):
BitFieldUExtract(x, 0, 8) & 0xffffUnpackHalf2x16(that).xIn all 11 failing lanes the result is the unselected low half quieted. Replacing the extract with shift+mask, dropping the redundant
& 0xffff, or feeding the half through the high word does not change the AMD result.EmitF16BitsToF32now decodes sign, exponent and mantissa with integer operations and scales subnormals by 2^-24 (exact in f32). Checked bit-exact against IEEE f16→f32 for all 65536 halves, including ±0, subnormals, infinities and NaN payloads.Tested
Windows 11 (10.0.26200), MinGW-w64 GCC 15.2.0 (WinLibs r7), Release,
-DBUILD_TESTING=ON, fullctestwithANYPS5_GPUset to each device: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)