Repository navigation
perf(shader): skip the f16 NaN rebuild for operands that are only compared - #4011
Conversation
|
Tested on Linux (CachyOS x86_64, GCC) on an NVIDIA GeForce RTX 5060 Ti (Blackwell, driver 615.78.08, Vulkan 1.4):
Confirms correct comparison semantics and performance benefits on Blackwell and Linux. LGTM! |
Dextroid17
left a comment
There was a problem hiding this comment.
No blocker on the two devices I have. Your tests are what make this checkable on a card you did not test, and one mutation of the analysis plus the predicate bound below are all I am adding to them.
What I built and ran. Head 55044ae7 (which contains 977116c3), Release with ANYPS5_ENABLE_SPIRV_TOOLS=ON, GCC 16 — on lavapipe, the software device the Linux CI job uses (Mesa 26.2.4, LLVM 23.1.1 here), and on an RTX 3080 Ti (Ampere), which is not in your Tested list. (An independent run on a Blackwell card already covers the focused suites, so I have not repeated them.)
lavapipe agc_driver_f16_to_f32_unpack PASS "… (65536 halves, integer decode only)"
lavapipe agc_driver_f16_to_f32_unpack_spirv PASS
3080 Ti agc_driver_f16_to_f32_unpack PASS "… (65536 halves, both decodes)"
3080 Ti agc_driver_f16_to_f32_unpack_spirv PASS
On the 3080 Ti the skipped path is live, because both decodes can only print when target.float16DenormPreserve resolved true — which this code only sets when shaderDenormPreserveFloat16 is VK_TRUE and denormBehaviorIndependence is ALL (VulkanDevice.cpp:930). What the device run then shows is that the comparison semantics hold for all 65,536 halves under a NaN the driver produces — the compare arm tests booleans, so it cannot see whether the rebuild ran; the structural proof that it did not is the _spirv check, where the compare-only program requires nanRebuilds == 0 while unpacks != 0, and the convert program still requires its rebuild.
Negative control (mine). default: return true in OnlyCompared — every unrecognised user counts as compare-only:
lavapipe --verify-spirv FAIL f16 to f32 unpack: v_cvt_f32_f16 rebuilds 0 of 1 NaN results with float16DenormPreserve=true
3080 Ti device test FAIL f16 to f32 unpack (UnpackHalf2x16): v_cvt_f32_f16 of 0xd9a47c01 is 0x7fffffff, expected 0x7f802000
lavapipe device test PASS (no flag → the integer decode, so this arm cannot see the change)
restore 55044ae7 → both green again
Those are the two messages your own Negative checks already report, so this is an independent reproduction of your compare-only control on a third device, not a new failure mode.
The predicate bound. The IR's f32 predicate set is 14 opcodes, and OnlyCompared admits 13: FPIsNan32, six ordered, six unordered — and deliberately not FPCmpClass32. That exclusion is the conservative call rather than an omission: a class test reads the sign and kind of a NaN, which the default NaN the driver produces is not guaranteed to reproduce. So the list is neither accidentally narrow nor accidentally broad.
One inherited item: the overlap bot lists #4031 against this PR via tests/ShaderMemory.cpp. That comes from #3988's commit inside this branch, not from this PR's own four files, so it resolves with the rebase rather than being new work.
What I could not verify. No AMD GPU here, and the SASS instruction counts and the runtime table are your measurements — quoted, not reproduced; I did not measure the runtime effect myself.
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.
|
Thanks @Dextroid17 and @Kikoarl. Yes, leaving out |
|
Rebase handoff — note this PR contains #3988's commit underneath its own, so I stacked the rebase: Verified on Linux (CachyOS, kernel 7.2.9, GCC 16.2.1, Ninja, Release,
To pick it up (pull the base first): Heads: |
…pared 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.
|
Rebased onto current |
55044ae to
352bf30
Compare
What
Follow-up to #3988. Where an f16 operand's f32 conversion only feeds comparisons, it skips the NaN rebuild #3988 adds, because only whether the value is NaN matters there. Results are unchanged.
Background. With
float16DenormPreserve, #3988 converts f16 operands withUnpackHalf2x16. NVIDIA returns the default NaN0x7ffffffffor every NaN half and the Intel UHD quiets them, so #3988 rebuilds the NaN bits from the half (an arithmetic shift, an OR, a NaN test and a select). That keeps payloads exact where they reach a result:v_cvt_f32_f16, mixed-precision results, f16 arithmetic. A comparison never sees the payload, though, and f16 compares read two operands, so they pay the rebuild twice.How.
EmitConvertF32F16checks the users of theConvertF32F16value. If every user is an f32 ordered or unordered comparison (FPOrd*32,FPUnord*32) orFPIsNan32, directly or throughFPAbs32,FPNeg32orIdentity, it usesUnpackHalf2x16's result as is. Those users produce booleans that depend only on the value and on whether it is NaN, and abs and neg keep NaN a NaN. Any other user, and every other caller ofEmitF16BitsToF32(images, formats, interpolation), keeps the rebuild. Devices without the flag still use the integer decode.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:mainv_cmp_{lt,eq,le,gt,ge,o,u,nge,nlg,ngt,nle,neq,nlt}_f16v_cmp_lg_f16No other opcode in the sweep changes size. f16 arithmetic operands also feed the arithmetic, so they keep the rebuild.
Why results don't change. A comparison's result is a function of the two values, with every NaN comparing unordered.
FPIsNan32depends only on NaN-ness. Abs and neg change only the sign bit, so a NaN stays a NaN and a number keeps its magnitude. Which NaN bits the driver produced therefore can't reach any result.Tested
Windows 11, MinGW-w64 GCC 15.2.0, Release,
mainat 0a8b1d3 + #3988. RTX 4060 Laptop (driver 591.86), Intel UHD Graphics (Raptor Lake, driver 101.6790), lavapipe from Mesa 25.2.8 (Windows build,GALLIUM_OVERRIDE_CPU_CAPS=avx).agc_driver_f16_to_f32_unpack:v_cmp_u_f16of each of the 65,536 halves with itself andv_cmp_lt_f16against 1.0, checking every lane against the exact decode, once per decode the device supports (both on the RTX 4060 and the Intel UHD, the integer decode on lavapipe).v_cvt_f32_f16conversion to rebuild the NaN bits and the compared conversions not to.agc_driver_vopc_compare_tableruns every non-f64 VOPC compare, the f16 ones included (e32, e64, cmpx), against values measured on RDNA, with each device's own target.v_cvt_f32_f16 of 0xd9a47c01 is 0x7fffffff, expected 0x7f802000on the RTX 4060, andv_cvt_f32_f16 rebuilds 0 of 1 NaN resultsin the SPIR-V check.v_cmp_u_f16 and v_cmp_lt_f16 rebuilds 1 of 2 NaN results).agc_driver_*,agc_shader_*,agc_spirv_*,vector_*,agc_masked_select,agc_validate_program,agc_denormal*,agc_float16_*), without the BDA tests that hang on driver 591.86 until Recompiler: NVIDIA hangs (device lost) when read_bda_span lanes split between in-place loads and the fallback; branch and commit ready #3291 lands (agc_driver_flat_loads*,agc_driver_global_loads*,agc_driver_flat_apertures_*,agc_driver_pixel_interlock):agc_driver_ds_swizzle,agc_driver_f32_dot2_rounding(noFloat64; it throws instead of skipping) andagc_driver_vop1_float_unary(fract_f321 ulp off), which fail the same way withmain's recompiler (see perf(shader): convert f16 operands with UnpackHalf2x16 where f16 denormals are kept #3988).agc_driver_depth_fast_clear_submitsegfault, which also happens withmain's recompiler.Limits.
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)mainonce it merges)