From 52b77c20221019b7896fbc128f22d6ff4c975b8b Mon Sep 17 00:00:00 2001 From: EdouardSence <103503538+EdouardSence@users.noreply.github.com> Date: Fri, 9 Oct 2026 09:36:34 +0200 Subject: [PATCH] fix(agc): defer shaders that cannot be prepared at registration to their first use Co-Authored-By: Claude Opus 5.5 --- core/libs/prx/libSceAgcDriver/CMakeLists.txt | 4 +- .../include/Driver/Shaders/ShaderRegistry.hpp | 1 + .../src/Driver/Shaders/ShaderRegistry.cpp | 34 +++++++++++++---- .../libs/prx/libSceAgcDriver/tests/Submit.cpp | 20 ++++++++++ .../tests/execution/PreparedShaders.cpp | 36 +++++++++++++++++- core/shader/recompiler/CMakeLists.txt | 1 + .../include/ControlFlow/GraphBuilder.hpp | 3 ++ .../ControlFlow/src/GraphBuilder.cpp | 38 ++++++++++++++----- 8 files changed, 118 insertions(+), 19 deletions(-) diff --git a/core/libs/prx/libSceAgcDriver/CMakeLists.txt b/core/libs/prx/libSceAgcDriver/CMakeLists.txt index 1d8e5eacf4f..2319d36cdb8 100644 --- a/core/libs/prx/libSceAgcDriver/CMakeLists.txt +++ b/core/libs/prx/libSceAgcDriver/CMakeLists.txt @@ -686,10 +686,12 @@ if(BUILD_TESTING) endif() endforeach() endforeach() - foreach(preparedMode IN ITEMS indirect uncached debug debug_failure) + foreach(preparedMode IN ITEMS indirect uncached debug debug_failure deferred) set(preparedTest "agc_driver_prepared_shaders_${preparedMode}") if(preparedMode STREQUAL "indirect") add_test(NAME ${preparedTest} COMMAND agc_driver_prepared_shaders_tests --indirect) + elseif(preparedMode STREQUAL "deferred") + add_test(NAME ${preparedTest} COMMAND agc_driver_prepared_shaders_tests --deferred) elseif(preparedMode STREQUAL "debug_failure") add_test(NAME ${preparedTest} COMMAND agc_driver_prepared_shaders_tests --fail-before-registration) set_tests_properties(${preparedTest} PROPERTIES WILL_FAIL TRUE TIMEOUT 30) diff --git a/core/libs/prx/libSceAgcDriver/Execution/include/Driver/Shaders/ShaderRegistry.hpp b/core/libs/prx/libSceAgcDriver/Execution/include/Driver/Shaders/ShaderRegistry.hpp index 84672986893..bc8b1647fca 100644 --- a/core/libs/prx/libSceAgcDriver/Execution/include/Driver/Shaders/ShaderRegistry.hpp +++ b/core/libs/prx/libSceAgcDriver/Execution/include/Driver/Shaders/ShaderRegistry.hpp @@ -46,6 +46,7 @@ struct PreparedShaderState { std::vector rectangleProgress; std::vector> fragments; bool rectangleRequested = false; + bool deferred = false; }; struct PreparedShaders : PreparedShaderState { std::mutex mutex; diff --git a/core/libs/prx/libSceAgcDriver/Execution/src/Driver/Shaders/ShaderRegistry.cpp b/core/libs/prx/libSceAgcDriver/Execution/src/Driver/Shaders/ShaderRegistry.cpp index 109b381a7fd..da9e07149d0 100644 --- a/core/libs/prx/libSceAgcDriver/Execution/src/Driver/Shaders/ShaderRegistry.cpp +++ b/core/libs/prx/libSceAgcDriver/Execution/src/Driver/Shaders/ShaderRegistry.cpp @@ -11,6 +11,7 @@ #include "prx/libSceAgcDriver/Execution/include/ShaderPreparationScope.hpp" #include "CompiledVariant.hpp" #include "Optimization/ResourceProgram.hpp" +#include #include #include #include @@ -175,9 +176,11 @@ std::shared_ptr SourceHandleFor(const Shad for (const auto& entry : snapshot.prepared->entries) { if (entry.codeOffset == codeOffset && ShaderRecompiler::MatchesPreparedShader(request, *entry.handle, key)) return entry.handle; } - if (snapshot.header.empty()) { - if (snapshot.type != 0 || request.shader.stage != ShaderRecompiler::ShaderStage::Compute) throw std::runtime_error("AGC driver: unregistered program is not a compute shader"); - APS5_LOG_ERR("Compute shader 0x%llx was not registered; preparing its artifact at dispatch", static_cast(snapshot.codeAddress)); + if (snapshot.header.empty() || snapshot.prepared->deferred) { + if (snapshot.header.empty()) { + if (snapshot.type != 0 || request.shader.stage != ShaderRecompiler::ShaderStage::Compute) throw std::runtime_error("AGC driver: unregistered program is not a compute shader"); + APS5_LOG_ERR("Compute shader 0x%llx was not registered; preparing its artifact at dispatch", static_cast(snapshot.codeAddress)); + } auto handle = ShaderRecompiler::PrepareShader(request); snapshot.prepared->entries.push_back({codeOffset, handle}); return handle; @@ -216,9 +219,11 @@ ShaderRecompiler::PreparedShaderInvocation InvocationFor(const ShaderSnapshot& s invocationRequest.shader.code = ShaderRecompiler::GetPreparedCode(*entry.handle); if (auto invocation = ShaderRecompiler::PreparedShaderInvocation::TryCreate(invocationRequest, entry.handle, key)) return std::move(*invocation); } - if (snapshot.header.empty()) { - if (snapshot.type != 0 || request.shader.stage != ShaderRecompiler::ShaderStage::Compute) throw std::runtime_error("AGC driver: unregistered program is not a compute shader"); - APS5_LOG_ERR("Compute shader 0x%llx was not registered; preparing its artifact at dispatch", static_cast(snapshot.codeAddress)); + if (snapshot.header.empty() || snapshot.prepared->deferred) { + if (snapshot.header.empty()) { + if (snapshot.type != 0 || request.shader.stage != ShaderRecompiler::ShaderStage::Compute) throw std::runtime_error("AGC driver: unregistered program is not a compute shader"); + APS5_LOG_ERR("Compute shader 0x%llx was not registered; preparing its artifact at dispatch", static_cast(snapshot.codeAddress)); + } auto handle = ShaderRecompiler::PrepareShader(request); invocationRequest = request; invocationRequest.shader.code = ShaderRecompiler::GetPreparedCode(*handle); @@ -307,7 +312,7 @@ void BuildRegisteredAbiKey(const QueueState& state, const VulkanDevice& device, } } -std::vector PrepareRegistered(const ShaderSnapshot& snapshot, const VulkanDevice& device, const QueueState& state, bool registration) { +std::vector PrepareRegistered(const ShaderSnapshot& snapshot, const VulkanDevice& device, const QueueState& state, bool registration, bool* deferred = nullptr) { using Stage = ShaderRecompiler::ShaderStage; const auto header = ReadHeader(snapshot); std::uint32_t programRegister; @@ -324,6 +329,12 @@ std::vector PrepareRegistered(const ShaderSnapshot& snap case 7: stage = Stage::TessellationControl; programRegister = 0x108; resourceRegister = 0x10b; break; default: throw std::runtime_error("AGC driver: unsupported registered shader type"); } + const auto defer = [&](const char* reason) { + APS5_LOG_ERR("Shader 0x%llx could not be prepared at registration (%s); preparing it when it is used", static_cast(snapshot.codeAddress), reason); + *deferred = true; + return std::vector{}; + }; + if (deferred != nullptr && (!state.shader.contains(programRegister) || !state.shader.contains(programRegister + 1))) return defer("its program address is not in the shader registers"); const auto high = RegisterValue(state.shader, programRegister + 1); if ((high & ~0xffu) != 0) throw std::runtime_error("AGC driver: invalid registered program address"); const auto address = (static_cast(RegisterValue(state.shader, programRegister)) << 8u) | (static_cast(high) << 40u); @@ -366,6 +377,13 @@ std::vector PrepareRegistered(const ShaderSnapshot& snap std::vector userData(userCount); if (stage != Stage::Compute && stage != Stage::Fragment && snapshot.type != 6) vertex = Graphics::DecodeVertexStageInfo(snapshot.header, snapshot.headerAddress, userData, nullptr, true); const ShaderRecompiler::SwappcInfo swappc{vertex.has_value(), firstUser, userCount}; + if (deferred != nullptr) { + if (const auto counter = ShaderRecompiler::UnresolvableSwappcTarget(decoded, &swappc)) { + char reason[96]; + std::snprintf(reason, sizeof(reason), "its s_swappc_b64 at program counter 0x%x calls a computed target", *counter); + return defer(reason); + } + } auto graph = ShaderRecompiler::GraphBuilder{}.Build(decoded, &swappc); ShaderRecompiler::Structurizer{}.Structurize(graph); const std::array memory{{{snapshot.codeAddress, std::as_bytes(std::span(snapshot.code))}, {snapshot.headerAddress, snapshot.header}}}; @@ -705,7 +723,7 @@ void Driver::RegisterShader(const Shader* shader) { registered.shader = snapshot.registeredState->shader; registered.context = snapshot.registeredState->context; registered.userConfig = snapshot.registeredState->userConfig; - snapshot.prepared->entries = PrepareRegistered(snapshot, *localDevice, registered, true); + snapshot.prepared->entries = PrepareRegistered(snapshot, *localDevice, registered, true, &snapshot.prepared->deferred); if ((snapshot.type == 0 || snapshot.type == 1) && !snapshot.prepared->entries.empty()) { std::vector key; BuildRegisteredAbiKey(registered, *localDevice, key); diff --git a/core/libs/prx/libSceAgcDriver/tests/Submit.cpp b/core/libs/prx/libSceAgcDriver/tests/Submit.cpp index e774ae46e25..256bca9a91f 100644 --- a/core/libs/prx/libSceAgcDriver/tests/Submit.cpp +++ b/core/libs/prx/libSceAgcDriver/tests/Submit.cpp @@ -481,6 +481,25 @@ void testShaderHeaderAlignment() { refused(expectFailure([&] { AgcDriverRegisterShader_nid_postfix(at(4, truncated)); }), "smaller than its fixed fields"); } +void testHeaderWithoutProgramAddress() { + alignas(256) static const std::array code{0xbf810000}; + struct Header { + Shader shader{}; + std::array registers{}; + }; + alignas(8) static Header header{}; + header.shader.file_header = 0x34333231; + header.shader.version = 0x18; + header.shader.header_size = sizeof(Header); + header.shader.shader_size = sizeof(code); + header.shader.code = code.data(); + header.shader.type = 4; + header.registers = {{{0x08a, 0x60000002}, {0x08b, 0x00030008}, {0x0ca, 0x03000002}}}; + header.shader.sh_registers = header.registers.data(); + header.shader.num_sh_registers = header.registers.size(); + AgcDriverRegisterShader_nid_postfix(&header.shader); +} + void testRegisteredFloatMode() { using AgcDriver::DriverDetail::RegisteredFloatMode; using AgcDriver::DriverDetail::ShaderSnapshot; @@ -616,6 +635,7 @@ int main() { testWaitFreeSubmissionTheCpuWaitsFor(); testMultiSubmissions(); testShaderHeaderAlignment(); + testHeaderWithoutProgramAddress(); testRegisteredFloatMode(); testWorkerFailure(); check(expectFailure([] { LibcRunShutdown_nid_postfix(); }).find("required shader register") != std::string::npos, "shutdown lost worker failure"); diff --git a/core/libs/prx/libSceAgcDriver/tests/execution/PreparedShaders.cpp b/core/libs/prx/libSceAgcDriver/tests/execution/PreparedShaders.cpp index 977f26c1a9b..111161d5e4a 100644 --- a/core/libs/prx/libSceAgcDriver/tests/execution/PreparedShaders.cpp +++ b/core/libs/prx/libSceAgcDriver/tests/execution/PreparedShaders.cpp @@ -261,11 +261,45 @@ void Registration(bool indirect) { Require(destination == 0, "failed dispatch executed a subsequent memory write"); } +void DeferredRegistration() { + alignas(256) std::array code{0xbe842104u, 0xbf810000u}; + struct Header { + Shader shader{}; + std::array registers{}; + ShaderSpecialRegs specials{}; + } header; + const auto address = reinterpret_cast(code.data()); + header.shader.file_header = 0x34333231u; + header.shader.version = 0x18; + header.shader.header_size = sizeof(header); + header.shader.shader_size = sizeof(code); + header.shader.code = code.data(); + header.shader.sh_registers = header.registers.data(); + header.shader.num_sh_registers = header.registers.size(); + header.shader.specials = &header.specials; + header.specials.dispatch_modifier = 0x8000; + header.registers = {{{0x20c, static_cast(address >> 8u)}, {0x20d, static_cast(address >> 40u)}, {0x207, 1}, {0x208, 1}, {0x209, 1}, {0x212, 0}, {0x213, 12}, {0x207, 1}}}; + AgcDriverRegisterShader_nid_postfix(&header.shader); + std::vector commands; + for (const auto reg : header.registers) commands.insert(commands.end(), {0xc0017600u, reg.offset, reg.value}); + commands.insert(commands.end(), {0xc0031500u, 1, 1, 1, 0x8041}); + Packet packet{commands.data(), static_cast(commands.size()), 0, {}}; + sceAgcDriverSubmitAcb(0x20, &packet); + ExpectFailure([] { AgcDriverWaitIdle_nid_postfix(); }, "not statically resolvable"); + ExpectFailure([] { AgcDriverShutdown_nid_postfix(); }, "not statically resolvable"); +} + } int main(int argc, char** argv) { try { - Require(argc == 1 || (argc == 2 && (std::string_view(argv[1]) == "--indirect" || std::string_view(argv[1]) == "--fail-before-registration")), "invalid test arguments"); + Require(argc == 1 || (argc == 2 && (std::string_view(argv[1]) == "--indirect" || std::string_view(argv[1]) == "--fail-before-registration" || std::string_view(argv[1]) == "--deferred")), "invalid test arguments"); + if (argc == 2 && std::string_view(argv[1]) == "--deferred") { + if (!OpenVulkanTestDevice()) return VulkanTestSkipped; + DeferredRegistration(); + std::cout << "deferred shader preparation tests passed\n"; + return 0; + } auto device = OpenVulkanTestDevice(); if (!device) return VulkanTestSkipped; Run(*device); diff --git a/core/shader/recompiler/CMakeLists.txt b/core/shader/recompiler/CMakeLists.txt index 324eca0f0dc..3b563d00fd7 100644 --- a/core/shader/recompiler/CMakeLists.txt +++ b/core/shader/recompiler/CMakeLists.txt @@ -235,6 +235,7 @@ function(add_shader_recompiler target runtimeTarget) SrgbDecodeBit SrgbUnormFormat ThreadOwned + UnresolvableSwappcTarget WriteFileAtomically ) set(shaderCacheManifest) diff --git a/core/shader/recompiler/ControlFlow/include/ControlFlow/GraphBuilder.hpp b/core/shader/recompiler/ControlFlow/include/ControlFlow/GraphBuilder.hpp index bca0da8c96e..1b450325e2f 100644 --- a/core/shader/recompiler/ControlFlow/include/ControlFlow/GraphBuilder.hpp +++ b/core/shader/recompiler/ControlFlow/include/ControlFlow/GraphBuilder.hpp @@ -4,6 +4,7 @@ #include "ControlFlow/ControlFlowGraph.hpp" #include "RdnaDecoder/RdnaProgram.hpp" #include +#include #include namespace ShaderRecompiler { @@ -24,6 +25,8 @@ struct SwappcCall { std::uint32_t returnTargetProgramCounter = 0; }; +[[nodiscard]] std::optional UnresolvableSwappcTarget(const RdnaProgram& program, const SwappcInfo* swappc); + class GraphBuilder { public: [[nodiscard]] ControlFlowGraph Build(const RdnaProgram& program, const SwappcInfo* swappc = nullptr) const; diff --git a/core/shader/recompiler/ControlFlow/src/GraphBuilder.cpp b/core/shader/recompiler/ControlFlow/src/GraphBuilder.cpp index 1bd3c7776f7..dda7aef70ae 100644 --- a/core/shader/recompiler/ControlFlow/src/GraphBuilder.cpp +++ b/core/shader/recompiler/ControlFlow/src/GraphBuilder.cpp @@ -463,6 +463,18 @@ const SwappcCall* findReturnAt(const std::vector& calls, std::uint32 return nullptr; } +bool isFetchCall(const RdnaProgram& program, std::uint32_t index, const SwappcInfo* swappc, bool outsideProgram) { + const auto& instruction = program.instructions[index]; + const bool positional = std::all_of(program.instructions.begin(), program.instructions.begin() + index, isFetchCallPrefixOpcode); + const std::uint32_t targetRegister = scalarIndex(instruction.source0); + const bool userDataPair = swappc != nullptr && targetRegister != NoScalarRegister && + targetRegister >= swappc->userDataBaseRegister && targetRegister + 1u < swappc->userDataBaseRegister + swappc->userDataCount && + std::none_of(program.instructions.begin(), program.instructions.end(), [&](const RdnaInstruction& other) { + return writesScalar(other, targetRegister) || writesScalar(other, targetRegister + 1u); + }); + return swappc != nullptr && swappc->fetchCallAllowed && positional && (userDataPair || outsideProgram); +} + std::vector analyzeSwappcCalls(const RdnaProgram& program, const SwappcInfo* swappc) { std::vector calls; for (std::uint32_t index = 0; index < program.instructions.size(); ++index) { @@ -489,15 +501,7 @@ std::vector analyzeSwappcCalls(const RdnaProgram& program, const Swa if (instruction.op == RdnaOpcode::SCallB64 && call.targetIndex == InvalidControlFlowId) { throw std::invalid_argument("s_call_b64 at program counter " + toHexString(instruction.programCounter) + " targets invalid instruction boundary " + toHexString(target)); } - const bool positional = std::all_of(program.instructions.begin(), program.instructions.begin() + index, isFetchCallPrefixOpcode); - const std::uint32_t targetRegister = scalarIndex(instruction.source0); - const bool userDataPair = swappc != nullptr && targetRegister != NoScalarRegister && - targetRegister >= swappc->userDataBaseRegister && targetRegister + 1u < swappc->userDataBaseRegister + swappc->userDataCount && - std::none_of(program.instructions.begin(), program.instructions.end(), [&](const RdnaInstruction& other) { - return writesScalar(other, targetRegister) || writesScalar(other, targetRegister + 1u); - }); - const bool outsideProgram = staticTarget && call.targetIndex == InvalidControlFlowId; - if (instruction.op == RdnaOpcode::SSwappcB64 && swappc != nullptr && swappc->fetchCallAllowed && positional && (userDataPair || outsideProgram)) { + if (instruction.op == RdnaOpcode::SSwappcB64 && isFetchCall(program, index, swappc, staticTarget && call.targetIndex == InvalidControlFlowId)) { call.fetch = true; call.returnTargetProgramCounter = instructionEndProgramCounter(instruction); calls.push_back(call); @@ -890,6 +894,22 @@ void GraphBuilder::linkBlocks(std::vector& blocks, const RdnaProgram } } +std::optional UnresolvableSwappcTarget(const RdnaProgram& program, const SwappcInfo* swappc) { + for (std::uint32_t index = 0; index < program.instructions.size(); ++index) { + const auto& instruction = program.instructions[index]; + if (instruction.op != RdnaOpcode::SSwappcB64) { + continue; + } + std::uint32_t target = instruction.branchTarget; + const bool staticTarget = resolveSetpcTarget(program, index, target); + const bool inProgram = staticTarget && instructionIndexOfProgramCounter(program, target) != InvalidControlFlowId; + if (!inProgram && !isFetchCall(program, index, swappc, staticTarget)) { + return instruction.programCounter; + } + } + return std::nullopt; +} + ControlFlowGraph GraphBuilder::Build(const RdnaProgram& program, const SwappcInfo* swappc) const { const std::vector calls = analyzeSwappcCalls(program, swappc); ControlFlowGraph graph;