Repository navigation
perf(agc): read DCC keys behind a pattern fill kernel's fast clear without a wait - #5497
Conversation
|
Two things before this can go in:
|
e345185 to
e095ee4
Compare
|
Rebased onto
The PPSA01317 before/after from the same scene is still to come. On stock AI-assisted: yes |
|
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.
|
|
Still open from 10-13: the PPSA01317 summary lines of |
…thout a wait A 64-thread compute kernel (23 dwords) stores pattern[i % 4], the pattern in s4..s7, to dword i of a 4-byte-stride buffer for every i below min(launched threads, s8, num_records), with the group id in s9. PPSA01317 fast-clears its render-target DCC keys with it every frame. As a plain dispatch its writes had no known value, so every key read of those targets waited for the GPU under the GPU mutex. The dispatch still runs as before. When it stores one repeated key code, its keys are noted in the key-store memo right after the dispatch is recorded, as a buffer fill of keys already is; the memo answers only while the dispatch's own write note is the newest over the range, so any later writer still makes the read wait. The key codes come from KeysFillMemo, as for the memory-value fill kernel. AI-assisted: yes
e095ee4 to
448ccee
Compare
|
Rebased onto On the measurement: I can't give you The only numbers I have are from our local tree, which isn't Do you want to leave this waiting until then? AI-assisted: yes |
|
@gjonn thanks for the rebase, and for being straight about the numbers. Yes, let's leave this waiting. On main, PPSA01317 still stops before the title screen, so it never reaches the race where this fill kernel runs every frame, and a main-vs-PR comparison from the same scene isn't possible yet. We're parking this PR until main can get there (the blockers in #4697, #7137, #7207, #5958 and #7175). Once main reaches the race, please post the summary lines of main and of this change from the same scene, in alternating blocks with the traces, as you described, and we'll pick it back up from there. A rebase at that point is fine; no need to keep it current in the meantime. |
github-actions
Bot
added
perf
|
Independent fixture check on literal main
Three fresh processes per GPU with the normal implicit host layers plus explicit Khronos validation: RTX5080 2 pass /1 fail, AMD Radeon(TM) Graphics 2 pass /1 fail. Repeating three fresh processes per GPU with Reproduce serially with Invoke it in three separate processes per device. RTX5080 |
|
Hi @gjonn, |
|
Native Linux reproduction of the retired-before-observation case in Dean's request, on pinned main The unmodified registered test passed 10/10 serial lavapipe runs. Adding only a 100 ms delay at entry to The batch was already retired; there was no pending write note left for the loop to find. This is a controlled scheduling reproduction, not a measured natural CI failure rate. I tested the resubmission option below. When no new note is observed for 10 ms, it first drains the previously submitted work, then resubmits the same case's idempotent fill. Draining before resubmission prevents queued duplicates from leaking into the next case. The existing 10-second deadline, key classification, pending/waited expectations and stored-dword assertions remain; timeout messages now identify the case.
Ubuntu 24.04, kernel 7.0.0-38, Ryzen 9950X, GCC 13.3, Release; NVIDIA 595.99.02, Mesa 25.2.8. ICD restricted separately and actual devices verified; required Vulkan, shader cache and pipeline journal disabled. No Khronos validation layer is installed. Windows/macOS, loaded validation and parallel-CI failure rates are not verified. No production files are changed. Raw logs are retained. Test-only candidate against that main pindiff --git a/core/libs/prx/libSceAgcDriver/tests/execution/SgprPatternFillKeys.cpp b/core/libs/prx/libSceAgcDriver/tests/execution/SgprPatternFillKeys.cpp
index e35849eb7..1d495d061 100644
--- a/core/libs/prx/libSceAgcDriver/tests/execution/SgprPatternFillKeys.cpp
+++ b/core/libs/prx/libSceAgcDriver/tests/execution/SgprPatternFillKeys.cpp
@@ -95,7 +95,12 @@ void MatchTests() {
Require(!MatchSgprPatternFill(packet, Code, data, noGroup), "a dispatch without the group id matched");
}
+std::vector<std::uint32_t> submittedData;
+std::uint32_t submittedGroups = 0;
+
void Submit(const std::vector<std::uint32_t>& data, std::uint32_t groups) {
+ submittedData = data;
+ submittedGroups = groups;
std::vector<std::uint32_t> words;
const auto registers = [&](std::uint32_t first, std::span<const std::uint32_t> values) {
words.push_back(0xc0007600u | (static_cast<std::uint32_t>(values.size()) << 16u));
@@ -122,7 +127,10 @@ struct KeyRead {
std::uint64_t lastNote = 0;
-KeyRead ReadAfterWrite(std::uint64_t surfaceBytes = SurfaceBytes) {
+KeyRead ReadAfterWrite(const std::string& what, std::uint64_t surfaceBytes = SurfaceBytes) {
+ const auto data = submittedData;
+ const auto groups = submittedGroups;
+ auto retryAt = std::chrono::steady_clock::now() + std::chrono::milliseconds(10);
const auto deadline = std::chrono::steady_clock::now() + std::chrono::seconds(10);
for (;;) {
{
@@ -134,7 +142,13 @@ KeyRead ReadAfterWrite(std::uint64_t surfaceBytes = SurfaceBytes) {
return {keys, !recorder->PendingWriteOverlaps(KeysAddress(), KeyBytes)};
}
}
- Require(std::chrono::steady_clock::now() < deadline, "the submitted dispatch never became a pending write over the keys");
+ const auto now = std::chrono::steady_clock::now();
+ Require(now < deadline, what + ": the submitted dispatch never became a pending write over the keys");
+ if (now >= retryAt) {
+ AgcDriverWaitIdle_nid_postfix();
+ Submit(data, groups);
+ retryAt = std::chrono::steady_clock::now() + std::chrono::milliseconds(10);
+ }
std::this_thread::yield();
}
}
@@ -160,37 +174,37 @@ void ExecuteTests() {
for (const auto& [key, keys] : codes) {
Settle();
Submit(UserData(Dwords, Dwords, Uniform(key)), Dwords / 64);
- const auto read = ReadAfterWrite();
+ const auto read = ReadAfterWrite("uniform key " + std::to_string(key));
Require(read.keys == keys && !read.waited, Describe("a pending key code fill by the kernel", read));
RequireStored(Dwords, Uniform(key), 0, "the kernel's key code fill");
}
Submit(UserData(Dwords, Dwords, Uniform(0x5a)), Dwords / 64);
- const auto other = ReadAfterWrite();
+ const auto other = ReadAfterWrite("non-key byte fill");
Require(other.keys == DccKeys::Mixed && other.waited, Describe("a pending fill of a byte that is not a key code", other));
RequireStored(Dwords, Uniform(0x5a), 0, "the kernel's fill of another byte");
const Pattern several{0x40404040u, 0x40404040u, 0x40404040u, 0x80808080u};
Submit(UserData(Dwords, Dwords, several), Dwords / 64);
- const auto mixed = ReadAfterWrite();
+ const auto mixed = ReadAfterWrite("mixed key fill");
Require(mixed.keys == DccKeys::Mixed && mixed.waited, Describe("a pending fill of several key codes", mixed));
RequireStored(Dwords, several, 0, "the kernel's fill of several key codes");
Submit(UserData(Dwords, Dwords, Uniform(0x40)), Dwords / 64);
- const auto first = ReadAfterWrite();
+ const auto first = ReadAfterWrite("0001 fill before the later writer");
Require(first.keys == DccKeys::Clear0001 && !first.waited, Describe("a pending 0001 fill by the kernel", first));
Submit(UserData(Dwords, Dwords / 2, Uniform(0x5a)), Dwords / 64);
- const auto later = ReadAfterWrite();
+ const auto later = ReadAfterWrite("later half-range writer");
Require(later.keys == DccKeys::Mixed && later.waited, Describe("a later writer over half of a pending 0001 fill", later));
Settle();
for (std::uint32_t i = 0; i < Dwords; ++i) Require(Keys[i] == (i < Dwords / 2 ? 0x5a5a5a5au : 0x40404040u), "the later writer over the 0001 fill: dword " + std::to_string(i) + " holds " + std::to_string(Keys[i]));
Submit(UserData(Dwords, Dwords / 2, Uniform(0xc0)), Dwords / 64);
- const auto half = ReadAfterWrite();
+ const auto half = ReadAfterWrite("half-range 1111 fill");
Require(half.keys == DccKeys::Mixed, Describe("a pending 1111 fill of half the keys over 0001 keys", half));
Settle();
Submit(UserData(Dwords, Dwords / 2, Uniform(0xc0)), Dwords / 64);
- const auto inside = ReadAfterWrite(SurfaceBytes / 2);
+ const auto inside = ReadAfterWrite("inside the half-range 1111 fill", SurfaceBytes / 2);
Require(inside.keys == DccKeys::Clear1111, Describe("the half of the keys a pending 1111 fill covers", inside));
Settle();
for (std::uint32_t i = 0; i < Dwords; ++i) Require(Keys[i] == (i < Dwords / 2 ? 0xc0c0c0c0u : 0x40404040u), "the 1111 fill of half the keys: dword " + std::to_string(i) + " holds " + std::to_string(Keys[i])); |
4 participants
What
PPSA01317 fast-clears its render-target DCC keys every frame with a 64-thread compute kernel (23 code words) that stores
pattern[i % 4], the pattern in the user SGPRs s4..s7, to dwordiof the stride-4 V# in s0..s3 for everyi < min(groups * 64, s8, num_records), with the group id in s9:On current
mainit runs as a plain dispatch, so its store is a pending write of unknown value. The next key read of those targets (currentDccKeys/WaitForKeyWritersinGraphics/src/DccMetadata.cpp) findsPendingWriteOverlapstrue andPendingStoreKeysempty, and callsRecorder::SyncThrough, waiting for the GPU while holding the GPU mutex (#3505).This PR leaves the dispatch as it is.
MatchSgprPatternFillrecognises the kernel (exact code, 9 user SGPRs with the group id enabled, 64x1x1 threads, one group row, no partial or indirect dispatch, a 32-bit format, stride 4, not swizzled, no add-tid, index-bounded OOB mode). When the pattern is one repeated DCC key code (KeysFillMemofrom #4603, taken as for its memory-value fill kernel),Driver::dispatchcallsGraphics::NoteKeysFillOnGpuunder the GPU mutex right after the dispatch is recorded. It does this only if the dispatch's own write note is the newest note and covers exactly the filled range.PendingStoreKeysanswers from that note only while it is still the newest over the range, so any later writer makes the read wait as before. The kernel needs no 16-byte multiple or alignment and is not taken as a buffer fill.Related:
s_buffer_load.This PR covers the 23-word kernel that takes its pattern from user SGPRs.
Tested
Windows 11 Pro, MinGW GCC, Release, NVIDIA GeForce RTX 5090 Laptop GPU (driver 616.92).
New test
agc_driver_sgpr_pattern_fill_keys(tests/execution/SgprPatternFillKeys.cpp):Submit:The test fails on
main(a pending key code fill by the kernel: the keys read 0000 and the read waited for the GPU) and passes with this change, 15 of 15 repeated runs.These also pass:
dword_pattern_fill,zero_dword_fill,pattern_buffer_filldcc_keys_first_write,fill_cover_keyscmask_fast_clear,depth_fast_clear,depth_fast_clear_submitdepth_storage_dispatch,gated_dispatch_capture,oversized_dispatchstore_over_untracked,store_idle_results,compute_user_data_calls,resource_cache_storage_clearRunning
agc_driver_recorder_testswithout arguments fails with(9) a scan under a pending write was kepton unmodifiedmainas well.tools/check_conventions.pypasses.PPSA01317: the same note in a local tree raised race heavy-phase FPS from 1.395 to 1.833 (+31%, 3 of 3 A/B blocks) and halved the GPU waits made under the GPU mutex. The image was unchanged. That measurement was taken on a local branch, not on this exact
maincommit.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)