Repository navigation
fix(shader): read s_memtime from the device clock where RADV's subgroup clock is 20 bits - #1383
Conversation
|
Tested on the two RADV devices in my machine. One is in the "not tested" list (GFX12); the other is one of the affected families. Same host as the PR: Linux 7.2.9 (CachyOS), Mesa 26.2.4, GCC, Release build of eb13861. RX 9070 XT (RADV GFX1201, GFX12)
Ryzen 7 7800X3D iGPU (RADV RAPHAEL_MENDOCINO, gfx1036, GFX10.3)
AI-assisted: the runs and this write-up were done by Claude (Claude Code) on my machine. |
…up clock is 20 bits On GFX10.3 to GFX11.7, ACO lowers a subgroup-scope OpReadClockKHR to s_getreg_b32 of SHADER_CYCLES, 20 bits zero-extended to 64, so s_memtime wrapped every 2^20 cycles and went backwards. Detect those families from the RADV device name and read the device clock there instead. Fixes boykopovar#985 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Rebased onto current Checked on the rebased head: build and full ctest green (453/453), the four PR tests pass, and they don't build without the change. On the hardware oracle (gfx1035) |
eb13861 to
f025a9e
Compare
What
s_memtimewent backwards on RADV with RDNA 2 and RDNA 3 GPUs (#985). It is emitted asOpReadClockKHRwith Subgroup scope. On GFX10.3 to GFX11.7, ACO lowers that tos_getreg_b32ofSHADER_CYCLES, which is 20 bits wide and zero-extended to 64. The counter therefore wraps every 2^20 cycles.This follows option 2 from the issue thread: on those GPUs only,
s_memtimereads the Device-scope clock. Every other device, including RADV GFX12, NVIDIA and Intel, emits exactly what it did before.src/amd/compiler/instruction_selection/aco_select_nir_intrinsics.cpp, casenir_intrinsic_shader_clock.gfx_level >= GFX12branch reads 64 bits (p_shader_cycles_hi_lo_hi).gfx_level >= GFX10_3branch readss_getreg_b32 ((20 - 1) << 11) | 29.src/amd/common/amd_family.h, familiesCHIP_NAVI21toCHIP_GFX1171. RADV reports the family in the device name as(RADV <family>), using the names fromac_get_family_name. I checked that these names appear in the installedlibvulkan_radeon.so(Mesa 26.2.4).s_memtimeis 64-bit on the GPU itself, so the narrowing comes from ACO.Changes:
NarrowSubgroupClock(driverID, deviceName)(newExecution/src/SubgroupClock.cpp) returns true only forVK_DRIVER_ID_MESA_RADVwith one of the GFX10.3 to GFX11.7 family names.VulkanDevicequeriesVkPhysicalDeviceDriverPropertiesthroughVK_KHR_driver_properties, because the instance is Vulkan 1.1.SpirvTarget::narrowSubgroupClockis passed to the emitter.EmitShaderClockusesScopeDevicewhen it is set.false.TechnicalDebt.md(Functional) records the trade-off. The device clock is monotonic and 64-bit but counts the 100 MHz reference clock, so on those GPUss_memtimedeltas are about 20 times smaller than on hardware. A family added to Mesa later keeps the subgroup clock until it is listed.agc_driver_shader_clocknow runs its dispatch 128 times. On an affected GPU, a wrap then lands between two reads almost every run, so the test catches a regression instead of failing about 5% of the time.Tests:
agc_subgroup_clock: family and driver matching, including malformed names, GFX10.1 and GFX12.agc_shader_memory:OpReadClockKHRscope fors_memtimewith the flag set and unset, and fors_memrealtime;agc_shader_disk_cache: the key changes with the flag.Overlap with #1182 and #1190 (flagged below): both also take request serializer version 10, for
scratchDwords, and append to the same trailer inRequestSerializer.cpp. This PR does not build on them and they do not build on it, soDepends onis none. Whichever merges second moves its field to version 11: write11u, acceptversion > 11u, read the field underversion >= 11u, and update the version pin inagc_shader_memory. I can rebase this one onto them if they land first.Fixes #985
Tested
Linux (CachyOS, kernel 7.2.9), AMD Radeon RX 6750 XT (RADV NAVI22, gfx1031), Mesa 26.2.4, GCC 16.2.1, Release.
main(df16c4c)agc_driver_shader_clock, original single dispatch, 300 runsmemtime: lane 0 went back from 1020093 to 17981etc.)agc_driver_shader_clock, 128 dispatches per runctest -R agc_ -j6(the repro from the issue), 10 runswent backctest -j8Shader clock driver=3 narrowSubgroupClock=1.tools/check_conventions.py --base upstream/mainpasses.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)