From b37365d2dd0ab8ddd1fb23776c7be3388466afff Mon Sep 17 00:00:00 2001 From: Dean Galvin Date: Thu, 1 Oct 2026 21:19:10 +1300 Subject: [PATCH] fix(recompiler): zero the high half of 16-bit SDWA results written as a whole dword A 16-bit VOP1/VOP2 result in SDWA form with dst_sel:DWORD writes the whole register, so with dst_unused:UNUSED_PAD the high half is zero. write16Bits treated dst_sel:DWORD as the plain encoding and kept the old high half, which is right only for the non-SDWA form (GFX10 16-bit instructions preserve the high bits). The decoder already marks SDWA destinations with explicitSdwaDst; write16Bits now sends those through writeRawU32 like the other selectors. GFX10 compilers rely on this to zero-extend f16 results before packing two of them with an integer add, e.g. v_cvt_f16_f32_sdwa ... dst_sel:DWORD dst_unused:UNUSED_PAD followed by v_lshl_add_u32 v2, hi, 16, lo. PPSA21564's FidelityFX depth-of-field combine pass packs its near/far colours into LDS this way 60 times: whatever the low register held before leaked into the high half, so green (and alpha) of the packed pairs came back huge or infinite while red and blue stayed right. The bloom then spread the bad green over the intro cinematic. agc_driver_sdwa_destination_tests runs the padded, preserved and plain encodings and the packing idiom on the GPU against a model of the guest instructions. --- core/libs/prx/libSceAgcDriver/CMakeLists.txt | 12 ++ .../libSceAgcDriver/tests/SdwaDestination.cpp | 118 ++++++++++++++++++ .../src/TranslationContext/SubwordPacking.cpp | 2 +- 3 files changed, 131 insertions(+), 1 deletion(-) create mode 100644 core/libs/prx/libSceAgcDriver/tests/SdwaDestination.cpp diff --git a/core/libs/prx/libSceAgcDriver/CMakeLists.txt b/core/libs/prx/libSceAgcDriver/CMakeLists.txt index 556d4ae66..ca0b6a3e8 100644 --- a/core/libs/prx/libSceAgcDriver/CMakeLists.txt +++ b/core/libs/prx/libSceAgcDriver/CMakeLists.txt @@ -404,6 +404,18 @@ target_link_libraries(agc_driver_mix_precision_tests PRIVATE libSceAgcDriver SDL set_target_properties(agc_driver_mix_precision_tests PROPERTIES CXX_EXTENSIONS OFF) configure_windows_unwind(agc_driver_mix_precision_tests) +add_test_executable(agc_driver_sdwa_destination_tests EXCLUDE_FROM_ALL + tests/SdwaDestination.cpp +) +target_include_directories(agc_driver_sdwa_destination_tests PRIVATE + ${LIBS_INCLUDE_DIR} + ${CMAKE_SOURCE_DIR}/core/shader/recompiler + ${CMAKE_SOURCE_DIR}/3rdparty/Vulkan-Headers/include +) +target_link_libraries(agc_driver_sdwa_destination_tests PRIVATE libSceAgcDriver SDL2-static libc) +set_target_properties(agc_driver_sdwa_destination_tests PROPERTIES CXX_EXTENSIONS OFF) +configure_windows_unwind(agc_driver_sdwa_destination_tests) + if(BUILD_TESTING) foreach(agcTest IN ITEMS agc_driver_tests agc_shader_memory_tests agc_structurizer_tests agc_dpp_write_tests agc_shader_disk_cache_tests agc_shader_control_flow_tests agc_driver_flip_tests agc_driver_pm4_tests agc_driver_graphics_tests) string(REGEX REPLACE "_tests$" "" agcTestName "${agcTest}") diff --git a/core/libs/prx/libSceAgcDriver/tests/SdwaDestination.cpp b/core/libs/prx/libSceAgcDriver/tests/SdwaDestination.cpp new file mode 100644 index 000000000..355dfa2b1 --- /dev/null +++ b/core/libs/prx/libSceAgcDriver/tests/SdwaDestination.cpp @@ -0,0 +1,118 @@ +#include "prx/libSceAgcDriver/Execution/include/VulkanDevice.hpp" +#include "prx/libSceAgcDriver/Graphics/include/Draw.hpp" +#include "Recompiler.hpp" +#include +#include +#include +#include +#include +#include +#include + +namespace { + +using AgcDriver::Graphics::Require; +using ShaderRecompiler::ShaderStage; + +constexpr std::uint32_t Threads = 64; +constexpr std::uint32_t Inputs = 4; +constexpr std::uint32_t Results = 8; +constexpr std::uint32_t Garbage = 0xabcd1234u; +alignas(256) std::array Input{}; +alignas(256) std::array Output{}; + +alignas(256) constexpr std::array SdwaCode{ + 0x34020082, 0x34060083, 0xe0302000, 0x80000401, 0xe0302004, 0x80000501, 0xe0302008, 0x80000601, + 0x7e1402ff, 0xabcd1234, 0x7e1602ff, 0xabcd1234, 0x7e1802ff, 0xabcd1234, 0x7e1a02ff, 0xabcd1234, + 0x7e1c02ff, 0xabcd1234, 0x7e1e02ff, 0xabcd1234, 0xbf8c3f70, 0x7e1414f9, 0x00060604, 0x7e161504, + 0x7e1814f9, 0x00061505, 0x7e1a14f9, 0x00060604, 0x7e1c14f9, 0x00060605, 0xd746000d, 0x0435210e, + 0x641e0cf9, 0x05040606, 0xe0702000, 0x80010a03, 0xe0702004, 0x80010b03, 0xe0702008, 0x80010c03, + 0xe070200c, 0x80010d03, 0xe0702010, 0x80010f03, 0xbf810000, +}; + +float Quarter(std::uint32_t index) { + return static_cast(static_cast(index % 17u) - 8) / 4.0f; +} + +std::uint32_t HalfBits(float value) { + if (value == 0.0f) return std::signbit(value) ? 0x8000u : 0u; + int exponent = 0; + const float mantissa = std::frexp(std::fabs(value), &exponent); + const auto fraction = static_cast(std::ldexp(mantissa, 11)); + Require(exponent >= -13 && exponent <= 16 && std::ldexp(static_cast(fraction), exponent - 11) == std::fabs(value), "sdwa destination: a model value is not a normal f16"); + return (std::signbit(value) ? 0x8000u : 0u) | (static_cast(exponent + 14) << 10u) | (fraction & 0x3ffu); +} + +void FillInput() { + for (std::uint32_t tid = 0; tid < Threads; ++tid) { + auto* words = &Input[tid * Inputs]; + words[0] = std::bit_cast(Quarter(tid * 3u + 1u)); + words[1] = std::bit_cast(Quarter(tid * 5u + 2u)); + words[2] = HalfBits(Quarter(tid * 7u + 3u)) | (HalfBits(Quarter(tid * 11u + 4u)) << 16u); + } +} + +std::string Hex(std::uint32_t value) { + char text[16]; + std::snprintf(text, sizeof(text), "0x%08x", value); + return text; +} + +std::array BufferDescriptor(const void* data, std::uint32_t count) { + const auto address = reinterpret_cast(data); + return {static_cast(address), static_cast((address >> 32u) & 0xffffu) | (4u << 16u), count, 0x01016facu}; +} + +void Run(AgcDriver::VulkanDevice& device) { + Output.fill(0xdeadbeefu); + std::vector userData(8, 0u); + const auto input = BufferDescriptor(Input.data(), static_cast(Input.size())); + const auto output = BufferDescriptor(Output.data(), static_cast(Output.size())); + std::copy(input.begin(), input.end(), userData.begin()); + std::copy(output.begin(), output.end(), userData.begin() + 4); + const std::span code(SdwaCode); + const std::array memory{{{reinterpret_cast(code.data()), std::as_bytes(code)}}}; + const ShaderRecompiler::ShaderComputeStageInfo compute{{Threads, 1, 1}, 0, {false, false, false}, false, 1}; + ShaderRecompiler::RecompileRequest request{ + {ShaderStage::Compute, reinterpret_cast(code.data()), code, 0, {}}, + {32, 0, userData, compute, std::nullopt, std::nullopt, memory}, + device.Target(), + {0, 0, 0, 128} + }; + request.useCache = false; + const auto result = ShaderRecompiler::Recompile(request); + device.Dispatch(result, 1, 1, 1, {}, reinterpret_cast(code.data())); + device.WaitIdle(); +} + +void Check() { + for (std::uint32_t tid = 0; tid < Threads; ++tid) { + const auto* in = &Input[tid * Inputs]; + const auto* out = &Output[tid * Results]; + const auto a = HalfBits(std::bit_cast(in[0])); + const auto b = HalfBits(std::bit_cast(in[1])); + const auto sum = HalfBits(Quarter(tid * 7u + 3u) + Quarter(tid * 11u + 4u)); + const auto where = [&](std::uint32_t result) { return "sdwa destination: thread " + std::to_string(tid) + " result " + std::to_string(result) + " is " + Hex(out[result]); }; + Require(out[0] == a, where(0) + ", expected " + Hex(a) + ": v_cvt_f16_f32_sdwa dst_sel:DWORD dst_unused:UNUSED_PAD must zero the high half"); + Require(out[1] == ((Garbage & 0xffff0000u) | a), where(1) + ", expected " + Hex((Garbage & 0xffff0000u) | a) + ": v_cvt_f16_f32_e32 must keep the high half"); + Require(out[2] == ((b << 16u) | (Garbage & 0xffffu)), where(2) + ", expected " + Hex((b << 16u) | (Garbage & 0xffffu)) + ": dst_sel:WORD_1 dst_unused:UNUSED_PRESERVE must keep the low half"); + Require(out[3] == (a | (b << 16u)), where(3) + ", expected " + Hex(a | (b << 16u)) + ": two padded conversions packed with v_lshl_add_u32"); + Require(out[4] == sum, where(4) + ", expected " + Hex(sum) + ": v_add_f16_sdwa dst_sel:DWORD dst_unused:UNUSED_PAD must zero the high half"); + } +} + +} + +int main() { + try { + AgcDriver::VulkanDevice device; + FillInput(); + Run(device); + Check(); + std::puts("sdwa destination tests passed"); + return 0; + } catch (const std::exception& error) { + std::cerr << error.what() << '\n'; + return 1; + } +} diff --git a/core/shader/recompiler/Translation/src/TranslationContext/SubwordPacking.cpp b/core/shader/recompiler/Translation/src/TranslationContext/SubwordPacking.cpp index 4e0005569..b70730250 100644 --- a/core/shader/recompiler/Translation/src/TranslationContext/SubwordPacking.cpp +++ b/core/shader/recompiler/Translation/src/TranslationContext/SubwordPacking.cpp @@ -127,7 +127,7 @@ IrU32 TranslationContext::packHalf2x16(IrF32 low, IrF32 high) { } void TranslationContext::write16Bits(const RdnaOperand& operand, IrU32 value) { - if (operand.sdwaSel != 6u) { + if (operand.sdwaSel != 6u || operand.explicitSdwaDst) { writeRawU32(operand, value); return; }