From 1a5507698b7c083ecdaab93d6b19084a6db0f1de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Franch?= Date: Sun, 4 Oct 2026 08:33:11 +0200 Subject: [PATCH] fix(shader): choose v_div_scale_f32/v_div_fmas_f32 NaNs on the bits and refuse s_denorm_mode v_div_scale_f32 returns a NaN source unchanged instead of scaling it, and v_div_fmas_f32 quiets the first NaN input or returns the default NaN 0xffc00000, so results no longer depend on the driver's NaNs. The test compares NaN results bit for bit. s_denorm_mode throws: the recompiler does not pin a denormal mode, so no value matches what it models. Co-Authored-By: Claude --- .../tests/execution/Division.cpp | 20 ++++++------------- .../Translation/src/FloatInstructions.cpp | 14 +++++++++++-- .../Translation/src/ScalarInstructions.cpp | 4 +++- docs/dev/TechnicalDebt.md | 1 - 4 files changed, 21 insertions(+), 18 deletions(-) diff --git a/core/libs/prx/libSceAgcDriver/tests/execution/Division.cpp b/core/libs/prx/libSceAgcDriver/tests/execution/Division.cpp index ea91c11fc..001c96d22 100644 --- a/core/libs/prx/libSceAgcDriver/tests/execution/Division.cpp +++ b/core/libs/prx/libSceAgcDriver/tests/execution/Division.cpp @@ -30,8 +30,8 @@ alignas(256) constexpr std::array HelperCode{ alignas(256) constexpr std::array DivisionCode{ 0x34020082, 0xe0302000, 0x80000401, 0xe0302004, 0x80000501, 0xbf8c3f70, 0xd56d1408, 0x04120b05, - 0xd56d6a09, 0x04120b04, 0x7e145508, 0xbfa5000f, 0xd54b000b, 0x23ca1508, 0x5614150b, 0x10161509, - 0xd54b000c, 0x24261708, 0x5616150c, 0xd54b0008, 0x24261708, 0xbfa5000c, 0xd56f0008, 0x042e1508, + 0xd56d6a09, 0x04120b04, 0x7e145508, 0xbf800000, 0xd54b000b, 0x23ca1508, 0x5614150b, 0x10161509, + 0xd54b000c, 0x24261708, 0x5616150c, 0xd54b0008, 0x24261708, 0xbf800000, 0xd56f0008, 0x042e1508, 0xd55f000a, 0x04120b08, 0xe0702000, 0x80010a01, 0xbf810000, }; @@ -604,14 +604,6 @@ private: ShaderRecompiler::RecompileResult result; }; -bool IsNan(std::uint32_t bits) { - return (bits & 0x7fffffffu) > 0x7f800000u; -} - -bool Same(std::uint32_t actual, std::uint32_t expected) { - return actual == expected || (IsNan(actual) && IsNan(expected)); -} - std::string Hex(std::uint32_t value) { char text[16]; std::snprintf(text, sizeof(text), "0x%08x", value); @@ -643,10 +635,10 @@ void CheckHelpers(AgcDriver::VulkanDevice& device, bool fused) { const auto* out = &Output[lane * Stride]; const std::string where = "division helpers: vector " + std::to_string(first + lane) + " (" + Hex(vector.s0) + ", " + Hex(vector.s1) + ", " + Hex(vector.s2) + ")"; if (vector.s0 == vector.s1 || vector.s0 == vector.s2) { - Require(Same(out[0], vector.scale) && out[1] == vector.scaleVcc, where + ": v_div_scale_f32 is " + Hex(out[0]) + " vcc " + std::to_string(out[1]) + ", expected " + Hex(vector.scale) + " vcc " + std::to_string(vector.scaleVcc)); + Require(out[0] == vector.scale && out[1] == vector.scaleVcc, where + ": v_div_scale_f32 is " + Hex(out[0]) + " vcc " + std::to_string(out[1]) + ", expected " + Hex(vector.scale) + " vcc " + std::to_string(vector.scaleVcc)); } - Require(!fused || Same(out[2], vector.fmas), where + " vcc " + std::to_string(vector.vcc) + ": v_div_fmas_f32 is " + Hex(out[2]) + ", expected " + Hex(vector.fmas)); - Require(Same(out[3], vector.fixup), where + ": v_div_fixup_f32 is " + Hex(out[3]) + ", expected " + Hex(vector.fixup)); + Require(!fused || out[2] == vector.fmas, where + " vcc " + std::to_string(vector.vcc) + ": v_div_fmas_f32 is " + Hex(out[2]) + ", expected " + Hex(vector.fmas)); + Require(out[3] == vector.fixup, where + ": v_div_fixup_f32 is " + Hex(out[3]) + ", expected " + Hex(vector.fixup)); } } } @@ -664,7 +656,7 @@ void CheckDivision(AgcDriver::VulkanDevice& device) { for (std::uint32_t lane = 0; lane < Threads && first + lane < count; ++lane) { const auto& vector = DivisionVectors[first + lane]; const auto actual = Output[lane * Stride]; - Require(Same(actual, vector.quotient), "division macro: " + Hex(vector.numerator) + " / " + Hex(vector.denominator) + " is " + Hex(actual) + ", expected " + Hex(vector.quotient)); + Require(actual == vector.quotient, "division macro: " + Hex(vector.numerator) + " / " + Hex(vector.denominator) + " is " + Hex(actual) + ", expected " + Hex(vector.quotient)); } } } diff --git a/core/shader/recompiler/Translation/src/FloatInstructions.cpp b/core/shader/recompiler/Translation/src/FloatInstructions.cpp index 94dabbf23..4307ca9c9 100644 --- a/core/shader/recompiler/Translation/src/FloatInstructions.cpp +++ b/core/shader/recompiler/Translation/src/FloatInstructions.cpp @@ -287,7 +287,10 @@ bool TranslationContext::vDivScaleF32(const RdnaInstruction& inst) { ir.LogicalOr(ir.LogicalNot(nearMin.Value()), isDenominator.Value()))); const IrF32 up(ir.Emit(IrOpcode::FPMul32, IrType::F32, {&value.Value(), &ir.ConstantF32(18446744073709551616.0f)})); const IrF32 down(ir.Emit(IrOpcode::FPMul32, IrType::F32, {&value.Value(), &ir.ConstantF32(5.42101086242752217e-20f)})); - const IrU32 scaled(ir.Select(scaleUp.Value(), ir.BitCastU32(up.Value()), ir.Select(scaleDown.Value(), ir.BitCastU32(down.Value()), bits.Value()))); + const IrU1 valueNan(ir.UGreaterThan(ir.BitwiseAnd(bits.Value(), ir.Constant(0x7fffffffu)), ir.Constant(0x7f800000u))); + const IrU1 scaleUpNumber(ir.LogicalAnd(scaleUp.Value(), ir.LogicalNot(valueNan.Value()))); + const IrU1 scaleDownNumber(ir.LogicalAnd(scaleDown.Value(), ir.LogicalNot(valueNan.Value()))); + const IrU32 scaled(ir.Select(scaleUpNumber.Value(), ir.BitCastU32(up.Value()), ir.Select(scaleDownNumber.Value(), ir.BitCastU32(down.Value()), bits.Value()))); const auto isZero = [&](IrU32 word) { return IrU1(ir.IEqual(ir.BitwiseAnd(word.Value(), ir.Constant(0x7fffffffu)), ir.Constant(0u))); }; const IrU1 zero(ir.LogicalOr(isZero(denominator).Value(), isZero(numerator).Value())); const IrU32 result(ir.Select(zero.Value(), ir.Constant(0xffc00000u), scaled.Value())); @@ -322,7 +325,14 @@ bool TranslationContext::vDivFmasF32(const RdnaInstruction& inst) { &ir.Emit(IrOpcode::FPMul32, IrType::F32, {addend, &power})}); IrValue& afterwards = ir.Emit(IrOpcode::FPMul32, IrType::F32, {&plain, &power}); IrValue& scaledResult = ir.Emit(IrOpcode::SelectF32, IrType::F32, {&rescale.Value(), &rescaled, &afterwards}); - writeOperand(inst.destination, &ir.Emit(IrOpcode::SelectF32, IrType::F32, {&scale.Value(), &scaledResult, &plain})); + const auto isNan = [&](IrValue& value) { return IrU1(ir.UGreaterThan(ir.BitwiseAnd(ir.BitCastU32(value), ir.Constant(0x7fffffffu)), ir.Constant(0x7f800000u))); }; + const auto quiet = [&](IrValue& value) { return IrU32(ir.BitwiseOr(ir.BitCastU32(value), ir.Constant(0x400000u))); }; + IrValue& computed = ir.Emit(IrOpcode::SelectF32, IrType::F32, {&scale.Value(), &scaledResult, &plain}); + IrU32 result(ir.Select(isNan(computed).Value(), ir.Constant(0xffc00000u), ir.BitCastU32(computed))); + result = IrU32(ir.Select(isNan(*addend).Value(), quiet(*addend).Value(), result.Value())); + result = IrU32(ir.Select(isNan(*rhs).Value(), quiet(*rhs).Value(), result.Value())); + result = IrU32(ir.Select(isNan(*lhs).Value(), quiet(*lhs).Value(), result.Value())); + writeOperand(inst.destination, &ir.BitCastF32(result.Value())); return true; } diff --git a/core/shader/recompiler/Translation/src/ScalarInstructions.cpp b/core/shader/recompiler/Translation/src/ScalarInstructions.cpp index d4eba2f76..28a9f089d 100644 --- a/core/shader/recompiler/Translation/src/ScalarInstructions.cpp +++ b/core/shader/recompiler/Translation/src/ScalarInstructions.cpp @@ -1,6 +1,7 @@ #include "Translation/ScalarInstructions.hpp" #include "Translation/TranslationContext.hpp" #include +#include namespace ShaderRecompiler { @@ -377,9 +378,10 @@ bool TranslationContext::emitScalar(const RdnaInstruction& inst) { case RdnaOpcode::SIcacheInv: case RdnaOpcode::SIncperflevel: case RdnaOpcode::SDecperflevel: - case RdnaOpcode::SDenormMode: emitControlNop(); return true; + case RdnaOpcode::SDenormMode: + throw std::runtime_error("s_denorm_mode " + std::to_string(inst.source0.value) + " at pc " + std::to_string(inst.programCounter) + ": the recompiler does not model denormal modes"); case RdnaOpcode::SWaitcntDepctr: case RdnaOpcode::SWaitIdle: emitWaitcnt(); diff --git a/docs/dev/TechnicalDebt.md b/docs/dev/TechnicalDebt.md index 1458600bd..a66170e1c 100644 --- a/docs/dev/TechnicalDebt.md +++ b/docs/dev/TechnicalDebt.md @@ -19,7 +19,6 @@ Throughout the project, every function at every stage either **does exactly what - The shader recompiler [skips baryctric coordinates](../../core/shader/recompiler/Recompiler.cpp) (is not even passed to SpirvTargetOptions at row 212). - [libSceAvPlayer](../../core/libs/prx/libSceAvPlayer/Export.cpp): `sceAvPlayerSetLogCallback` accepts a callback that is never called, as the player produces no log messages, and `sceAvPlayerSetAvailableBandwidth` has no effect, as it governs HLS sources, which `sceAvPlayerAddSource` does not implement. - [sceAvPlayerSetTrickSpeed](../../core/libs/prx/libSceAvPlayer/src/Source.cpp) (libSceAvPlayer) with a negative speed runs the clock backwards but delivers no frames; when a forward speed is set again, playback resumes from the rewound time. -- The shader recompiler [ignores `s_denorm_mode`](../../core/shader/recompiler/Translation/src/ScalarInstructions.cpp): denormals follow the Vulkan driver's float controls, not the mode the shader sets (the f32 division macro switches it around its Newton-Raphson steps). ### Unknown function info