From bdefb9faf1d199a5ef7a09a169d6e1a8ab536252 Mon Sep 17 00:00:00 2001 From: Brian Degenhardt Date: Sun, 19 Jul 2026 19:07:31 -0700 Subject: [PATCH] =?UTF-8?q?EE=20FPU:=20clamp=20MAX.S/MIN.S=20operands=20to?= =?UTF-8?q?=20=C2=B1fMax=20at=20eeClampMode>=3D1?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit recMAX_S_xmm/recMIN_S_xmm emitted bare Fmaxnm/Fminnm with no operand clamp. x86 routes MAX/MIN through recCommutativeOp(op>=2), whose gate `CHECK_FPU_EXTRA_OVERFLOW || (op>=2)` always fires, so it fpuFloat2-clamps both operands (sign-preserving inf/NaN -> ±fMax) whenever CHECK_FPU_OVERFLOW — eeClampMode >= 1, a strictly lower threshold than the arithmetic ops' >= 2. AetherSX2's shipped arm64 rec gates the same MAX/MIN clamp on fpuOverflow (options bit 8) vs ADD/SUB's bit 8+9 (verified against the 3606 build disasm). Without the clamp a raw Inf/NaN operand (via MOV.S/LWC1/MTC1) survives the NaN-eating Fmaxnm/Fminnm as the wrong finite value — the True Crime: New York City rainbow (BlueTongue engine, SLUS-21106, eeClampMode:2), a min(max(uv,0),size) UV-clamp idiom producing a corrupt palette index. Add fpuClampMinMaxOperand (mirror of fpuClampInput, gated CHECK_FPU_OVERFLOW instead of CHECK_FPU_EXTRA_OVERFLOW) and apply it to both operands. Pinned by EeRecFpu.Max/MinSClamps* (mode-2 Inf/NaN cases + a mode-1 gate test + a mode-0 no-clamp lower bound). Interp fp_max/fp_min run on raw bits with no clamp, so the correct JIT diverges from interp here — the tests use RunJitNoDiff/GetFprBitsJit (x86 JIT is the FPU-clamp oracle, not interp). Co-Authored-By: Claude --- pcsx2/arm64/iFPU-arm64.cpp | 38 ++++++- .../core/recompilers/ee_rec_fpu_tests.cpp | 107 ++++++++++++++++++ 2 files changed, 140 insertions(+), 5 deletions(-) diff --git a/pcsx2/arm64/iFPU-arm64.cpp b/pcsx2/arm64/iFPU-arm64.cpp index e8d5942e70..255c75c0fc 100644 --- a/pcsx2/arm64/iFPU-arm64.cpp +++ b/pcsx2/arm64/iFPU-arm64.cpp @@ -419,6 +419,26 @@ static a64::VRegister fpuClampInput(const a64::VRegister& src, const a64::VRegis return scratch; } +// Source-operand clamp for MAX.S / MIN.S. Identical sign-preserving inf/NaN -> +// ±fMax to fpuClampInput, but gated on CHECK_FPU_OVERFLOW (eeClampMode >= 1) +// instead of CHECK_FPU_EXTRA_OVERFLOW (>= 2). x86 routes MAX/MIN through +// recCommutativeOp with op>=2, whose gate `CHECK_FPU_EXTRA_OVERFLOW || (op>=2)` +// is always true, so it fpuFloat2-clamps both operands whenever CHECK_FPU_OVERFLOW +// — a strictly lower threshold than the arithmetic ops. AetherSX2's shipped arm64 +// rec gates the same MAX/MIN clamp on fpuOverflow (options bit 8) vs ADD/SUB's +// bit 8+9 (verified by disassembly of the 3606 build). Without it a raw Inf/NaN +// operand (via MOV.S/LWC1/MTC1) survives the NaN-eating Fmaxnm/Fminnm as the +// wrong finite value — the True Crime: New York City rainbow. Off (mode 0) +// returns the source reg and emits nothing. +static a64::VRegister fpuClampMinMaxOperand(const a64::VRegister& src, const a64::VRegister& scratch) +{ + if (!CHECK_FPU_OVERFLOW) + return src; + armAsm->Fmov(scratch, src); + fpuClampCompareOperand(scratch); + return scratch; +} + // PS2 add/sub guard-bit emulation for the single-precision fast path. // // A compliant IEEE FPU keeps "guard" bits to the right of the mantissa during @@ -1080,12 +1100,18 @@ void recRSQRT_S() XMMINFO_WRITED | XMMINFO_READS | XMMINFO_READT); } -// PS2 FPU has no NaN concept — match x86 MAXSS/MINSS NaN-eating semantics -// with Fmaxnm/Fminnm (Fmax/Fmin IEEE-propagate NaN, same trap as mVUclamp1). -// No clamp needed: MAX/MIN cannot widen finite inputs. +// PS2 FPU has no NaN concept. At eeClampMode >= 1 the operands are first +// clamped to ±fMax (sign-preserving, via fpuClampMinMaxOperand — matching x86 +// recCommutativeOp's always-firing op>=2 clamp and AetherSX2's arm64 rec), so a +// raw Inf/NaN operand cannot reach the max/min. The result never needs clamping +// (max/min of two finite ±fMax-bounded inputs stays in range). Fmaxnm/Fminnm +// (not Fmax/Fmin, which IEEE-propagate NaN) give MAXSS/MINSS NaN-eating for the +// unclamped mode-0 case. static void recMAX_S_xmm(int info) { - armAsm->Fmaxnm(armSRegister(EEREC_D), armSRegister(EEREC_S), armSRegister(EEREC_T)); + const a64::VRegister s = fpuClampMinMaxOperand(armSRegister(EEREC_S), RSSCRATCH); + const a64::VRegister t = fpuClampMinMaxOperand(armSRegister(EEREC_T), RSSCRATCH2); + armAsm->Fmaxnm(armSRegister(EEREC_D), s, t); } void recMAX_S() @@ -1096,7 +1122,9 @@ void recMAX_S() static void recMIN_S_xmm(int info) { - armAsm->Fminnm(armSRegister(EEREC_D), armSRegister(EEREC_S), armSRegister(EEREC_T)); + const a64::VRegister s = fpuClampMinMaxOperand(armSRegister(EEREC_S), RSSCRATCH); + const a64::VRegister t = fpuClampMinMaxOperand(armSRegister(EEREC_T), RSSCRATCH2); + armAsm->Fminnm(armSRegister(EEREC_D), s, t); } void recMIN_S() diff --git a/tests/ctest/core/recompilers/ee_rec_fpu_tests.cpp b/tests/ctest/core/recompilers/ee_rec_fpu_tests.cpp index dc7a8f7088..754a01896a 100644 --- a/tests/ctest/core/recompilers/ee_rec_fpu_tests.cpp +++ b/tests/ctest/core/recompilers/ee_rec_fpu_tests.cpp @@ -37,6 +37,26 @@ struct FpuExtraOverflowGuard ~FpuExtraOverflowGuard() { EmuConfig.Cpu.Recompiler.fpuExtraOverflow = saved; } }; +// Scoped eeClampMode selector. Sets the two Recompiler bits SetEEClampMode +// derives from the mode: fpuOverflow (== CHECK_FPU_OVERFLOW, mode >= 1) gates +// the result clamp and the MAX/MIN operand clamp; fpuExtraOverflow (mode >= 2) +// gates the arithmetic operand clamp. Restores both on scope exit. +struct FpuClampModeGuard +{ + bool savedO = EmuConfig.Cpu.Recompiler.fpuOverflow; + bool savedX = EmuConfig.Cpu.Recompiler.fpuExtraOverflow; + explicit FpuClampModeGuard(int mode) + { + EmuConfig.Cpu.Recompiler.fpuOverflow = (mode >= 1); + EmuConfig.Cpu.Recompiler.fpuExtraOverflow = (mode >= 2); + } + ~FpuClampModeGuard() + { + EmuConfig.Cpu.Recompiler.fpuOverflow = savedO; + EmuConfig.Cpu.Recompiler.fpuExtraOverflow = savedX; + } +}; + u32 FloatBits(float f) { u32 bits; @@ -860,6 +880,93 @@ TEST(EeRecFpu, SqrtSNegativeArgumentReturnsAbsRoot) h.ExpectFpr(3, FloatBits(5.0f)); } +// ----- MAX.S / MIN.S operand clamp (eeClampMode >= 1) ---------------------- +// +// x86 recCommutativeOp clamps MAX/MIN operands (sign-preserving inf/NaN -> +// ±fMax via fpuFloat2) whenever CHECK_FPU_OVERFLOW — the op>=2 argument makes +// the gate always fire, so mode >= 1, a strictly LOWER threshold than ADD/SUB's +// mode >= 2 operand clamp. AetherSX2's shipped arm64 rec gates the identical +// clamp on fpuOverflow (options bit 8), confirmed by disassembly. Our port +// emitted bare Fmaxnm/Fminnm with no operand clamp, so a raw Inf/NaN FPR +// (reachable via MOV.S/LWC1/MTC1) survived as the wrong finite value — Fmaxnm/ +// Fminnm are NaN-eating and return the *other* operand — which is the True +// Crime: New York City rainbow (a min(max(uv,0),size) UV-clamp idiom feeding a +// corrupt palette index). These pin the x86/AetherSX2 behavior. +// +// The interpreter's fp_max/fp_min run on raw bits with NO operand clamp, so the +// (correct) JIT legitimately diverges from interp here — interp is not the +// FPU-clamp oracle, the x86 JIT is. Hence RunJitNoDiff + GetFprBitsJit rather +// than the auto-diffing Run()/ExpectFpr. +TEST(EeRecFpu, MaxSClampsInfOperandAtClampMode2) +{ + FpuClampModeGuard guard(2); + EeRecTestHarness h; + h.EnableCop1(); + h.SetFprBits(1, 0x7F800000u); // +Inf raw bits (poisoned fpr) + h.SetFpr(2, 3.0f); + h.LoadProgram({ee::MAX_S(3, 1, 2)}); + h.RunJitNoDiff(); + // clamp(+Inf) = +fMax, MAX(+fMax, 3) = +fMax. Bare Fmaxnm gives +Inf. + EXPECT_EQ(h.GetFprBitsJit(3), 0x7F7FFFFFu); +} + +TEST(EeRecFpu, MaxSClampsNanOperandToPosFmax) +{ + FpuClampModeGuard guard(2); + EeRecTestHarness h; + h.EnableCop1(); + h.SetFprBits(1, 0x7FC00000u); // +NaN raw bits + h.SetFpr(2, -5.0f); + h.LoadProgram({ee::MAX_S(3, 1, 2)}); + h.RunJitNoDiff(); + // clamp(+NaN) = +fMax, MAX(+fMax, -5) = +fMax. Bare Fmaxnm NaN-eats -> -5.0. + EXPECT_EQ(h.GetFprBitsJit(3), 0x7F7FFFFFu); +} + +TEST(EeRecFpu, MinSClampsNegNanOperandToNegFmax) +{ + FpuClampModeGuard guard(2); + EeRecTestHarness h; + h.EnableCop1(); + h.SetFprBits(1, 0xFFC00000u); // -NaN raw bits + h.SetFpr(2, 5.0f); + h.LoadProgram({ee::MIN_S(3, 1, 2)}); + h.RunJitNoDiff(); + // clamp(-NaN) = -fMax, MIN(-fMax, 5) = -fMax. Bare Fminnm NaN-eats -> 5.0. + EXPECT_EQ(h.GetFprBitsJit(3), 0xFF7FFFFFu); +} + +// The gate is fpuOverflow (mode >= 1), NOT fpuExtraOverflow (mode >= 2): the +// clamp must still fire at mode 1. A "fix" that reused fpuClampInput (which is +// gated on mode >= 2) would pass the mode-2 tests above but fail this one. +TEST(EeRecFpu, MaxSClampsInfOperandAtClampMode1) +{ + FpuClampModeGuard guard(1); + EeRecTestHarness h; + h.EnableCop1(); + h.SetFprBits(1, 0x7F800000u); // +Inf + h.SetFpr(2, 3.0f); + h.LoadProgram({ee::MAX_S(3, 1, 2)}); + h.RunJitNoDiff(); + EXPECT_EQ(h.GetFprBitsJit(3), 0x7F7FFFFFu); +} + +// Lower bound: at mode 0 x86 skips the operand clamp (fpuFloat2 is a no-op when +// !CHECK_FPU_OVERFLOW), so +Inf passes through unclamped. Guards against the fix +// over-clamping (making the clamp unconditional). +Inf, not NaN, is used here: +// mode-0 MAXSS-vs-Fmaxnm NaN handling is a separate pre-existing divergence. +TEST(EeRecFpu, MaxSDoesNotClampAtClampMode0) +{ + FpuClampModeGuard guard(0); + EeRecTestHarness h; + h.EnableCop1(); + h.SetFprBits(1, 0x7F800000u); // +Inf + h.SetFpr(2, 3.0f); + h.LoadProgram({ee::MAX_S(3, 1, 2)}); + h.RunJitNoDiff(); + EXPECT_EQ(h.GetFprBitsJit(3), 0x7F800000u); // +Inf, unclamped +} + // =========================================================================== // Allocator-state interaction patterns — consecutive ops sharing operands. // The allocator path keeps operands in NEON across opcodes; an aliasing or