Repository navigation
fix(shader): round and flush f16 conversion results by FLOAT_MODE in every denormal mode - #5943
Conversation
…every denormal mode Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ha6Tsy62AFUyp9a5Mf7vjA
jotitadev
left a comment
There was a problem hiding this comment.
Run on an Apple GPU, since the PR was tested only on lavapipe.
GPUs tested: Apple M2 Max, MoltenVK 1.4.2 (Vulkan 1.4.357), macOS 27.2, x86-64 under Rosetta; also MoltenVK main built with KhronosGroup/SPIRV-Cross#2704 (keeps the sign of -0)
main does not run the GPU tests on macOS yet, so I tested on macos-port at b7eb6b6b (mugurc/AnyPS5) plus the ImportWatched change from #1, with this PR's own diff (merge-base..head 793224067) applied. Not main. Release build, LunarG Vulkan SDK 1.4.363.0. Suite: ctest -R "^agc" -E "agc_display_buffer_dcc|agc_driver_flip|agc_driver_lds_atomics64" -j1 --timeout 30. Without the PR, 13 tests fail on this setup (7 with the #2704 build), all known macOS failures listed in #1. The diff applies cleanly.
agc_driver_f16_conversion_float_modepasses with both MoltenVK builds.- With only the new test added to the unchanged branch, it fails with
v_cvt_f16_f32 round16 0 denorm16 0 inputs 0x33400000 0x00000000 is 0x00000001, expected 0x00000000, the error in the description. So the test runs on this device. - Suite: the same 13 failures with and without the PR (7 and 7 with the #2704 build). None new, none fixed.
agc_driver_f16_denormal_flushpasses.agc_driver_float16_roundingfails with stock MoltenVK both with and without the PR, and passes with the #2704 build. That is the known -0 issue from #1, not this change.
Written with AI assistance (Claude Code). I ran the builds and tests above on my machine.
|
Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.
|
|
Measured |
|
Independent validation of the merged FP16 fixture on main 16a0f50, through fork integration 8858902:
AI-assisted: Codex; author responsible. |
What
Two gaps in
v_cvt_f16_f32,v_cvt_f16_i16,v_cvt_f16_u16andv_ldexp_f16that TechnicalDebt recorded as unmeasured:v_cvt_f16_f32kept denormal halves; with round to nearest too, for example0x33400000gave0x0001instead of0x0000. On hardware, the result is a zero of its sign when the value, rounded in the mode with an unbounded exponent, is below 2^-14. So a value just below 2^-14 that rounds up to it stays0x0400, while flushing the exact value would give zero there.directedF16Roundingno longer depends on the denormal mode.writeF16Conversionapplies the measured flush after the conversion throughflushTinyF16Conversion; its threshold depends on the rounding direction and the sign.v_ldexp_f16keeps its own flush, which already matched.The
clampmodifier is still applied before the conversion; gfx1030 matches that in every mode. The TechnicalDebt entry for directed f16 rounding is updated accordingly.Source: measured on RDNA2, gfx1030 (
RX 6800, Windows 11, AMD driver HIP runtime), with the hardware oracle's kernel template.v_cvt_f16_f32, with and withoutclamp: 8,996 f32 inputs, both signs. Each half in the denormal range, around the smallest normal, around 1.0 and below the overflow boundary is taken exactly and at a quarter, half and three quarters of its step. Smaller powers of two, f32 denormals, overflow values, infinities and NaNs are included.v_cvt_f16_u16,v_cvt_f16_i16andv_cvt_f16_u16withclamp: every 16-bit value.v_ldexp_f16(with and withoutclamp): 38 halves × 65 exponents.Tested
Ubuntu 24.04, GCC 13.3, lavapipe, on
maina453108c.agc_driver_f16_conversion_float_mode(new) pins 62 measured rows over the five forms, each with an expected value for all 16 modes.mainit fails (v_cvt_f16_f32 0x33400000gives0x0001at round16 0, denorm16 0); with the change it passes in 1.4 s.v_cvt_f16_f32, 29,184 forv_cvt_f16_u16and 366 forv_ldexp_f16.agc_driver_float16_roundingandagc_driver_f16_denormal_flush.Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed in TechnicalDebt (n/a).mdfiles or images added to the repository (attach them to this PR)🤖 Generated with Claude Code
https://claude.ai/code/session_01Ha6Tsy62AFUyp9a5Mf7vjA