Repository navigation
fix(shader): translate f32 conversions in f32 denormal modes 1 and 2 - #10595
TechLevelZero wants to merge 2 commits into
Conversation
MarcZX10
left a comment
There was a problem hiding this comment.
Reviewed the translation changes against the surrounding FLOAT_MODE handling and the reported hardware measurements. The f32 conversion set now uses the same denormal input-flush bit as the compare opcodes, while the explicitly unmeasured divide-scale/fixup and f16 MAD-mix opcodes remain rejected. Moving VCvtPkU8F32 to Directed32 is consistent with setRounding tracking the runtime mode. The new GPU test covers signed denormals and normals across the four modes and the extended round-mode test checks descriptor and runtime settings. Hosted conventions, intake and labels pass; the repository workflow did not run GPU/product tests, so the multi-GPU hardware results in the PR description are author-reported and were not reproduced locally.
|
One clarification: v_div_scale_f32 and v_div_fixup_f32 were measured. Mode 1 differs from mode 3 on 7,968 and 836 of the operand triples, so they stay rejected until their output flush is modelled. Only v_mad_mixlo/mixhi_f16 are unmeasured (gfx1036 lacks them). |
|
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.
|
imellag
left a comment
There was a problem hiding this comment.
Tested on a device without shaderFloat64 that flushes f32 denormals in hardware: Apple M4 Pro, macOS 27.0.1, MoltenVK 1.4.2 ([float-controls] f32 denormal flush: hardware), x86-64 Release under Rosetta 2. main does not build on macOS yet, so I applied this PR's code and test changes to #8658's macOS port, ANYPS5_REQUIRE_VULKAN=1 ANYPS5_NO_SHADER_CACHE=1 VK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation, 3 runs:
| without this PR | with this PR | |
|---|---|---|
agc_driver_vcvt_pk_u8_f32_round_mode |
passes | passes |
agc_driver_f32_denormal_flush |
fails (fused lane 8 no float mode v_fma_f32 v14, v4, v5, v6 is 0x00000000, expected 0x00800000, the hardware flush) |
fails the same way |
agc_driver_f32_convert_denormal_modes |
not present | fails before checking anything |
The new test stops at ShaderRecompiler::Recompile: 64-bit float instructions need the Float64 capability, which the device lacks: its shader includes v_cvt_f64_f32, and MoltenVK has no shaderFloat64. The f64 tests on main skip in that case (TargetHasCapability(device->Target(), spv::CapabilityFloat64) then VulkanTestSkipped, as in F64Conversions.cpp and DivisionOutputModifiers.cpp).
Skipping the whole test would lose the eleven f32-only conversions on exactly the kind of device CheckKept's "or flushed" branch is written for. As a local experiment only, I replaced v_cvt_f64_f32 with s_nop and left its two columns out of the checks: the test then passes 3 of 3 here, in all four modes at IEEE 0 and 1. So running v_cvt_f64_f32 in a second shader only when the target has CapabilityFloat64 would keep the rest covered on such devices.
From that run, in mode 3 the results whose kept and flushed values differ matched the kept value, except v_cvt_flr_i32_f32 of the five negative denormal inputs, which read 0 (the flushed value) instead of -1. That is the host's flush of the floor's input, which the test accepts as intended, and mode 3's translation is the same as on main.
No was not found but was requested line; the only validation message is VUID-VkDeviceCreateInfo-pProperties-04451 (MoltenVK's portability subset at device creation), with and without this PR.
AI-assisted: yes
The f32 conversions that only flush their inputs follow bit 0 of the FLOAT_MODE f32 denormal field, as the f32 compares do: mode 1 keeps denormal inputs like mode 3 and mode 2 flushes them like mode 0. v_cvt_pk_u8_f32 also follows a round mode set at run time. Measured on RDNA2 (gfx1036) with the hardware oracle. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…denormal test The conversion test failed before checking anything on a device without shaderFloat64, as its single shader included v_cvt_f64_f32. That instruction now runs in a second shader only when the target has the Float64 capability, so the other f32 conversions are still checked on such devices. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
8c339be to
55ac01b
Compare
|
Thanks for testing on MoltenVK, and for the experiment. I did what you suggested: Checked with
The |
imellag
left a comment
There was a problem hiding this comment.
Thanks, that covers it. Rechecked 55ac01bc1e on the same M4 Pro (MoltenVK 1.4.2, no shaderFloat64, f32 denormals flushed in hardware), the two commits applied to #8658's macOS port, same variables, 3 runs:
agc_driver_f32_convert_denormal_modespasses every time and printsv_cvt_f64_f32 skipped, the device has no shaderFloat64, so the eleven f32 conversions are now checked on this device too.agc_driver_vcvt_pk_u8_f32_round_modepasses;agc_driver_f32_denormal_flushfails as before, the same with and without this PR, as you said.- No
was not found but was requestedline; the only validation message is still MoltenVK'sVUID-VkDeviceCreateInfo-pProperties-04451at device creation.
AI-assisted: yes
What
A shader whose
FLOAT_MODEf32 denormal field is 1 or 2 failed to translate on any f32 conversion ("f32 denormal mode N is not implemented"), andv_cvt_pk_u8_f32also threw when a directed round mode was set at run time. Measured on hardware, these conversions only flush their f32 inputs, by bit 0 of the field, like the f32 compares:f32DenormalFlushFornow returns the compares' input flush forv_cvt_i32_f32,v_cvt_u32_f32,v_cvt_rpi_i32_f32,v_cvt_flr_i32_f32,v_frexp_exp_i32_f32,v_cvt_f64_f32,v_cvt_pknorm_i16_f32,v_cvt_pknorm_u16_f32,v_cvt_pk_u8_f32,v_cvt_pkrtz_f16_f32andv_cvt_f16_f32.v_cvt_pk_u8_f32is now aDirected32rounding use:setRoundingalready writes the tracked round mode intofloatMode, which its translation reads.v_div_scale_f32,v_div_fixup_f32andv_mad_mixlo/mixhi_f16still throw in modes 1 and 2.Source: measured on RDNA2, gfx1036 (AMD Ryzen 7 9700X integrated graphics, Fedora 44, Linux 7.2.7, ROCm 7.1.1 HSA runtime), with the hardware oracle's kernel template.
v_cvt_pk_u8_f32: 2,148 sources (k/8 for k = -16 to 2,099, the values next to 0.5, 1.5, 2.5, 254.5, 255.5, 256 and -0.5, ±1e10, denormals and minimum normals of both signs, infinities, quiet and signaling NaNs) at IEEE 0 and 1, wave32 and wave64, f32 denormal modes 0 to 3 and f32 round modes 0 to 3, each set once in the kernel descriptor and once at run time bys_round_modeands_denorm_mode: 128 configurations, all matching one model. The 38 vectors already pinned inVCvtPkU8F32RoundMode(measured on gfx1030) match gfx1036 in all four denormal modes.v_cvt_f64_f32,v_cvt_f16_f32andv_cvt_pkrtz_f16_f32were also run in every f16/f64 denormal mode and f16 round mode, with the same result.v_div_scale_f32(7,968 of 10,648 triples) andv_div_fixup_f32(836) differ between modes 1 and 3, so they are left throwing.v_mad_mixlo/mixhi_f16don't exist on gfx1036 and were not measured.Related: #5941, #6009 and #6010 change
f32DenormalFlushForfor other instructions (v_mullit_f32, min/max, add/multiply). This PR doesn't touch their case labels.Tested
Fedora 44 (host) / Ubuntu 26.04 containers, GCC 15, built on
mainac28b89bd(the branch rebases ontofa515def8without conflicts).agc_driver_f32_convert_denormal_modes(new): 32 operand pairs × 12 result words in each f32 denormal mode at IEEE 0 and 1. Modes 0 and 2 must equal the measured flushed values. Mode 3 must equal the measured kept values, or the flushed value on a driver that flushes f32 denormals. Mode 1 must equal mode 3 on the same device.agc_driver_vcvt_pk_u8_f32_round_mode(extended): all four f32 denormal modes, and each round mode set bys_round_modein the shader as well as inFLOAT_MODE.agc_driver_f32_denormal_flushstill refuses modes 1 and 2 for its other instructions.ANYPS5_REQUIRE_VULKAN=1,ANYPS5_NO_SHADER_CACHE=1andVK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validationon the three GPUs below. The validation layer loaded on each, and there are noVUID-messages.tools/select_tests.pyselects for this change ran onmainand on the branch (ctest -V -j6, same environment): no newVUID-messages, and the same failures on both sides apart fromagc_driver_store_idle_resultsandagc_driver_store_idle_results_wake, which failed once on the branch under parallel load and then passed 5 of 5 runs alone on bothmainand the branch. The 27 failures on both sides are mostly tests that need guest write watching, which the container's seccomp profile blocks (userfaultfd failed (Operation not permitted)).python3 tools/check_conventions.py --base origin/main: 0 errors. The twodebt-lengthwarnings are on entries 88 and 128, which were already over 1,000 characters onmain.Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file.mdfiles or images added to the repository (attach them to this PR)🤖 Generated with Claude Code