From e2664bef839b3fee7394aea87415c73aaa26dd72 Mon Sep 17 00:00:00 2001 From: Joey Fisher Date: Fri, 9 Oct 2026 09:38:18 -0600 Subject: [PATCH] fix(agc): register the null pixel program with the inputs and user SGPRs its draws use Since #1178 the SPI_PS_INPUT_ENA/ADDR emplace calls did nothing, so the registered variant had no pixel inputs, and null pixel draws took the last pixel shader's user SGPR count and the bound targets' export mapping. Registration now overwrites those registers, the draw gives the null program no user SGPRs and an empty export mapping, and the at-draw preparation is left for wave32 only. Root cause found by @Raghav314. --- .../include/Driver/Shaders/ShaderRegistry.hpp | 5 +++ .../Execution/src/Driver/Draw/DrawDecode.cpp | 4 +- .../src/Driver/Shaders/ShaderRegistry.cpp | 42 ++++++++++--------- .../Graphics/src/ShaderInputState.cpp | 2 +- .../tests/execution/PreparedGraphics.cpp | 40 ++++++++++++++++++ 5 files changed, 71 insertions(+), 22 deletions(-) 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 f0a2e97bb..4339ca349 100644 --- a/core/libs/prx/libSceAgcDriver/Execution/include/Driver/Shaders/ShaderRegistry.hpp +++ b/core/libs/prx/libSceAgcDriver/Execution/include/Driver/Shaders/ShaderRegistry.hpp @@ -18,6 +18,10 @@ namespace ShaderRecompiler { class ShaderPreparationContext; } +namespace AgcDriver { +class VulkanDevice; +} + namespace AgcDriver::DriverDetail { struct ShaderSnapshot; @@ -78,6 +82,7 @@ std::shared_ptr ReadRawComputeShader(std::uint64_t address std::shared_ptr PrepareShaderWithDiagnostics(const ShaderRecompiler::RecompileRequest& request, ShaderRecompiler::ShaderPreparationContext* preparation = nullptr); std::uint64_t NullPixelProgramAddress(); +ShaderSnapshot PrepareNullPixelProgram(const VulkanDevice& device); std::optional RegisteredFloatMode(const ShaderSnapshot& snapshot); void PublishRegisteredShader(std::shared_ptr& registry, const std::shared_ptr& snapshot); diff --git a/core/libs/prx/libSceAgcDriver/Execution/src/Driver/Draw/DrawDecode.cpp b/core/libs/prx/libSceAgcDriver/Execution/src/Driver/Draw/DrawDecode.cpp index 91727f535..7a860207c 100644 --- a/core/libs/prx/libSceAgcDriver/Execution/src/Driver/Draw/DrawDecode.cpp +++ b/core/libs/prx/libSceAgcDriver/Execution/src/Driver/Draw/DrawDecode.cpp @@ -35,8 +35,8 @@ void DecodeGraphicsPrograms(DrawDecode& decoded, const QueueState& queue, const require(address - snapshot.codeAddress < snapshot.code.size() * sizeof(std::uint32_t), "graphics program is outside registered shader code"); require((address - snapshot.codeAddress) % sizeof(std::uint32_t) == 0, "graphics entry point is not dword aligned"); require(snapshot.type == type, "graphics program refers to an incompatible shader binary type"); - Graphics::NoteRegisterRead(Graphics::RegisterBank::Shader, rsrc2); - const auto resources = nullPixel && !queue.shader.contains(rsrc2) ? 0u : ReadGraphicsRegister(queue.shader, rsrc2); + if (!nullPixel) Graphics::NoteRegisterRead(Graphics::RegisterBank::Shader, rsrc2); + const auto resources = nullPixel ? 0u : ReadGraphicsRegister(queue.shader, rsrc2); const auto userCount = ((resources >> 1u) & 0x1fu) | (((resources >> 27u) & 1u) << 5u); require(userCount <= 32, "graphics user SGPR count exceeds the register bank"); const auto codeOffset = static_cast((address - snapshot.codeAddress) / sizeof(std::uint32_t)); 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 2438ab9bb..5aa48d967 100644 --- a/core/libs/prx/libSceAgcDriver/Execution/src/Driver/Shaders/ShaderRegistry.cpp +++ b/core/libs/prx/libSceAgcDriver/Execution/src/Driver/Shaders/ShaderRegistry.cpp @@ -182,8 +182,8 @@ void PublishRegisteredShader(std::shared_ptr& registry, const st namespace { bool PreparedAtUse(const ShaderSnapshot& snapshot, const ShaderRecompiler::RecompileRequest& request) { - if (snapshot.codeAddress == NullPixelProgramAddress() && request.shader.stage == ShaderRecompiler::ShaderStage::Fragment) { - APS5_LOG_ERR("The null pixel program has no artifact for this draw (wave %u, %zu user SGPRs); preparing it at draw", request.context.waveSize, request.context.userData.size()); + if (snapshot.codeAddress == NullPixelProgramAddress() && request.shader.stage == ShaderRecompiler::ShaderStage::Fragment && request.context.waveSize == 32u) { + APS5_LOG_ERR("The null pixel program has no wave%u artifact for this draw; preparing it at draw", request.context.waveSize); return true; } if (!snapshot.header.empty()) return snapshot.prepared->deferred; @@ -718,6 +718,26 @@ void ResolvePreparedGraphics(const ShaderSnapshot& front, const std::shared_ptr< transaction.Commit(); } +ShaderSnapshot PrepareNullPixelProgram(const VulkanDevice& device) { + ShaderSnapshot null{NullPixelProgramAddress(), reinterpret_cast(&NullPixelShader), NullPixelShader.type, {}, {}}; + null.code.assign(std::begin(NullPixelCode), std::end(NullPixelCode)); + null.header.resize(sizeof(Shader)); + std::memcpy(null.header.data(), &NullPixelShader, sizeof(Shader)); + auto nullRegisteredState = DecodeRegisteredState(null); + nullRegisteredState.shader.insert_or_assign(0x008u, static_cast(null.codeAddress >> 8u)); + nullRegisteredState.shader.insert_or_assign(0x009u, static_cast(null.codeAddress >> 40u)); + nullRegisteredState.shader.insert_or_assign(0x00bu, 0u); + nullRegisteredState.context.insert_or_assign(0x1b3u, 0x2u); + nullRegisteredState.context.insert_or_assign(0x1b4u, 0x2u); + null.registeredState = std::make_shared(std::move(nullRegisteredState)); + QueueState nullState{}; + nullState.shader = null.registeredState->shader; + nullState.context = null.registeredState->context; + nullState.userConfig = null.registeredState->userConfig; + null.prepared->entries = PrepareRegistered(null, device, nullState, true); + return null; +} + void Driver::RegisterShader(const Shader* shader) { PerformanceContext timingContext(FrameTiming::Preparation()); PerformanceTimer timing("Shader.Register"); @@ -789,23 +809,7 @@ void Driver::RegisterShader(const Shader* shader) { rethrowFailure(); PublishRegisteredShader(shaders, std::make_shared(std::move(snapshot))); if (shaders->find(NullPixelProgramAddress()) == shaders->end()) { - ShaderSnapshot null{NullPixelProgramAddress(), reinterpret_cast(&NullPixelShader), NullPixelShader.type, {}, {}}; - null.code.assign(std::begin(NullPixelCode), std::end(NullPixelCode)); - null.header.resize(sizeof(Shader)); - std::memcpy(null.header.data(), &NullPixelShader, sizeof(Shader)); - auto nullRegisteredState = DecodeRegisteredState(null); - nullRegisteredState.shader.emplace(0x008u, static_cast(null.codeAddress >> 8u)); - nullRegisteredState.shader.emplace(0x009u, static_cast(null.codeAddress >> 40u)); - nullRegisteredState.shader.emplace(0x00bu, 0u); - nullRegisteredState.context.emplace(0x1b3u, 0x2u); - nullRegisteredState.context.emplace(0x1b4u, 0x2u); - null.registeredState = std::make_shared(std::move(nullRegisteredState)); - QueueState nullState{}; - nullState.shader = null.registeredState->shader; - nullState.context = null.registeredState->context; - nullState.userConfig = null.registeredState->userConfig; - null.prepared->entries = PrepareRegistered(null, *localDevice, nullState, true); - PublishRegisteredShader(shaders, std::make_shared(std::move(null))); + PublishRegisteredShader(shaders, std::make_shared(PrepareNullPixelProgram(*localDevice))); } transaction.Commit(); } diff --git a/core/libs/prx/libSceAgcDriver/Graphics/src/ShaderInputState.cpp b/core/libs/prx/libSceAgcDriver/Graphics/src/ShaderInputState.cpp index 3048cb099..ad47fdec3 100644 --- a/core/libs/prx/libSceAgcDriver/Graphics/src/ShaderInputState.cpp +++ b/core/libs/prx/libSceAgcDriver/Graphics/src/ShaderInputState.cpp @@ -158,7 +158,7 @@ ShaderRecompiler::ShaderPixelStageInfo DecodePixelStageInfo(const Registers& con .conservativeZExport = static_cast(conservativeZExport), .orderedPixelShader = ((shaderControl >> 16u) & 0x1u) != 0, .targetOutputMode = targetOutputMode, - .targetExportMapping = exportMappings + .targetExportMapping = nullProgram ? std::array{} : exportMappings }; } diff --git a/core/libs/prx/libSceAgcDriver/tests/execution/PreparedGraphics.cpp b/core/libs/prx/libSceAgcDriver/tests/execution/PreparedGraphics.cpp index 52a89a691..eb1a511a1 100644 --- a/core/libs/prx/libSceAgcDriver/tests/execution/PreparedGraphics.cpp +++ b/core/libs/prx/libSceAgcDriver/tests/execution/PreparedGraphics.cpp @@ -5,6 +5,7 @@ #include "prx/libSceAgcDriver/Graphics/include/ShaderInputState.hpp" #include "Optimization/ResourceProgram.hpp" #include +#include #include #include #include @@ -247,6 +248,44 @@ void Check(AgcDriver::VulkanDevice& device, AgcDriver::Graphics::ShaderPath path Reject([&] { DecodeGraphicsPrograms(invalid, queue, registry, true, true); }, "reserved graphics program address"); } +void NullPixelMatchesRegistration(AgcDriver::VulkanDevice& device) { + using namespace AgcDriver::DriverDetail; + Fixture front; + front.Initialize(2u); + const auto null = std::make_shared(PrepareNullPixelProgram(device)); + const auto registered = null->prepared->entries; + Require(!registered.empty(), "the null pixel program was not prepared at registration"); + ShaderRegistry registry; + registry.emplace(front.snapshot->codeAddress, front.snapshot); + registry.emplace(null->codeAddress, null); + AgcDriver::QueueState queue{}; + queue.context[0x8e] = 0xfu; + queue.context[0x8f] = 0xfu; + front.Bind(queue, 0xc8u, 0x8bu); + alignas(8) const std::array merged{}; + const auto mergedAddress = reinterpret_cast(merged.data()); + queue.shader[0x82u] = static_cast(mergedAddress); + queue.shader[0x83u] = static_cast(mergedAddress >> 32u); + queue.shader[0x008u] = 0u; + queue.shader[0x009u] = 0u; + queue.shader[0x00bu] = 4u << 1u; + queue.context[0x1b3u] = 0x30u; + queue.context[0x1b4u] = 0x30u; + DrawDecode draw{}; + draw.state.stages.path = AgcDriver::Graphics::ShaderPath::Vertex; + draw.state.stages.vertexWaveSize = 32u; + draw.state.stages.fragmentWaveSize = 64u; + std::array mappings{}; + mappings.fill(0xe4u); + draw.pixel = AgcDriver::Graphics::DecodePixelStageInfo(queue.context, mappings, true); + DecodeGraphicsPrograms(draw, queue, registry, false, true); + const auto& fragment = draw.programs.back(); + Require(fragment.snapshot == null && fragment.userData.empty(), "a null pixel draw took the last pixel shader's user SGPRs"); + const auto stages = PrepareGraphicsStages(draw, device.Target()); + const auto& handle = stages.back().entry.handle; + Require(std::ranges::any_of(registered, [&](const auto& entry) { return entry.handle == handle; }), "a null pixel draw did not match the variant prepared at registration"); +} + } int main(int argc, char** argv) { @@ -255,6 +294,7 @@ int main(int argc, char** argv) { if (!device) return VulkanTestSkipped; Require(argc <= 2, "invalid test arguments"); const auto dump = argc == 2 ? std::filesystem::path(argv[1]) : std::filesystem::path{}; + NullPixelMatchesRegistration(*device); Check(*device, AgcDriver::Graphics::ShaderPath::Vertex, dump); Check(*device, AgcDriver::Graphics::ShaderPath::Geometry, dump); Check(*device, AgcDriver::Graphics::ShaderPath::Tessellation, dump);