From a431082abc1efef6f9350cec85dfe3379eb573c5 Mon Sep 17 00:00:00 2001 From: Dean Galvin Date: Sat, 3 Oct 2026 19:27:09 +1300 Subject: [PATCH] perf(agc): keep a surface's storage image when another descriptor names uncompressed DCC keys PPSA21564's galaxy map renders a 1920x1080 surface as a color target whose CB names DCC keys at one address and then writes it from a compute shader whose T# names keys at another. Both key ranges read uncompressed every frame, but the storage cache remade the surface's image whenever a descriptor named metadata other than the image's, so the image was flushed, destroyed, created and uploaded twice a frame (1231 constructions in a 56 s scene), with the views and draw recipes built on it thrown away each time. A descriptor naming other keys now keeps the image when the move is unobservable: the image's own keys and the named keys both read uncompressed, and its content was neither uploaded nor filled under a clear code. That is exactly the content an image made under the named keys would hold once this one's pending results were stored. Any other case (a fast clear of either key range, a register clear, an image holding a clear) still remakes the image as before, so clears of the new keys reach it and pending results are stored first. Every proof that stands in for a lookup checks the same rule: the sampled view hit, the fast revalidation of views and storage elements (storage elements now record their descriptor's keys), the own-object refresh, and the draw recipe's resident targets. Named-key scans keep per-image proofs, so a repeated check costs no scan while the key bytes are unstamped. agc_dcc_key_sharing drives KeysServeSurface, the rule ServesKeysAt uses, through a model of the storage cache. --- core/libs/prx/libSceAgcDriver/CMakeLists.txt | 14 ++ .../Graphics/include/DccMetadata.hpp | 8 + .../Graphics/include/ShaderResources.hpp | 2 + .../Graphics/include/Texture.hpp | 7 + .../prx/libSceAgcDriver/Graphics/src/Draw.cpp | 2 +- .../Graphics/src/ShaderResources.cpp | 17 +- .../libSceAgcDriver/Graphics/src/Texture.cpp | 21 +++ .../libSceAgcDriver/tests/DccKeySharing.cpp | 150 ++++++++++++++++++ 8 files changed, 218 insertions(+), 3 deletions(-) create mode 100644 core/libs/prx/libSceAgcDriver/tests/DccKeySharing.cpp diff --git a/core/libs/prx/libSceAgcDriver/CMakeLists.txt b/core/libs/prx/libSceAgcDriver/CMakeLists.txt index 0e0164af0..4ca9b6773 100644 --- a/core/libs/prx/libSceAgcDriver/CMakeLists.txt +++ b/core/libs/prx/libSceAgcDriver/CMakeLists.txt @@ -226,6 +226,20 @@ target_link_libraries(agc_vop1_control_decode_tests PRIVATE shader_recompiler_ag set_target_properties(agc_vop1_control_decode_tests PROPERTIES CXX_EXTENSIONS OFF) configure_windows_unwind(agc_vop1_control_decode_tests) +add_test_executable(agc_dcc_key_sharing_tests EXCLUDE_FROM_ALL tests/DccKeySharing.cpp) +target_include_directories(agc_dcc_key_sharing_tests PRIVATE ${LIBS_INCLUDE_DIR} ${CMAKE_SOURCE_DIR}/core/shader/recompiler ${CMAKE_SOURCE_DIR}/3rdparty/Vulkan-Headers/include) +target_link_libraries(agc_dcc_key_sharing_tests PRIVATE libc) +set_target_properties(agc_dcc_key_sharing_tests PROPERTIES CXX_EXTENSIONS OFF) +configure_windows_unwind(agc_dcc_key_sharing_tests) +if(BUILD_TESTING) + set_target_properties(agc_dcc_key_sharing_tests PROPERTIES EXCLUDE_FROM_ALL OFF) + add_test(NAME agc_dcc_key_sharing COMMAND agc_dcc_key_sharing_tests) + if(WIN32) + set_tests_properties(agc_dcc_key_sharing PROPERTIES ENVIRONMENT_MODIFICATION + "PATH=path_list_prepend:$") + endif() +endif() + add_test_executable(agc_shader_disk_cache_tests EXCLUDE_FROM_ALL tests/ShaderDiskCache.cpp) target_include_directories(agc_shader_disk_cache_tests PRIVATE ${LIBS_INCLUDE_DIR}) target_link_libraries(agc_shader_disk_cache_tests PRIVATE shader_recompiler_agc_driver libc) diff --git a/core/libs/prx/libSceAgcDriver/Graphics/include/DccMetadata.hpp b/core/libs/prx/libSceAgcDriver/Graphics/include/DccMetadata.hpp index 3cd024d6c..3efc2b3b2 100644 --- a/core/libs/prx/libSceAgcDriver/Graphics/include/DccMetadata.hpp +++ b/core/libs/prx/libSceAgcDriver/Graphics/include/DccMetadata.hpp @@ -21,6 +21,14 @@ struct Context; enum class DccKeys { Uncompressed, Clear0000, Clear0001, Clear1110, Clear1111, ClearRegister, Mixed, Unreadable }; const char* DccKeysName(DccKeys keys); + +template +bool KeysServeSurface(std::uint64_t followedDcc, DccKeys uploaded, DccKeys filled, std::uint64_t namedDcc, ReadFollowed&& readFollowed, ReadNamed&& readNamed) { + if (namedDcc == 0 || namedDcc == followedDcc) return true; + if (uploaded != DccKeys::Uncompressed || filled != DccKeys::Uncompressed) return false; + if (followedDcc != 0 && readFollowed() != DccKeys::Uncompressed) return false; + return readNamed() == DccKeys::Uncompressed; +} // The keys covering a surface of `surfaceBytes`, when they all agree. DccKeys ReadDccKeys(std::uint64_t metaAddress, std::uint64_t surfaceBytes); bool IsDccClear(DccKeys keys); diff --git a/core/libs/prx/libSceAgcDriver/Graphics/include/ShaderResources.hpp b/core/libs/prx/libSceAgcDriver/Graphics/include/ShaderResources.hpp index b48c42c3c..e06800b11 100644 --- a/core/libs/prx/libSceAgcDriver/Graphics/include/ShaderResources.hpp +++ b/core/libs/prx/libSceAgcDriver/Graphics/include/ShaderResources.hpp @@ -30,6 +30,7 @@ std::shared_ptr CachedStorageSurface(const Context& context, con // Whether `image` is still the storage cache's image of its surface (what CachedStorageSurface // would return); a true answer counts as a use for the cache's eviction order, as a lookup would. bool StorageImageCached(const Context& context, const StorageTexture* image); +bool StorageImageServesKeys(const StorageTexture& image, std::uint64_t dccAddress); // Defined in Texture.cpp beside the pending-results registry, for the fast Revalidate below: whether // a storage image other than `except` has results pending in [address, address + bytes). @@ -399,6 +400,7 @@ private: std::vector textureFirstLayer; std::vector> storageTextures; std::vector storageMips; + std::vector storageKeys; std::vector storageFirstLayer; std::vector storageWritten; std::vector> samplers; diff --git a/core/libs/prx/libSceAgcDriver/Graphics/include/Texture.hpp b/core/libs/prx/libSceAgcDriver/Graphics/include/Texture.hpp index f93986402..ed80ade6c 100644 --- a/core/libs/prx/libSceAgcDriver/Graphics/include/Texture.hpp +++ b/core/libs/prx/libSceAgcDriver/Graphics/include/Texture.hpp @@ -264,6 +264,7 @@ public: DccKeys UploadedKeys() const { return uploadedKeys; } DccKeys FilledKeys() const { return filledKeys; } DccKeyProof& KeyProof() const { return keyProof; } + bool ServesKeysAt(std::uint64_t dccAddress) const; // Brings the image up to date with guest memory before another use; returns whether its content // was still current (nothing uploaded). // Keeps the image current with guest memory (see GuestMemory::CollectWrites). @@ -387,6 +388,12 @@ private: DccKeys uploadedKeys = DccKeys::Uncompressed; DccKeys filledKeys = DccKeys::Uncompressed; mutable DccKeyProof keyProof; + struct ForeignKeyProof { + std::uint64_t dccAddress = 0; + DccKeyProof proof; + }; + mutable std::array foreignKeyProofs{}; + mutable std::uint32_t nextForeignKeyProof = 0; // Write generation `original` is known current at (the oldest of layerGeneration). std::uint64_t generation = 0; std::uint32_t trackedLayers = 1; diff --git a/core/libs/prx/libSceAgcDriver/Graphics/src/Draw.cpp b/core/libs/prx/libSceAgcDriver/Graphics/src/Draw.cpp index 1c9e47706..598c16422 100644 --- a/core/libs/prx/libSceAgcDriver/Graphics/src/Draw.cpp +++ b/core/libs/prx/libSceAgcDriver/Graphics/src/Draw.cpp @@ -1897,7 +1897,7 @@ DrawRecipeOutcome DrawWithRecipe(const Context& context, const State& state, con for (std::size_t index = 0; index < recipe.targets.size(); ++index) { timer.phase(PhaseSetup); auto stored = recipe.targets[index].lock(); - if (stored == nullptr || !StorageImageCached(context, stored.get())) return miss(DrawRecipeMiss::TargetGone); + if (stored == nullptr || !StorageImageCached(context, stored.get()) || !StorageImageServesKeys(*stored, state.colors[index].dccAddress)) return miss(DrawRecipeMiss::TargetGone); auto resident = refreshResidentTarget(context, state, state.colors[index], outcome, profile, [&] { stored->Refresh(); return stored; diff --git a/core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp b/core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp index 9c5bbcf75..c65a4bb3e 100644 --- a/core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp +++ b/core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp @@ -199,7 +199,7 @@ void logLookup(const LookupRecord& record) { std::shared_ptr cachedStorageTexture(const Context& context, std::span words, const GuestTextureResource& resource, std::uint32_t mip, std::uint64_t guestBytes = 0); bool MetadataMoved(const StorageTexture& image, const GuestTextureResource& resource) { - return resource.dccAddress != 0 && image.Descriptor().dccAddress != resource.dccAddress; + return resource.dccAddress != 0 && image.Descriptor().dccAddress != resource.dccAddress && !image.ServesKeysAt(resource.dccAddress); } // Whether a sampled texture over `resource` can be a view of the surface's cached storage image @@ -377,7 +377,7 @@ std::shared_ptr cachedTexture(const Context& context, std::spansource) && (source != nullptr || *keys == DccKeys::Uncompressed)) { + if ((source == nullptr || source == it->source) && (source != nullptr || *keys == DccKeys::Uncompressed) && StorageImageServesKeys(*it->source, resource.dccAddress)) { if (!GuestMemory::UnchangedSince(address, bytes, it->source->Generation())) it->source->Refresh(); it->keys = *keys; touchTexture(cache, it); @@ -631,6 +631,12 @@ std::shared_ptr CachedStorageSurface(const Context& context, con return cachedStorageTexture(context, {}, resource, 0); } +bool StorageImageServesKeys(const StorageTexture& image, std::uint64_t dccAddress) { + GuestTextureResource resource{}; + resource.dccAddress = dccAddress; + return !MetadataMoved(image, resource); +} + namespace { const char* roleName(ShaderRecompiler::DescriptorRole role) { @@ -1373,6 +1379,7 @@ bool ShaderResources::fastRevalidate(std::uint64_t serialBefore, std::spanGuestBytes(), source->KeyProof()) : DccKeys::Uncompressed; if (sourceKeys != source->UploadedKeys()) return fail(FastFail::Keys); const auto keys = surface.resource.dccAddress == 0 ? DccKeys::Uncompressed : SameKeySurface(source, surface.resource, surface.bytes) ? sourceKeys : ProvedClearKeys(surface.resource, surface.bytes, textures[i]->KeyProof()); + if (surface.resource.dccAddress != 0 && surface.resource.dccAddress != own.dccAddress && (keys != DccKeys::Uncompressed || !StorageImageServesKeys(*source, surface.resource.dccAddress))) return fail(FastFail::Keys); scannedKeys[i] = keys; // A view of a fast-cleared surface stays one only while its image still has results // pending over it (cachedTexture's hit rule), unless the image's own descriptor names @@ -1399,6 +1406,7 @@ bool ShaderResources::fastRevalidate(std::uint64_t serialBefore, std::span= storageKeys.size() || !StorageImageServesKeys(*storageTextures[i], storageKeys[i]))) return fail(FastFail::StorageKeys); if (i != 0 && storageTextures[i] == storageTextures[i - 1]) continue; const auto* image = storageTextures[i].get(); if (image == nullptr) return fail(FastFail::NoRecord); @@ -1473,6 +1481,7 @@ bool ShaderResources::fastRevalidateEach() { surface.collected = GuestMemory::CollectWrites(address, bytes); if (surface.collected == 0) return false; if (PendingStorageOverlaps(address, bytes, surface.source)) return false; + if (surface.source != nullptr && surface.resource.dccAddress != 0 && surface.resource.dccAddress != surface.source->Descriptor().dccAddress) return false; if (surface.source != nullptr) { // The view follows the image: it needs the image current with guest memory, as the // lookup's Refresh would make it. @@ -1487,6 +1496,7 @@ bool ShaderResources::fastRevalidateEach() { if (surface.source != nullptr && surface.keys != DccKeys::Uncompressed && StorageTexture::FindPending(address, surface.bytes).get() != surface.source) return false; } for (std::size_t i = 0; i < storageTextures.size(); ++i) { + if (storageTextures[i] != nullptr && (i >= storageKeys.size() || (storageKeys[i] != 0 && storageKeys[i] != storageTextures[i]->Descriptor().dccAddress))) return false; // Consecutive elements of one mip chain share the image (see addStorageImageBinding). if (i != 0 && storageTextures[i] == storageTextures[i - 1]) continue; const auto* image = storageTextures[i].get(); @@ -1530,6 +1540,7 @@ ShaderResources::OwnRefreshFallback ShaderResources::refreshOwnObjects(std::span const auto& source = textures[i]->SharedStorageSource(); if (source == nullptr || source.get() != surface.source) return OwnRefreshFallback::Snapshot; if (!source->Cached()) return OwnRefreshFallback::Uncached; + if (!StorageImageServesKeys(*source, surface.resource.dccAddress)) return OwnRefreshFallback::Keys; const auto address = surface.resource.baseAddress; const auto bytes = static_cast(surface.bytes); const bool imported = SampledFromStorageEligible(context, surface.resource, surface.bytes); @@ -1557,6 +1568,7 @@ ShaderResources::OwnRefreshFallback ShaderResources::refreshOwnObjects(std::span const auto refreshStorage = [&](std::size_t i) { auto& image = storageTextures[i]; if (image == nullptr || !image->Cached()) return OwnRefreshFallback::Uncached; + if (i >= storageKeys.size() || !StorageImageServesKeys(*image, storageKeys[i])) return OwnRefreshFallback::Keys; countOwnRefresh(true); image->Refresh(); return OwnRefreshFallback::Count; @@ -2479,6 +2491,7 @@ void ShaderResources::resolveImageBinding(const ShaderRecompiler::DescriptorBind if (sameAsPrevious && StorageDedupeEnabled()) storageTextures.push_back(storageTextures.back()); else storageTextures.push_back(cachedStorageTexture(context, words, resource, mip, guestBytes)); storageMips.push_back(mip); + storageKeys.push_back(resource.dccAddress); storageFirstLayer.push_back(firstLayer); // Images the shader only reads have nothing to store back. storageWritten.push_back(element >= binding.imageWritten.size() || binding.imageWritten[element]); diff --git a/core/libs/prx/libSceAgcDriver/Graphics/src/Texture.cpp b/core/libs/prx/libSceAgcDriver/Graphics/src/Texture.cpp index a3f312e8b..5d813af9f 100644 --- a/core/libs/prx/libSceAgcDriver/Graphics/src/Texture.cpp +++ b/core/libs/prx/libSceAgcDriver/Graphics/src/Texture.cpp @@ -1213,6 +1213,27 @@ bool StorageTexture::Refresh() { return false; } +bool StorageTexture::ServesKeysAt(std::uint64_t dccAddress) const { + const bool locked = GuestMemory::GpuMutex().HeldByThisThread(); + const auto readFollowed = [&] { + DccKeyProof unlocked; + return ProvedClearKeys(descriptor, guestBytes, locked ? keyProof : unlocked); + }; + const auto readNamed = [&] { + auto named = descriptor; + named.dccAddress = dccAddress; + DccKeyProof unlocked; + if (!locked) return ProvedClearKeys(named, guestBytes, unlocked); + auto slot = std::find_if(foreignKeyProofs.begin(), foreignKeyProofs.end(), [&](const ForeignKeyProof& entry) { return entry.dccAddress == dccAddress; }); + if (slot == foreignKeyProofs.end()) { + slot = foreignKeyProofs.begin() + nextForeignKeyProof++ % foreignKeyProofs.size(); + *slot = ForeignKeyProof{dccAddress, {}}; + } + return ProvedClearKeys(named, guestBytes, slot->proof); + }; + return KeysServeSurface(descriptor.dccAddress, uploadedKeys, filledKeys, dccAddress, readFollowed, readNamed); +} + namespace { // Debug aid (APS5_TRACE_DCC_KEYS=1): who stores uncompressed keys over a surface (recommendation diff --git a/core/libs/prx/libSceAgcDriver/tests/DccKeySharing.cpp b/core/libs/prx/libSceAgcDriver/tests/DccKeySharing.cpp new file mode 100644 index 000000000..eda84ef40 --- /dev/null +++ b/core/libs/prx/libSceAgcDriver/tests/DccKeySharing.cpp @@ -0,0 +1,150 @@ +#include "prx/libSceAgcDriver/Graphics/include/DccMetadata.hpp" +#include +#include +#include +#include + +using namespace AgcDriver::Graphics; + +namespace { + +int failures = 0; + +void Expect(bool condition, const char* what) { + if (condition) return; + std::fprintf(stderr, "FAIL: %s\n", what); + ++failures; +} + +struct Keys { + std::map ranges; + int reads = 0; + + DccKeys Read(std::uint64_t address) { + ++reads; + if (address == 0) return DccKeys::Uncompressed; + const auto it = ranges.find(address); + return it == ranges.end() ? DccKeys::Uncompressed : it->second; + } +}; + +struct Image { + std::uint64_t dcc = 0; + DccKeys uploaded = DccKeys::Uncompressed; + DccKeys filled = DccKeys::Uncompressed; +}; + +struct SurfaceCache { + Keys& keys; + bool shared = true; + std::optional image; + int made = 0; + int remade = 0; + int uploads = 0; + + bool Serves(const Image& held, std::uint64_t named) { + if (!shared) return named == 0 || named == held.dcc; + return KeysServeSurface(held.dcc, held.uploaded, held.filled, named, [&] { return keys.Read(held.dcc); }, [&] { return keys.Read(named); }); + } + + const Image& Lookup(std::uint64_t named) { + if (image.has_value() && Serves(*image, named)) { + const auto current = keys.Read(image->dcc); + if (current != image->uploaded) { + image->uploaded = current; + ++uploads; + } + return *image; + } + if (image.has_value()) ++remade; + else ++made; + image = Image{named, keys.Read(named), DccKeys::Uncompressed}; + ++uploads; + return *image; + } +}; + +constexpr std::uint64_t TargetKeys = 0x570526000; +constexpr std::uint64_t StorageKeys = 0x5705e0000; + +void worldMapFrames() { + for (const bool shared : {true, false}) { + Keys keys; + SurfaceCache cache{keys, shared}; + for (int frame = 0; frame < 100; ++frame) { + cache.Lookup(TargetKeys); + cache.Lookup(StorageKeys); + cache.Lookup(StorageKeys); + } + if (shared) { + Expect(cache.made == 1 && cache.remade == 0, "a surface whose two descriptors name uncompressed keys was remade"); + Expect(cache.uploads == 1, "a shared surface was uploaded again with nothing changed"); + Expect(cache.image->dcc == TargetKeys, "the shared image left the keys it follows"); + } else { + Expect(cache.remade == 199, "the unshared model does not reproduce the map's two remakes a frame"); + } + } +} + +void namedClearRemakes() { + Keys keys; + SurfaceCache cache{keys}; + cache.Lookup(TargetKeys); + keys.ranges[StorageKeys] = DccKeys::Clear0000; + const auto& image = cache.Lookup(StorageKeys); + Expect(cache.remade == 1 && image.dcc == StorageKeys && image.uploaded == DccKeys::Clear0000, "a fast clear of the named keys did not reach the surface's image"); + const auto& own = cache.Lookup(StorageKeys); + Expect(cache.remade == 1 && own.uploaded == DccKeys::Clear0000, "the image of the cleared keys did not serve its own keys"); + cache.Lookup(TargetKeys); + Expect(cache.remade == 2, "uncompressed keys shared an image holding a clear"); +} + +void followedClearRemakes() { + Keys keys; + SurfaceCache cache{keys}; + cache.Lookup(TargetKeys); + keys.ranges[TargetKeys] = DccKeys::Clear1111; + const auto& image = cache.Lookup(StorageKeys); + Expect(cache.remade == 1 && image.dcc == StorageKeys && image.uploaded == DccKeys::Uncompressed, "a descriptor over uncompressed keys was served an image whose own keys are a clear"); +} + +void filledKeysRemake() { + Keys keys; + SurfaceCache cache{keys}; + cache.Lookup(TargetKeys); + cache.image->filled = DccKeys::Clear0001; + cache.Lookup(StorageKeys); + Expect(cache.remade == 1, "an image holding a key fill served other keys"); +} + +void serveRules() { + Keys keys; + keys.ranges[TargetKeys] = DccKeys::Clear0000; + Expect(KeysServeSurface(TargetKeys, DccKeys::Clear0000, DccKeys::Uncompressed, 0, [&] { return keys.Read(TargetKeys); }, [&] { return keys.Read(0); }), "a descriptor without metadata was refused"); + Expect(KeysServeSurface(TargetKeys, DccKeys::Clear0000, DccKeys::Uncompressed, TargetKeys, [&] { return keys.Read(TargetKeys); }, [&] { return keys.Read(TargetKeys); }), "the image's own keys were refused"); + Expect(keys.reads == 0, "the own-keys and no-metadata answers scanned keys"); + Expect(!KeysServeSurface(TargetKeys, DccKeys::Clear0000, DccKeys::Uncompressed, StorageKeys, [&] { return keys.Read(TargetKeys); }, [&] { return keys.Read(StorageKeys); }), "an image uploaded under a clear served other keys"); + Expect(keys.reads == 0, "a refusal by the uploaded keys scanned keys"); + keys.ranges[StorageKeys] = DccKeys::Mixed; + keys.ranges[TargetKeys] = DccKeys::Uncompressed; + Expect(!KeysServeSurface(TargetKeys, DccKeys::Uncompressed, DccKeys::Uncompressed, StorageKeys, [&] { return keys.Read(TargetKeys); }, [&] { return keys.Read(StorageKeys); }), "keys that do not read uncompressed were served"); + int followedReads = 0; + Expect(KeysServeSurface(0, DccKeys::Uncompressed, DccKeys::Uncompressed, TargetKeys, [&] { ++followedReads; return DccKeys::Clear0000; }, [&] { return keys.Read(TargetKeys); }), "an image without metadata did not serve uncompressed keys"); + Expect(followedReads == 0, "an image without metadata scanned keys of its own"); +} + +} + +int main() { + worldMapFrames(); + namedClearRemakes(); + followedClearRemakes(); + filledKeysRemake(); + serveRules(); + if (failures != 0) { + std::fprintf(stderr, "%d DCC key sharing checks failed\n", failures); + return 1; + } + std::printf("DCC key sharing tests passed\n"); + return 0; +}