Repository navigation
perf(agc): skip the staging, write-back and label-run leading barriers a covering barrier already orders - #7489
Conversation
…g barrier already covers
|
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.
|
|
A data point from Linux. This is PPSA31162 on an NVIDIA RTX 4070 Ti (driver 595.99.02), Ubuntu 24.04. I merged this PR at 0d704d3 onto
Recorded plus merged stays at about 99 per present for both staging classes. So in this title the PR leaves out about half of the copy-back barriers and about a third of the copy-in barriers. The write-back gains about 25 to 30 merged barriers per present. The label run's barrier is left out about once per present, out of roughly 750, so that condition rarely holds in this title. I can't see a GPU time change. The runs reached different parts of the game (336 to 598 presents over the same stretch), and per-present GPU time follows that much more than it follows the switch. All four runs ran to the 180 s timeout. AI-assisted: yes (Claude Code) |
|
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 |
|
@milescoviello thanks for the second title. The counts match the mechanism. Recorded plus merged stays at about 99 per present in both staging classes, so the PR leaves out about half of the copy-back barriers and a third of the copy-in barriers. The write-back gains 25 to 30 merged barriers per present. The label-run case almost never holds in PPSA31162 (1 in ~750), so in that title the gain is the staging and write-back sites. I agree there is no GPU-time reading in rows that cover 336 to 598 presents of different stretches. The lab machine will run an A/B of this PR against AI-assisted: yes (Claude Code) |
github-actions
Bot
added
perf
# Conflicts: # core/libs/prx/libSceAgcDriver/CMakeLists.txt
The environment-reference check now requires a row for every APS5_* name a pull request reads; main added that check after this branch was written.
|
This pull request is labeled |
|
Intel and AMD Windows data: Windows 11 Pro 26300, WinLibs GCC 15.2.0 r7, Release, every execution test built,
This is a correctness check only; I measured no speed. I also ran full CTest with this PR and the 13 other open AGC and shader PRs I tested in the same batch merged together (#7668, #7659, #7504, #7432, #7426, #7413, #7480, #7465, #7466, #7464, #7449, #7673, #7560). Neither GPU showed a failure beyond AI-assisted: yes (Claude Code) |
…var#7489, linear-time IR uses) AI-assisted: yes
What
Several commands start with a barrier that waits for every earlier write: the GPU buffer copy-in (
recordGpuCopies), the copy-back (RecordCopyBacks), the storage write-back's import-ready barrier (StorageTexture::writeBackWindows) and the label store run (closeStoreRun). When the command before them has just recorded a barrier of its own that already covers theirs, they add a second barrier back to back with the first. A dispatch, a fill, a copy and the write-back's seed barrier already skip theirs in that case (Recorder::Commands(&covered),MarkCovered, themergedcount on the[barriers]line). This extends the same rule to the four sites above.BarrierMaskand two pure functions (Graphics/include/BarrierBatch.hpp,Graphics/src/BarrierBatch.cpp):Covers(outer, inner)(stages, with ALL_COMMANDS holding every stage but the host; access masks as subsets) andOrderedByTrailing(covered, wanted)(the trailing barrier's destination accesses hold the shader and transfer accesses and the ones the left-out barrier asks for; the left-out barrier has no host destination, no layout change and no queue family transfer).Recorder::ElideCovered(class, wanted, covered):OrderedByTrailingplus the switch, counted asmergedfor the class.ElideCoveredwith the maskCommands(&covered)returned. The write-back asks only when it recorded no seed or padding copy before the barrier, since those are commands recorded afterCommands.closeStoreRunitself (its pass end barrier is the last command, and the pass has no after-pass callback) andCovers(PassEndBarrier, LabelLeadingBarrier). Both masks are now constants;endOpenRenderPassrecordsPassEndBarrier, so the check and the barrier can't drift apart. The barrier at Submit (host destination) is recorded as before.On by default.
APS5_NO_BARRIER_BATCH=1records them again (set at all means off, like the otherAPS5_NO_*switches);APS5_NO_BARRIER_ELISION=1andAPS5_FULL_BARRIERS=1turn it off as well, since they turn off every leading barrier merge. The left-out barriers add to themergedcounts per class (staging-in,staging-out,storage-writeback,label-run) on the[barriers]line underAPS5_PROFILE_DRAWorAPS5_PROFILE_GPU.Why the result is the same: a barrier is left out only when its work is already ordered by the one before it.
Commands(&covered)returns a non-zero mask only when the last command recorded into the batch is a barrier with ALL_COMMANDS as its destination stage and nothing was recorded since; the nextCommandscall clears it. That is the rule the dispatch, fill and copy sites already rely on. Every write recorded before that barrier is available and visible to the accesses of the mask at any later stage.OrderedByTrailing). A trailing barrier that marks less (the copy-in's own, which marks shader and uniform reads and shader writes) leaves the next barrier recorded.OrderedByTrailingrefuses any barrier that changes a layout or transfers a queue family, which also keeps the non-direct write-back's barrier (TRANSFER_SRC_OPTIMAL) as it is.Limits: the ordering is argued from the Vulkan rules above; the validation layers were not run on this PR's head, and nothing was run on console hardware. A GPU or driver that needs the second barrier to flush a cache the first leaves alone would break the argument, and
APS5_NO_BARRIER_BATCH=1is the way back.Tested
agc_*and recorder set of the lab test filter): 709 of 709 passed, among themagc_barrier_batch(the new test). Not run on Windows.agc_barrier_batch_tests(tests/BarrierBatch.cpp, registered in the CMake test list):Coverson the pass end against the label run's leading barrier (covered), against the copy-in's (not: ALL_COMMANDS holds no host stage), against narrower and wider stages and accesses (a compute-only barrier, a source without the colour write, a destination without the transfer write, the reverse direction), andOrderedByTrailingon the masks the trailing barriers of a dispatch, a fill, a pass end and the copy-in leave (left out for the copy-in, the copy-out and the import-ready barrier; kept for an empty mask after an intervening command, a shader-only mask, a mask without the transfer read, the copy-in's own mask, a destination access the mask lacks, a host destination, an image barrier that changes the layout, a queue family transfer). Memory barriers carry no buffer or image range, so there is no range case: a global barrier covers every range, which is what the argument uses. The recorder paths are not unit tested; they run in the existingagc_*execution tests (the draw, dispatch, staging and write-back tests of the filter) with the switch on, and nothing in them failed.lab/barrier-batch, on an older base: job 9b, a = the lab stack lead-best11 +REFRESH_LEVER=2, b = the same +APS5_LAB_BARRIER_BATCH=1), not on this PR's head. The lab version differs from the PR: it also planned the order of the label store run (overlapping stores only get a barrier between waves), merged the write-back's image barrier and seed barrier into one call, and had a verify ledger and its own counters; none of that is in this PR, so its label-run and write-back figures are not this PR's. All rows, as measured:The FPS rows are memory-noisy (the app held 15.8 GiB private and the commit reached 64 GiB; the gameplay spread inside arm a alone is 4.1 to 6.9 FPS, and the one verify row crashed once with 1.3 GiB free and ran on the retry), so I claim nothing for FPS or GPU busy from them.
GPU ms per present by class, mean over the
[gputime]windows after the first three (ranges per present in brackets),APS5_PROFILE_GPU=1on all rows:In gameplay the four classes together went from 12.76 to 12.08 ms per present (about -0.7 ms, of which 0.45 ms in storage-writeback and 0.13 in label-run); at the title they did not change. Barriers recorded per 10 s, last
[barrierbatch]line of each b row (before/after in the lab version): gameplay 57,143 to 44,256, 65,249 to 50,270, 112,726 to 85,766 (-22.6 %, -23.0 %, -23.9 %), title 108,363 to 82,306 and 49,993 to 39,322 (-24.0 %, -21.3 %); the verify row's windows 37,607 to 29,382 and 34,254 to 26,755 (-22 %). Most of it is the copy-in and copy-back leading barriers (gameplay, last b row: staging-in 16,086 to 10,121 and staging-out 16,086 to 8,043) and the write-back (40,158 to 28,363); the label run went from 40,396 to 39,239, of which the lab version's store waves are part, so this PR's share there is smaller. In the verify row, a ledger in the lab version checked every elided or merged barrier against the barrier left in its place: 156 and 100 exact covers, 7,900 and 7,245 covers through the trailing barrier's mask, 0 violations in both verify windows (the ledger ran in verify mode only). That ledger is a model of the rule, not a GPU result. 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 e5c2b3c.tools/pr_overlap.py --branch pr/barrier-batchlists 76 open pull requests that touch a file this one changes; none implements the same functions (I read the diffs of those that mention barriers). Textual overlaps in the lines I change: fix(agc): copy only the dwords a shader changed back from a GPU-copied buffer #6185 and fix(agc): keep CPU stores made after a deferred DCC key store was recorded #6186 (the copy-back's leading barrier line inRecordCopyBacks, whose destination stage they widen, andrecordGpuCopies, where they add a shadow copy and a barrier after the leading barrier), feat(agc): back extra guest mip levels with storage images #7085 (the import-ready barrier line inwriteBackWindows, which it turns intoimageBarrier(...)) and perf(agc): retile storage write-back windows straight into their unit-shadow slabs #6490 (the barrier part ofwriteBackWindows: the import-ready barrier and a new scratch-done barrier; its hole copies are recorded before the import-ready barrier, so the condition there that no seed or padding copy was recorded would have to name them too). fix(agc): defer unrouted vertex barriers #6150, fix(agc): include the host stage in the CMASK clear's host-access barriers #6781 and fix(shader): emit nothing for a vertex s_barrier that precedes every memory access #6613 touch no barrier of these functions (shader registry deferral, the CMASK clear dispatch, a recompiler barrier). Whichever merges second needs a rebase; each site's barrier is oneBarrierMaskconstant, so a changed destination stage is one edit.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)