Repository navigation
perf(agc): store whole words of 1- and 2-byte texels in the detile shader - #6905
Merged
Merged
Conversation
|
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.
|
CnRJay
reviewed
Oct 16, 2026
CnRJay
left a comment
CnRJay
left a comment
Contributor
There was a problem hiding this comment.
Good PR: the microbenchmark is clear (buffer detile 1-byte SW_64KB_S 228.9 to 37.9 us, 6x; image retile 2.2x), unchanged SPIR-V for 4/8/16-byte elements is shown (0.98-1.05x), the fallback to the per-element atomic path keeps the bytes identical, and the test compares every byte against a CPU reference including bytes no element maps to.
- The title runs (Minecraft Dungeons II, Sword of the Sea) show no change, which the description explains. The gain is therefore real only for titles that write back 1- or 2-byte standard-layout images each frame. Name one, or say it is a latent improvement; the speed-up claim in the title shouldn't read as a game-level one.
- Two new shader variants (
WORD_GROUPS, buffer and image) are compiled for every title that detiles 1- or 2-byte formats. Please state the extra compile time on a cold cache and whether the variants are built on first use or prewarmed (the journal/prewarm path has to know them, otherwise they compile on the queue worker at the first write-back). - The groups use workgroup shared memory and a barrier. Post the shared memory use per workgroup, since it caps occupancy on some devices (the RTX 4060 only was measured); an AMD or Intel run of
agc_detile_small_elementswould show the barrier path is correct there too.
12 tasks done
Contributor
|
AMD / Windows run of this PR at 1061179: Radeon 780M (RDNA 3, 0x15bf), AMD driver 25.10.20.01 (32.0.21020.1007), Windows 11 Home 10.0.26100, WinLibs GCC 15.2.0 r7, Release.
No benchmark run on this device. AI-assisted: yes (runs made on the device above). |
Contributor
Author
|
@CnRJay thanks for the review. On the three points: Latent, not game-level. Agreed. I have no title yet whose frames are bound by 1- or 2-byte detiles: Minecraft Dungeons II and Sword of the Sea write back other formats each frame, so they show no change. I've changed the description to say that this is a throughput fix for those paths and that it makes no frame-rate claim. The title only names what the shader does. Compile cost and prewarm.
Shared memory and other vendors. AI-assisted: yes (Claude Code) |
jotitadev
reviewed
Oct 18, 2026
jotitadev
left a comment
jotitadev
left a comment
Contributor
There was a problem hiding this comment.
Apple GPU run of the new word-group detile variants. This is a third vendor after NVIDIA and AMD, and Apple GPUs differ here in threadgroup memory and in subgroup width (32).
Machine: Apple M2 Max, macOS 27.2, x86-64 build under Rosetta. Head 1061179. Method: macos-port @ b7eb6b6 + ImportWatched + PR diff, not main. The shader, TextureDetiler.cpp and the shader build rules applied cleanly. Three conflicts came from code the port doesn't have, and I resolved them by hand:
-
TextureDetiler.hpp: added the two new module handles and kept the port's image descriptor pools (the port has noDescriptorRing). -
CMakeLists.txt: added theagc_detile_small_elementstest next to the port's tests. -
Recorder.cpp: inserted theSmallElementDetileTestnamespace and the--small-element-detile-onlydispatch without the other recorder tests from main. -
agc_detile_small_elementspasses on MoltenVK 1.4.2 (small element detile tests passed) and on MoltenVK main + SPIRV-Cross #2704. With--repeat until-fail:10it passes 10/10 on each driver. -
The word path really runs on this device: with the whole-word store in
TextureDetile.compchanged togroupWords[slot] ^ 1u(local mutation, reverted afterwards), the test fails at its first case:SW_64KB_S 1B 37x21, whole mip, buffer retile: byte 0 is 10, expected 11. So the host selects the word variant on Apple, and the test catches a wrong combined word. -
Neighbouring tests pass:
agc_storage_baseline_settled,agc_storage_upload_lifetime,agc_storage_upload_failureandagc_unit_shadow.
No timing measured.
GPUs tested: Apple M2 Max (MoltenVK 1.4.2 and MoltenVK main + SPIRV-Cross #2704)
Written with AI assistance (Claude Code). I ran the builds and tests above on my machine.
github-actions
Bot
added
perf
parl0v
force-pushed
the
perf/detile-word-copies
branch
from
October 18, 2026 20:48
1061179 to
283b057
Compare
|
This pull request is labeled |
github-actions
Bot
removed
needs rebase
Contributor
Author
|
Traces of the performance capture in the description (they open in ui.perfetto.dev): |
Contributor
|
Follow-up to my Apple run above: the full suite, which I left out there, is now done. Full The new head 283b057 is a rebase: Written with AI assistance (Claude Code). I ran the builds and tests above on my machine. |
parl0v
force-pushed
the
perf/detile-word-copies
branch
from
October 19, 2026 07:08
283b057 to
33b1953
Compare
github-actions
Bot
added
needs rebase
Collaborator
|
@parl0v this now conflicts with Could you rebase onto current |
parl0v
force-pushed
the
perf/detile-word-copies
branch
from
October 21, 2026 01:11
33b1953 to
f41aa95
Compare
Contributor
Author
|
@oneandonlydean rebased onto current RTX 4060 (NVIDIA 617.42, Windows 10 19045), with |
12 tasks done
12 tasks done
12 tasks done
This was referenced Oct 25, 2026
Contributor
|
Linux run of this head
AI-assisted: yes (Claude Code) |
Contributor
✅ NVIDIA GPU test: passedPR head
Unstable on main (1, these also failed in recent main runs on this machine; not counted against this PR)Failing on main too (5, not counted against this PR)Validated but not compared (1, not in the main baseline's validation pass)Validation IDs already present on main (4 IDs in 436 tests)Skipped (39)ReproduceAutomated build and test on a contributor's NVIDIA machine; it does not replace upstream CI. A new comment is posted for each new push. Full logs. |
12 tasks done
12 tasks done
12 tasks done
# Conflicts: # core/libs/prx/libSceAgcDriver/CMakeLists.txt # core/libs/prx/libSceAgcDriver/Graphics/shaders/TextureDetile.comp
This was referenced Nov 1, 2026
Contributor
Verdict: should-fix
Freshness Envelope
Cross-PR Ledger
|
This was referenced Nov 1, 2026
Contributor
✅ NVIDIA GPU test: passedPR head
Unstable on main (2, these also failed in recent main runs on this machine; not counted against this PR)Failing on main too (3, not counted against this PR)Validated but not compared (1, not in the main baseline's validation pass)Validation IDs already present on main (5 IDs in 484 tests)Skipped (45)ReproduceAutomated build and test on a contributor's NVIDIA machine; it does not replace upstream CI. A new comment is posted for each new push. Full logs. |
12 tasks done
12 tasks done
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
TextureDetile.compmoves one element per invocation. For 1- and 2-byte elements (R8, R16 and the other 8/16-bit formats) the destination word is shared by 4 or 2 invocations, so every element is written with anatomicAnd+atomicOrpair on that word (copyElementin the buffer paths,loadElementin the image retile). Neighbouring invocations hit the same word, and those paths run at 9-44 GB/s on an RTX 4060 against 253 GB/s for 4-byte elements.-DWORD_GROUPS=1, buffer and image) combine the elements of each destination word in workgroup shared memory and store the word once. Each invocation still locates its own element (locate, the per-element addressing split out ofmainunchanged); after a barrier, a group of 4 (1-byte) or 2 (2-byte) neighbouring invocations whose elements are all in the window's range and land in one aligned 4-byte word, in order, store that word with a plain write. Any other group (a row or window edge, a partial word at a range boundary, an unaligned base) falls back to the existing per-element atomic path, so the bytes written are exactly the old ones.TextureDetiler::pipeline) picks the word variants only where they help: 1- and 2-byte buffer detiles (the linear destination always keeps neighbours in one word), and retiles of the linear and standard (non-equation) layouts. Equation-swizzled retiles (_X, thick) keep the old shader: their tiled words hold elements of other rows, and grouping them measured slower. 4-, 8- and 16-byte elements are unchanged and use the same SPIR-V as before.agc_detile_small_elements(agc_driver_recorder_tests --small-element-detile-only) runs the buffer retile, buffer detile and image retile on the device for 1- and 2-byte elements overSW_64KB_S,SW_4KB_S,SW_64KB_R_Xand linear, odd sizes (37x21, 70x9, 37x5), a tail mip of each 64 KB mode, and three windows (whole mip, a byte range with unaligned ends, columns 3 to width-2). Every byte of each destination is compared with a CPU reference (the swizzle equations for_X, the shader's standard-swizzle bit layout forSW_*_S), including the bytes no element maps to. Breaking the word store (^ 1u) fails it at the first byte.This is a throughput fix for those paths: no title I have is bound by 1- or 2-byte detiles yet, so it makes no frame-rate claim (title runs under Tested).
Tested
Windows 10 19045, MinGW GCC, Release.
agc_detile_small_elements,agc_driver_graphics,agc_unit_shadow,agc_shadowed_key_storesand theagc_storage_*tests pass.--detile-benchharness timing 20 dispatches per path with timestamp queries, 1920x1080, median;main4524944 vs this PR in the same build tree, two runs each, mean of the two):mainSW_64KB_SSW_64KB_R_XSW_64KB_SSW_64KB_SSW_64KB_R_XSW_64KB_SSW_64KB_R_XAPS5_PROFILE_GPU=1: no change, because their per-frame write-backs are not 1- or 2-byte standard-layout retiles. Minecraft Dungeons II (PPSA16064): 6.60 / 6.71 / 6.60 FPS onmain, 6.68 / 6.58 / 6.67 with this PR,storage-writeback17.3-17.6 ms per present on both. Sword of the Sea (PPSA25344): 9.62 / 9.60 FPS onmain, 9.60 / 9.60 with this PR,storage-writeback15.3-15.5 ms andstorage-upload5.0-5.1 ms per present on both. The paths this speeds up are the buffer detile of 8- and 16-bit textures (sampled texture uploads) and the standard-layout storage write-backs of such images; I have no title scene yet that is bound by them, so this is offered as a throughput fix with the benchmark above, not a frame-rate claim.tools/check_conventions.py --base upstream/mainpasses.Performance capture (as the Performance report form describes): Minecraft Dungeons II (PPSA16064) title screen, RTX 3060 12 GB (i5-12400F), NVIDIA 617.42, Windows 10.0.19045, 3840x2160 output. Only
APS5_TRACE_SECONDS=120:12set, caches warm (the title ran once on the build and was restarted), stderr not redirected.main48d61f5 against this PR's head 283b057:Frame and GPU times (ms):
Top phases (ms per frame):
{"commit":"48d61f53b46f","dirty":false,"env":{"APS5_TRACE_SECONDS":"120:12"},"gpu":"NVIDIA GeForce RTX 3060","driver":"NVIDIA 617.42","os":"Windows 10.0.19045","resolution":"3840x2160","window":{"spec":"seconds 120:12","first_present":2158,"ms":12053.995},"presents":99,"frame_ms":{"p50":117.162,"p90":145.047,"p99":158.422,"max":158.422},"gpu_busy_ms_per_present":100.711,"worker_busy_pct":{"queue worker 0x0":99.421,"queue worker 0x20":99.581},"top_phases_ms_per_frame":{"Driver.Packet":242.204,"Driver.Packet.dispatch":86.382,"Driver.Dispatch":85.879,"Driver.Packet.memory_wait":68.067,"Vulkan.Dispatch":40.265,"Vulkan.Dispatch.shader_resources":38.025,"Driver.Packet.device_lock":34.172,"Driver.Packet.prepare_memory":30.126},"trace_dropped":false,"trace":"aps5-trace-48d61f53b46f-9720.json.gz"} {"commit":"283b057afe16","dirty":false,"env":{"APS5_TRACE_SECONDS":"120:12"},"gpu":"NVIDIA GeForce RTX 3060","driver":"NVIDIA 617.42","os":"Windows 10.0.19045","resolution":"3840x2160","window":{"spec":"seconds 120:12","first_present":2159,"ms":12090.542},"presents":100,"frame_ms":{"p50":116.903,"p90":144.762,"p99":163.666,"max":163.666},"gpu_busy_ms_per_present":100.576,"worker_busy_pct":{"queue worker 0x0":99.411,"queue worker 0x20":99.593},"top_phases_ms_per_frame":{"Driver.Packet":240.507,"Driver.Packet.dispatch":84.174,"Driver.Dispatch":83.654,"Driver.Packet.memory_wait":67.603,"Vulkan.Dispatch":39.326,"Vulkan.Dispatch.shader_resources":37.045,"Driver.Packet.device_lock":36.328,"Driver.Packet.prepare_memory":28.913},"trace_dropped":false,"trace":"aps5-trace-283b057afe16-15136.json.gz"}Traces:
aps5-trace-48d61f53b46f-9720.json.gz(main) andaps5-trace-283b057afe16-15136.json.gz(this PR), attached below.trace_digest.py, main
seconds 120:12 from present 2158: 99 presents, frame p50 117.2 / p90 145.0 / p99 158.4 / max 158.4 ms, GPU busy 100.7 ms per present
NVIDIA GeForce RTX 3060, NVIDIA 617.42, Windows 10.0.19045, 3840x2160, commit 48d61f5, dropped: no
{"commit":"48d61f53b46f","dirty":false,"env":{"APS5_TRACE_SECONDS":"120:12"},"gpu":"NVIDIA GeForce RTX 3060","driver":"NVIDIA 617.42","os":"Windows 10.0.19045","resolution":"3840x2160","window":{"spec":"seconds 120:12","first_present":2158,"ms":12053.995},"presents":99,"frame_ms":{"p50":117.162,"p90":145.047,"p99":158.422,"max":158.422},"gpu_busy_ms_per_present":100.711,"worker_busy_pct":{"queue worker 0x0":99.421,"queue worker 0x20":99.581},"top_phases_ms_per_frame":{"Driver.Packet":242.204,"Driver.Packet.dispatch":86.382,"Driver.Dispatch":85.879,"Driver.Packet.memory_wait":68.067,"Vulkan.Dispatch":40.265,"Vulkan.Dispatch.shader_resources":38.025,"Driver.Packet.device_lock":34.172,"Driver.Packet.prepare_memory":30.126},"trace_dropped":false,"trace":"aps5-trace-48d61f53b46f-9720.json.gz"}Slowest frames (largest slices inside each, inclusive)
Threads (ms per frame)
Largest slices (ms per frame, inclusive, all threads): Driver.Packet 235.3, batch 100.7, Driver.Packet.dispatch 86.1, Driver.Dispatch 85.2, Driver.Packet.memory_wait 68.0, Vulkan.Dispatch 39.4, Vulkan.Dispatch.shader_resources 37.1, Driver.Packet.device_lock 32.2, Driver.Packet.prepare_memory 27.8, Recorder.Sync 20.7
Longest waits
Largest GPU batches (GPU ranges inside, and the CPU slices that recorded each, inclusive)
trace_digest.py, this PR
seconds 120:12 from present 2159: 100 presents, frame p50 116.9 / p90 144.8 / p99 163.7 / max 163.7 ms, GPU busy 100.6 ms per present
NVIDIA GeForce RTX 3060, NVIDIA 617.42, Windows 10.0.19045, 3840x2160, commit 283b057, dropped: no
{"commit":"283b057afe16","dirty":false,"env":{"APS5_TRACE_SECONDS":"120:12"},"gpu":"NVIDIA GeForce RTX 3060","driver":"NVIDIA 617.42","os":"Windows 10.0.19045","resolution":"3840x2160","window":{"spec":"seconds 120:12","first_present":2159,"ms":12090.542},"presents":100,"frame_ms":{"p50":116.903,"p90":144.762,"p99":163.666,"max":163.666},"gpu_busy_ms_per_present":100.576,"worker_busy_pct":{"queue worker 0x0":99.411,"queue worker 0x20":99.593},"top_phases_ms_per_frame":{"Driver.Packet":240.507,"Driver.Packet.dispatch":84.174,"Driver.Dispatch":83.654,"Driver.Packet.memory_wait":67.603,"Vulkan.Dispatch":39.326,"Vulkan.Dispatch.shader_resources":37.045,"Driver.Packet.device_lock":36.328,"Driver.Packet.prepare_memory":28.913},"trace_dropped":false,"trace":"aps5-trace-283b057afe16-15136.json.gz"}Slowest frames (largest slices inside each, inclusive)
Threads (ms per frame)
Largest slices (ms per frame, inclusive, all threads): Driver.Packet 233.6, batch 100.5, Driver.Packet.dispatch 83.9, Driver.Dispatch 83.0, Driver.Packet.memory_wait 67.5, Vulkan.Dispatch 38.4, Vulkan.Dispatch.shader_resources 36.1, Driver.Packet.device_lock 34.3, Driver.Packet.prepare_memory 26.5, Recorder.WaitSerial 20.3
Longest waits
Largest GPU batches (GPU ranges inside, and the CPU slices that recorded each, inclusive)
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)