Repository navigation
perf(recompiler): leave the descriptor-range comparison out of direct buffer loads when the device zeroes past the range - #7484
Conversation
… buffer loads when the device zeroes past the range With robustBufferAccess2 and a robustStorageBufferAccessSizeAlignment of 1, the device returns zero for a storage buffer load past the descriptor range, so the shader's own OpArrayLength comparison against the same range is redundant. The guest V# bound check stays. A guest buffer's and a draw snapshot's descriptor range of 4 bytes or more is rounded down to whole dwords so both zeros agree. APS5_NO_EXACT_ROBUST_LOADS keeps the comparison. The flag is part of the shader cache key.
|
Other open pull requests touch the same code. Merging one will break the other, or both implement the same thing. If this one builds on them, list them in Depends on.
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.
|
|
AMD on Windows: the tests this PR names pass on an RDNA 3.5 GPU. Radeon 860M (RDNA 3.5, driver 32.0.13058.8001), Windows 11 26300, WinLibs GCC 15.2.0 r7, Release.
AI-assisted |
|
Rebase handoff — rebased onto current Verified on Linux (CachyOS, kernel 7.2.9, GCC 16.2.1, Ninja, Release, To pick it up: Head is |
|
Thanks @blawesom. I checked @AlxcBTW since this is a rebase rather than a fast-forward, |
|
This pull request is labeled |
|
Ran this on GTA V (PPSA04263 01.010.002), Linux 7.2.8-zen1-2-zen, NVIDIA GeForce RTX 5060 Laptop GPU, NVIDIA 615.71.09 ( Frame p50: main 77.1, 77.6 ms, with #7484 75.4, 75.9 ms (-1.7 ms, -2.2 %). GPU busy per present: 62.4, 62.3 -> 60.4, 59.8 ms, 2.3 ms less, close to the 2.2 ms in the description. Summary lines (main, then the PR): |
What
A direct (non-BDA) buffer load in
readBuffer(SpirvRuntimeBuffer.cpp) has two zero conditions: the guest V# bound (byteAddress().inBounds, passed in asenable) and the descriptor range, which the shader compares with theOpArrayLengthof the storage-buffer block (EmitMemoryElementInBounds). WithrobustBufferAccess2the load is already a select (GuardedElementIndexpoints a refused load at element 0xfffffffe, where the device returns zero), but theOpArrayLengthstill reads the descriptor and the compare and the selects cost time on every load.The recompiler now leaves the descriptor-range comparison out of a direct buffer load when all of this holds:
robustBufferAccess2enabled (State::robustBufferAccess2: the device reports the feature androbustBufferAccessis enabled; since fix(agc): give AMD's proprietary driver robustBufferAccess2 again #8267mainno longer switches it off on AMD's proprietary driver, and this PR no longer refers to that check) and reportsrobustStorageBufferAccessSizeAlignment == 1(queried throughVkPhysicalDeviceRobustness2PropertiesEXT);APS5_NO_EXACT_ROBUST_LOADSis not set (like the otherAPS5_NO_*switches: set at all means off);LoadsSelectable, the same condition the select form has today).Then the load still selects on the guest V# bound (
enable), and the device produces the zero past the descriptor range. For a buffer that can be misaligned the high word keeps its comparison with the length (!unaligned || next < length;unalignedis a specialization constant, so an aligned pipeline folds the term and theOpArrayLengthaway). Where the device lacks the feature or the alignment is not 1, or the switch is set, the checked form is emitted as before. Stores, atomics, the coherent 64-bit load, GDS, LDS, scratch and BDA loads are not touched.The flag travels as
SpirvTarget::exactRobustLoadsintoSpirvTargetOptionsand the emitter state and is part of the shader cache key (CacheKey.hpp).ShaderResources::wholeDwordRangerounds the descriptor range of a guest buffer and of a draw snapshot down to whole dwords when the flag is on (ranges of 4 bytes or more;Context::exactRobustLoads). The entry "Guest memory load guards" indocs/dev/debt/shader.mdsays so, anddocs/dev/ENVIRONMENT.mdhas a row forAPS5_NO_EXACT_ROBUST_LOADS.Why the result is the same bytes:
floor(range / 4), and the device returns zero for a 32-bit load that leaves the range. The guest V# bound stays in the shader and selects the same fallback index, so the guest sees the same zero for raw, structured and swizzled loads as before.robustStorageBufferAccessSizeAlignmentreturns zero, and an access is out of bounds if any part of it is. With an alignment of 1 there is no padding in which the device could return memory contents, which is why the path needs the alignment to be 1. The bound is the descriptor range (VkDescriptorBufferInfo::range), not the size of theVkBuffer.floor) and, with alignment 1, outside for the device ("any part"); the host rounding of the range down to a multiple of 4 makes both agree even for an implementation that treats it differently. It removes no word a direct access can reach: every direct access (word load, word store, byte store through the u8 block, 64-bit access through the u64 block) is already gated byfloor(range / 4).Limits: equivalence to console behaviour is not measured on console hardware. The shader-side comparison of both variants on a device ("verify") is not in this PR; the argument above and the interpreter test below are.
Tested
main(the last is 16259fa, base 0c6a5bd), at 5095da6 (the PR head 16259fa plus the CI scaffolding commit; run 39678131804, ctest regexguarded_memory_loads|shader_disk_cache|recompiler|spirv): 20 of 20 passed, none skipped, among themagc_guarded_memory_loads(0.05 s),agc_shader_disk_cache(2.05 s) andagc_shader_disk_cache_large_file. This is a narrower set than the 712 tests of the earlier run (the wholeagc_*andshader_*set) at the pre-merge head, which are not repeated here. Not run on Windows.agc_guarded_memory_loadshas a new case, "exact robust loads read what the checked variant reads": the SPIR-V interpreter of that test runs the checked and the exact module of a raw 32-bit buffer load over 432 combinations (active or not, offsets 0 to 4092, 32 to 128 records, base misalignment 0 to 3, valid and invalid V#) and requires the same effective reads, where an index at or past the 16-word array counts as the device's zero. It also checks that an exact request withoutrobustBufferAccess2leaves the plain variant unchanged.agc_shader_disk_cachehas a new key case, "exact robust loads": the flag changes the cache key.lab/libs-robust2), arm a without the switch and arm b with it on (the lab switch for this change), so the change is the only difference between the arms. The device reportsrobustBufferAccess2true androbustStorageBufferAccessSizeAlignment1. All rowsAPS5_TEXTURE_CACHE_MIB=3072,APS5_NO_PAD_INPUT=1,APS5_PROFILE_GPU=1, and the same lab-only settings in both arms (a refresh lever and the depth clear HLE). The counters of the b rows say the change was taken: every direct load was compiled without the descriptor-range check (65536 without, 0 with).At the title the GPU busy time per present is 67.04 to 67.29 ms without the change and 64.86 to 65.13 ms with it in every row, about 2.2 ms less. That is the only figure I claim. Mean FPS at the title is 9.13 without and 9.21 with it, inside the spread of the rows: the title is CPU bound on this machine (frame p50 105 to 112 ms against 65 ms of GPU busy time), so a shorter GPU time barely moves the frame rate. The gameplay rows are memory noisy (31 GiB, two rows per arm): GPU busy 58.5 and 56.6 ms without, 53.4 and 53.4 ms with, which agrees in direction but I claim nothing from it. Pictures were not compared. Traces are on the lab machine and I can attach them on request.
python3 tools/check_conventions.py --base origin/mainpasses. Based on 0c6a5bd (merged into the branch).tools/pr_overlap.pylists open PRs that touch some of the same files (perf(agc): GPU descriptor heap (APS5_GPU_HEAP) #6947 and perf(agc): draw finisher pipeline (APS5_DRAW_PIPELINE) #6946ShaderResources, feat: run PS4 games on the AGC driver — Limbo (CUSA01664) in game (integration branch) #6977, feat: macOS on Apple Silicon host support (Rosetta 2 and MoltenVK) #7332 and Macos port #7201VulkanDevice.cpp,Recompiler.cppandSpirvRuntimeBuffer.cpp, perf(recompiler): keep runtime-specialized shader modules in the shader disk cache #6924, feat(shader): resolve nested image tables through runtime pointers #7223 and feat(shader): read samplers through nested and pointer-keyed descriptor tables #7395ShaderDiskCache.cpp, feat(agc): draw the last provoking vertex through the barycentric tessellation stages #7426Recompiler.hpp); none of them implements the same function.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)