Repository navigation
perf(agc): draw an indirect rect list from its records on the GPU - #5979
Conversation
CnRJay
left a comment
CnRJay
left a comment
There was a problem hiding this comment.
Removing a CPU read that waits 3–15 ms on the producing batch once per frame is worthwhile, and the classifier change is small.
On the evidence: the two pairs disagree. Run 1 is slower on the GPU path (p50 60.5 → 69.9 ms) and run 2 is faster (65.5 → 54.6 ms). They come from a local stack, not main, and as the description says, that is within the scene's variance. The traces were offered on request. Attaching the four aps5-summary.jsonl lines and traces would let a maintainer check whether the indirect-args time moved somewhere else on queue 0 (for example into memory_wait or flush_between_packets), since the worker stays at about 97 % either way.
On coverage: the new test draws one rect from a {3, 1, 0, 0} record. Two cases would pin the GPU path where rect lists differ from triangle lists:
- a record with 6 vertices and 2 instances, so that several patches per instance and
firstInstancego through the generated tessellation stages; and - an indexed indirect rect list (
DRAW_INDEX_INDIRECT), if the classifier lets it through. If it doesn't, a line in the description saying so would do.
ClassifyIndirectDraw sent every indirect rect-list draw to the CPU path (non-vertex path), which reads the records on the CPU. When the GPU writes those records earlier in the frame, the read waits for every batch up to the producer: in Kena (PPSA01802) each such DRAW_INDIRECT waited about 15 ms, about once per frame. A rect list on the vertex path is drawn by its generated tessellation stages from three-vertex patches, with no draw-dependent constants, so the records can be consumed on the GPU like any vertex-path draw. The classifier now keeps a rect list without mesh or tessellation stages on the GPU path, and Graphics::Draw accepts it. APS5_NO_RECT_INDIRECT=1 restores the CPU path. agc_driver_rect_list_indirect submits a direct rect-list draw and the same rectangle from a DRAW_INDIRECT record and requires the same bytes, and a record without vertices to draw nothing; it passes on either path. agc_driver_draw_offset_indices requires an auto indirect rect-list record whose packet names no offset registers to be classified to the GPU. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
e53bb43 to
17ca453
Compare
|
@CnRJay thanks. Both points are addressed; rebased onto Coverage.
With Evidence. The four summary lines are below, and the traces are attached here. Per-present time on queue worker 0x0, from the traces:
So run 1's slower GPU numbers look like a cold pipeline cache rather than the wait moving elsewhere. Two pairs are still too few to claim a frame-time gain, and the description still says only that the wait goes away. aps5-summary.jsonl lines (CPU 1, GPU 1, CPU 2, GPU 2)AI-assisted: yes (Claude Code) |
|
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.
|
adriadam10
left a comment
adriadam10
left a comment
There was a problem hiding this comment.
Checked build + ctest on the PR merged onto current main: agc_driver_draw_offset_indices and agc_driver_rect_list_indirect pass. One thing to fix before merging:
core/libs/prx/libSceAgcDriver/tests/execution/RectListIndirect.cpp: the test passes with the non-test changes reverted (IndirectDraw.cpp,Draw.cpp), because the CPU path writes the same bytes as the direct draw. It checks the output is equal, not that the indirect draw took the GPU path, so the 6-vertex/2-instance, indexed and empty-record cases would pass onmaintoo. Only the single non-offset case inDrawOffsetIndices.cppfails without the change. Please pin the route for each of those draws (classify withClassifyIndirectDrawasDrawOffsetIndices.cppdoes and require the GPU, or assert on the indirect-draw trace/counter), and check that the test fails withAPS5_NO_RECT_INDIRECT=1.
Not blocking, for the record: the frame-time numbers are within variance (run 1 slower, run 2 faster), so the claim here is the removed indirect-args wait only.
agc_driver_rect_list_indirect compared the indirect draws' output with the direct draws only, so it also passed when the records were read on the CPU. Its records lived in a static array that is not guest memory, so every indirect draw fell back to the CPU path as not imported. The records now sit in a registered 64 KiB guest allocation, the test turns on APS5_PROFILE_DRAW and reads the indirect-draw counters through the new Graphics::IndirectDrawCounts, and it requires all four indirect draws on the GPU path. It fails with APS5_NO_RECT_INDIRECT=1 and with the IndirectDraw.cpp and Draw.cpp changes reverted (4 non-vertex path). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@adriadam10 thanks, you were right, and it was worse than the test missing a check: the end-to-end draws never took the GPU path at all. The records were in a static array that isn't guest memory, so all four indirect draws got past Fixed in
Linux (Arch), RX 9070 XT (RADV, Mesa 26.2.4), Release:
AI-assisted: yes (Claude Code) |
adriadam10
left a comment
adriadam10
left a comment
There was a problem hiding this comment.
Re-checked ac64ea452: the records now live in a registered guest allocation and the test reads the indirect-draw counters, requiring 4 GPU draws and 0 CPU-path draws. agc_driver_rect_list_indirect, agc_driver_draw_offset_indices and agc_driver_graphics pass on RADV and on CI's lavapipe; with APS5_NO_RECT_INDIRECT=1 both new tests fail ("read their records on the CPU: 4 non-vertex path" / "expected the GPU"). Merging once the full Build is green.
|
Since this PR, PPSA15304 renders bright red, blue and magenta blotches between the tree canopies at the start position of the first area, and red light patches on the tree trunks.
Ubuntu 24.04, kernel 7.0.0, NVIDIA GeForce RTX 5080, driver 595.99.02, Release build. I have not found which part of those draws differs yet (records, the rewritten constant, or the generated rect-list stages with indirect counts). I can trace one of the draws if you tell me which values you want; the frame dumps of both runs are available. AI-assisted: yes |
|
Thanks for the bisect, the frame dumps and the counters. On the GPU path these draws take the same record rules as indirect triangle lists (
A few draws with the same AI-assisted: yes (Claude Code) |
|
@alfalfa-star The records match. Same save, same camera position, plain
The rect-list draws are
Example, same offset:
So the GPU draw reads the same kind of records with the same constants; about 60% of them have zero instances, the rest up to about 300. Ubuntu 24.04, kernel 7.0.0, NVIDIA GeForce RTX 5080, driver 595.99.02, Release build. AI-assisted: yes |
|
@ee6aba54 Thanks, that settles the records: same count, first index, offsets and instance distribution, and no value in a Constant dimension. That leaves the draw itself. With
One run separates them: the GPU route (no AI-assisted: yes (Claude Code) |
|
@alfalfa-star With Same save and camera position, GPU route (no
For comparison, the earlier runs on Ubuntu 24.04, kernel 7.0.0, NVIDIA GeForce RTX 5080, driver 595.99.02, Release build. AI-assisted: yes |
|
@ee6aba54 Thanks, that settles it: the draw call is fine, and the GPU route reads something before it is final. I need to correct my last comment, though. Extending
Could you do two more runs on the GPU route (no
If both still show blotches, it is a shader buffer read, and I'll send a build that logs which one overlaps work still in flight. AI-assisted: yes (Claude Code) |
|
@alfalfa-star On current Same save and camera position, GPU route (no
The last build with blotches was Ubuntu 24.04, kernel 7.0.0, NVIDIA GeForce RTX 5080, driver 595.99.02, Release build. AI-assisted: yes |
|
@ee6aba54 Thanks for checking. If plain AI-assisted: yes (Claude Code) |
What
ClassifyIndirectDrawsends every indirect rect-list draw to the CPU path (non-vertex path), which reads the records on the CPU. When the GPU writes those records earlier in the frame, the read waits for every batch up to the producer. In Kena: Bridge of Spirits (PPSA01802) that is oneDRAW_INDIRECTrect list about once per frame, each waiting 3 to 15 ms for the producing batch ([syncwait] ... indirect-args 127/995msin one 10 s window, 116/1744ms in another).A rect list on the vertex path is drawn by its generated tessellation stages from three-vertex patches. Those stages use no draw-dependent constants, so the records can be consumed on the GPU like any vertex-path draw, including the existing argument rewrite for constant offsets.
Graphics::Drawno longer requires the driver to resolve indirect rect lists.APS5_NO_RECT_INDIRECT=1restores the CPU path.Tested
Arch Linux (kernel 7.2.9), AMD Radeon RX 9070 XT, RADV Mesa 26.2.4, Release build of this branch on
maind92a8da.agc_driver_rect_list_indirect. It requires every indirect draw on the GPU path (read from the indirect-draw counters, the records in imported guest memory), so it fails withAPS5_NO_RECT_INDIRECT=1or without the driver change. Each indirect draw must also write the same bytes as its direct counterpart:DRAW_INDIRECTrecord{3, 1, 0, 0}, and a record without vertices that must draw nothing;{6, 2, 0, 0}againstNUM_INSTANCES 2+DRAW_INDEX_AUTO 6). The vertex shader puts patchvertex id / 3 + 2 × instance idin its own slot, and all four slots must be filled;DRAW_INDEX_INDIRECT{6, 1, 0, 0, 0}againstDRAW_INDEX_2, 16-bit indices{6, 7, 8, 3, 4, 5}). Only slots 1 and 2 may be filled. The classifier lets indexed indirect rect lists through as well.agc_driver_draw_offset_indices: an auto indirect rect-list record whose packet names no offset registers must be classified to the GPU. It fails withAPS5_NO_RECT_INDIRECT=1.ctest -R "agc_|shader|recompiler": 575 of 576 pass (3 skipped).agc_driver_tessellationsegfaults on this machine without this change as well.Kena on plain
maindoes not reach the title scene (it needs unmerged GPU PRs and stand-ins), so this is not a main-vs-PR capture. Same change on my local Kena stack, same save,APS5_TRACE_SECONDS=50:10, switch off (CPU path) and on, alternating:DRAW_INDIRECTtime / 10 sindirect-argswaits / 10 sThe wait goes away. The frame-time effect is within this scene's run-to-run variance in these four runs; queue worker 0 is about 97 % busy on both paths. The four summary lines, the traces and a per-phase breakdown are in this comment. Run 1 on the GPU path was its first run and compiled the new pipeline variants cold (
shader_compile_and_link258 ms against 16 to 19 ms in the other runs).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