armsx/ARMSX2
0
0
Fork 0
mirror of https://github.com/ARMSX2/ARMSX2.git synced 2026-08-24 16:50:16 -07:00
Code Issues Packages Projects Releases Wiki Activity

ee/fpu: make add/sub guard-bit emulation unconditional

Browse Source 
The fpuGuardedAddSub Recompiler option (and its GameDB
clampModes.guardedAddSub override) is gone; both JITs now always mask
the smaller-exponent operand's guard bits on the single-precision
ADD/SUB fast path, matching the Full-mode DOUBLE path's unconditional
guard. Games like True Crime NYC and Jak 3 misrender without the
masking, and per-game flagging proved impractical to maintain — the
failures take cross-build diffing to even attribute. The |exp diff|<=1
early-out keeps the common case at a plain op.

Test-side, EnableFpuGuarded() is removed (the masked behavior is now
the default the guard-bit suite pins directly) and the
DefaultOffEmitsPlainOpMatchingInterp test goes away with the option.
recompiler_tests: 1389/1389 green.

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
Brian Degenhardt
2026-07-20 00:22:24 -07:00
co-authored by Claude
parent ea62853294
commit 37bc5e1646
11 changed files with 19 additions and 107 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pcsx2-eerunner/Main.cpp
+5 -5
View File
@@ -548,7 +548,7 @@ static void PrintCommandLineHelp(const char* progname)
std::fprintf(stderr, " cold disc boot) and write a savestate. Bootstrap twindiff states on the WORKING\n");
std::fprintf(stderr, " binary, positioned BEFORE the corrupting computation (e.g. before a level load).\n");
std::fprintf(stderr, " --set Section/Key=Value: base-layer settings override, applied after the harness pinning\n");
std::fprintf(stderr, " (repeatable). E.g. --set EmuCore/CPU/Recompiler/fpuGuardedAddSub=true. GameDB\n");
std::fprintf(stderr, " (repeatable). E.g. --set EmuCore/CPU/Recompiler/fpuFullMode=true. GameDB\n");
std::fprintf(stderr, " per-game keys still win; the twin passes own the EnableEE/VU0/VU1/IOP toggles.\n");
std::fprintf(stderr, " -help: Displays this information and exits.\n");
std::fprintf(stderr, " -version: Displays version information and exits.\n");
@@ -700,7 +700,7 @@ bool EERunner::ParseCommandLineArgs(int argc, char* argv[], VMBootParameters& pa
const size_t slash = (eq == std::string::npos) ? std::string::npos : kv.rfind('/', eq);
if (eq == std::string::npos || slash == std::string::npos || slash == 0 || slash + 1 >= eq)
{
Console.Error("--set expects Section/Key=Value (e.g. EmuCore/CPU/Recompiler/fpuGuardedAddSub=true).");
Console.Error("--set expects Section/Key=Value (e.g. EmuCore/CPU/Recompiler/fpuFullMode=true).");
return false;
}
s_set_overrides.push_back({kv.substr(0, slash), kv.substr(slash + 1, eq - slash - 1), kv.substr(eq + 1)});
@@ -2853,9 +2853,9 @@ static int RunMkState()
// --dump-config <path> : boot the VM far enough for GameDB to apply, then
// serialize the EFFECTIVE EmuConfig to an INI. The twindiff driver diffs the
// two trees' dumps FIRST — config/GameDB drift (True Crime: fpuGuardedAddSub
// default-off + three divergent GameIndex copies) explains a broken game far
// more cheaply than any trace. Savestate optional: config keys off the disc
// two trees' dumps FIRST — config/GameDB drift (True Crime: guard-bit emulation
// default-off at the time + three divergent GameIndex copies) explains a broken
// game far more cheaply than any trace. Savestate optional: config keys off the disc
// serial, known right after Initialize.
static int RunDumpConfig()
{
pcsx2/Config.h
+1 -3
View File
@@ -674,8 +674,7 @@ struct Pcsx2Config
bool
fpuOverflow : 1,
fpuExtraOverflow : 1,
fpuFullMode : 1,
fpuGuardedAddSub : 1; // EE FPU add/sub guard-bit emulation (single-precision fast path). Off by default; opt-in per-game via GameDB (clampModes.guardedAddSub) or globally via INI. Independent of the clamp tiers — Full mode does its own guard on the DOUBLE path.
fpuFullMode : 1;
bool
EnableEECache : 1;
@@ -1559,7 +1558,6 @@ namespace EmuFolders
#define CHECK_FPU_EXTRA_OVERFLOW (EmuConfig.Cpu.Recompiler.fpuExtraOverflow) // If enabled, Operands are checked for infinities before being used in the FPU recs
#define CHECK_FPU_EXTRA_FLAGS 1 // Always enabled now // Sets D/I flags on FPU instructions
#define CHECK_FPU_FULL (EmuConfig.Cpu.Recompiler.fpuFullMode)
#define CHECK_FPU_GUARDED (EmuConfig.Cpu.Recompiler.fpuGuardedAddSub) // If enabled, add/sub emulate the PS2 FPU's missing mantissa guard bits (extra accuracy, slower). Off by default.
//------------ EE Recompiler defines - Comment to disable a recompiler ---------------
pcsx2/GameDatabase.cpp
-17
View File
@@ -188,12 +188,6 @@ void GameDatabase::parseAndInsert(const std::string_view serial, const ryml::Nod
node["clampModes"]["vu1ClampMode"] >> vuVal;
gameEntry.vu1ClampMode = static_cast<GameDatabaseSchema::ClampMode>(vuVal);
}
if (node["clampModes"].has_child("guardedAddSub"))
{
bool guardVal = false;
node["clampModes"]["guardedAddSub"] >> guardVal;
gameEntry.guardedAddSub = guardVal;
}
}
// Validate game fixes, invalid ones will be dropped!
@@ -516,17 +510,6 @@ void GameDatabaseSchema::GameEntry::applyGameFixes(Pcsx2Config& config, bool app
Console.Warning("GameDB: Skipping changing EE/FPU clamp mode [mode=%d]", clampMode);
}
if (guardedAddSub.has_value())
{
if (applyAuto)
{
Console.WriteLn("GameDB: Changing EE/FPU add/sub guard-bit emulation [enabled=%d]", static_cast<int>(*guardedAddSub));
config.Cpu.Recompiler.fpuGuardedAddSub = *guardedAddSub;
}
else
Console.Warning("GameDB: Skipping changing EE/FPU add/sub guard-bit emulation [enabled=%d]", static_cast<int>(*guardedAddSub));
}
if (vu0ClampMode != GameDatabaseSchema::ClampMode::Undefined)
{
const int clampMode = enum_cast(vu0ClampMode);
pcsx2/GameDatabase.h
-1
View File
@@ -102,7 +102,6 @@ namespace GameDatabaseSchema
ClampMode eeClampMode = ClampMode::Undefined;
ClampMode vu0ClampMode = ClampMode::Undefined;
ClampMode vu1ClampMode = ClampMode::Undefined;
std::optional<bool> guardedAddSub; // EE FPU add/sub guard-bit emulation override (unset = leave global default).
std::vector<GamefixId> gameFixes;
std::vector<std::pair<SpeedHack, int>> speedHacks;
std::vector<std::pair<GSHWFixId, s32>> gsHWFixes;
pcsx2/Pcsx2Config.cpp
-2
View File
@@ -473,7 +473,6 @@ Pcsx2Config::RecompilerOptions::RecompilerOptions()
fpuOverflow = true;
//fpuExtraOverflow = false;
//fpuFullMode = false;
//fpuGuardedAddSub = false; // add/sub guard-bit emulation is opt-in (perf); games needing it enable it via GameDB clampModes.guardedAddSub.
}
void Pcsx2Config::RecompilerOptions::ApplySanityCheck()
@@ -552,7 +551,6 @@ void Pcsx2Config::RecompilerOptions::LoadSave(SettingsWrapper& wrap)
SettingsWrapBitBool(fpuOverflow);
SettingsWrapBitBool(fpuExtraOverflow);
SettingsWrapBitBool(fpuFullMode);
SettingsWrapBitBool(fpuGuardedAddSub);
}
u32 Pcsx2Config::RecompilerOptions::GetEEClampMode() const
pcsx2/arm64/iFPU-arm64.cpp
+5 -23
View File
@@ -447,11 +447,11 @@ static a64::VRegister fpuClampMinMaxOperand(const a64::VRegister& src, const a64
// positions must read as zero on hardware. This masks the low mantissa bits of
// the smaller-exponent operand by the exponent difference, then does the single
// op. It is the arm64 fast-path port of x86 FPU_ADD_SUB (iFPU.cpp:402). Both
// JITs gate this masking on the same off-by-default CHECK_FPU_GUARDED option
// (x86 FPU_ADD/FPU_SUB, iFPU.cpp) — see the early-out below. It reproduces the
// masking already present in the DOUBLE path's FPU_ADD_SUB (iFPUd-arm64.cpp:200);
// the CHECK_FPU_FULL (double) config dispatches to that path instead and never
// reaches here (Full mode guards unconditionally on both arches).
// JITs apply this masking unconditionally (x86 FPU_ADD/FPU_SUB, iFPU.cpp) —
// games like True Crime NYC and Jak 3 misrender without it, and flagging them
// per-game proved impractical. It reproduces the masking already present in the
// DOUBLE path's FPU_ADD_SUB (iFPUd-arm64.cpp:200); the CHECK_FPU_FULL (double)
// config dispatches to that path instead and never reaches here.
//
// When |expd - expt| <= 1 the mask clears zero bits, so that (common) case skips
// straight to the plain op. Only |diff| >= 2 masks the smaller-exponent operand;
@@ -475,24 +475,6 @@ static a64::VRegister fpuClampMinMaxOperand(const a64::VRegister& src, const a64
static void fpuEmitGuardedAddSub(const a64::VRegister& dst,
const a64::VRegister& s, const a64::VRegister& t, bool issub)
{
// Guard-bit emulation is opt-in via the off-by-default fpuGuardedAddSub
// Recompiler option (per-game GameDB clampModes.guardedAddSub, or global
// INI). Off = a plain single op, matching AetherSX2 / PCSX2 v1.0 and the
// x86 FPU_ADD/FPU_SUB guard-off branch (iFPU.cpp). This is the
// default path — the majority of titles never need guard-bit accuracy and
// skip the ~13-insn common-path cost. Returns before the NEON-temp alloc and
// GPR-scratch use below so nothing is booked on the fast path. (Full clamp
// mode is unaffected: it runs the DOUBLE path, which masks guard bits itself
// — iFPUd-arm64.cpp.)
if (!CHECK_FPU_GUARDED)
{
if (issub)
armAsm->Fsub(dst, s, t);
else
armAsm->Fadd(dst, s, t);
return;
}
// Alloc the NEON temp FIRST, before any raw GPR scratch below goes live.
// The alloc can emit a victim eviction whose address materialization uses
// scratch (today only x16/x17 via armMoveAddressToReg); keeping w9/w10
pcsx2/x86/iFPU.cpp
+4 -7
View File
@@ -62,11 +62,8 @@ namespace DOUBLE
#define FPUflagSO 0x00000010
#define FPUflagSU 0x00000008
// Add/Sub guard-bit emulation (matching the PS2's missing mantissa guard bits)
// is now gated at runtime on CHECK_FPU_GUARDED (the off-by-default
// fpuGuardedAddSub Recompiler option) rather than a compile-time constant — so
// both JITs honor the same setting. The Full-mode DOUBLE path keeps its own
// unconditional guard (iFPUd.cpp).
// Add/Sub opcodes produce the same results as the ps2
#define FPU_CORRECT_ADD_SUB 1
alignas(16) static const u32 s_neg[4] = {0x80000000, 0xffffffff, 0xffffffff, 0xffffffff};
alignas(16) static const u32 s_pos[4] = {0x7fffffff, 0xffffffff, 0xffffffff, 0xffffffff};
@@ -484,7 +481,7 @@ void FPU_ADD_SUB(int regd, int regt, int issub)
void FPU_ADD(int regd, int regt)
{
if (CHECK_FPU_GUARDED)
if (FPU_CORRECT_ADD_SUB)
FPU_ADD_SUB(regd, regt, 0);
else
xADD.SS(xRegisterSSE(regd), xRegisterSSE(regt));
@@ -492,7 +489,7 @@ void FPU_ADD(int regd, int regt)
void FPU_SUB(int regd, int regt)
{
if (CHECK_FPU_GUARDED)
if (FPU_CORRECT_ADD_SUB)
FPU_ADD_SUB(regd, regt, 1);
else
xSUB.SS(xRegisterSSE(regd), xRegisterSSE(regt));
tests/ctest/core/recompilers/ee_rec_fpu_guardbit_tests.cpp
+4 -27
View File
@@ -8,8 +8,10 @@
// a compliant IEEE FPU would have carried in its guard positions must read as
// zero on PS2 hardware. The recompiler reproduces this by masking the low
// mantissa bits of the smaller-exponent operand by the exponent difference
// before the op - x86 FPU_ADD_SUB (iFPU.cpp:402, applied unconditionally in the
// fast path) and the arm64 fpuEmitGuardedAddSub (iFPU-arm64.cpp).
// before the op - x86 FPU_ADD_SUB (iFPU.cpp:402) and the arm64
// fpuEmitGuardedAddSub (iFPU-arm64.cpp) — unconditionally in both JITs (games
// like True Crime NYC and Jak 3 misrender without it, and per-game flagging
// proved impractical).
//
// THESE ARE JIT-ONLY TESTS. The shared interpreter's ADD_S/SUB_S (FPU.cpp) is a
// plain host float + float (fpuDouble() returns float and does no masking),
@@ -43,7 +45,6 @@ TEST(EeRecFpuGuardBit, SubMasksOneGuardBit)
{
EeRecTestHarness h;
h.EnableCop1();
h.EnableFpuGuarded();
h.SetFprBits(1, 0x40800000u); // 4.0
h.SetFprBits(2, 0x3f800003u); // 1.0 + 3ulp (dirty low bits)
h.LoadProgram({ee::SUB_S(3, 1, 2)});
@@ -57,7 +58,6 @@ TEST(EeRecFpuGuardBit, SubMasksTwoGuardBits)
{
EeRecTestHarness h;
h.EnableCop1();
h.EnableFpuGuarded();
h.SetFprBits(1, 0x41000000u); // 8.0
h.SetFprBits(2, 0x3f80000fu); // 1.0 + 15ulp
h.LoadProgram({ee::SUB_S(3, 1, 2)});
@@ -71,7 +71,6 @@ TEST(EeRecFpuGuardBit, SubMasksFourGuardBits)
{
EeRecTestHarness h;
h.EnableCop1();
h.EnableFpuGuarded();
h.SetFprBits(1, 0x42000000u); // 32.0
h.SetFprBits(2, 0x3f80003fu); // 1.0 + 63ulp
h.LoadProgram({ee::SUB_S(3, 1, 2)});
@@ -86,7 +85,6 @@ TEST(EeRecFpuGuardBit, AddMixedSignMasksGuardBit)
{
EeRecTestHarness h;
h.EnableCop1();
h.EnableFpuGuarded();
h.SetFprBits(1, 0x40800000u); // +4.0
h.SetFprBits(2, 0xbf800003u); // -(1.0 + 3ulp)
h.LoadProgram({ee::ADD_S(3, 1, 2)});
@@ -102,7 +100,6 @@ TEST(EeRecFpuGuardBit, MsubMasksGuardBitOnAccumulate)
{
EeRecTestHarness h;
h.EnableCop1();
h.EnableFpuGuarded();
h.SetAccBits(0x40800000u); // ACC = 4.0
h.SetFprBits(1, 0x3f800003u); // fs = 1.0 + 3ulp
h.SetFprBits(2, 0x3f800000u); // ft = 1.0 -> product = 1.0 + 3ulp (exact)
@@ -119,7 +116,6 @@ TEST(EeRecFpuGuardBit, ExpDiffOneIsUnmaskedAndMatchesInterp)
{
EeRecTestHarness h;
h.EnableCop1();
h.EnableFpuGuarded();
h.SetFprBits(1, 0x40000000u); // 2.0
h.SetFprBits(2, 0x3f800003u); // 1.0 + 3ulp
h.LoadProgram({ee::SUB_S(3, 1, 2)});
@@ -188,7 +184,6 @@ TEST(EeRecFpuGuardBit, RandomizedMatchesX86Model)
{
EeRecTestHarness h;
h.EnableCop1();
h.EnableFpuGuarded();
h.SetFprBits(1, a);
h.SetFprBits(2, b);
h.LoadProgram({ee::ADD_S(3, 1, 2)});
@@ -199,7 +194,6 @@ TEST(EeRecFpuGuardBit, RandomizedMatchesX86Model)
{
EeRecTestHarness h;
h.EnableCop1();
h.EnableFpuGuarded();
h.SetFprBits(1, a);
h.SetFprBits(2, b);
h.LoadProgram({ee::SUB_S(3, 1, 2)});
@@ -212,20 +206,3 @@ TEST(EeRecFpuGuardBit, RandomizedMatchesX86Model)
EXPECT_GT(checked, 1500) << "too many pairs skipped; the test is not exercising the mask";
}
// Default (guard OFF): guard-bit emulation is opt-in via fpuGuardedAddSub, off
// by default. A guard-sensitive subtraction must now emit a plain fsub — no
// masking — which makes the JIT bit-identical to the single-precision
// interpreter (interp never masked). This pins the off-by-default behavior AND
// that the fast path matches interp, so Run()'s JIT-vs-interp auto-diff holds.
// Same operands as SubMasksOneGuardBit, which (with the option ON) asserts the
// masked 0x403fffff; here the bare/interp value 0x403ffffe is the result.
TEST(EeRecFpuGuardBit, DefaultOffEmitsPlainOpMatchingInterp)
{
EeRecTestHarness h;
h.EnableCop1(); // note: no EnableFpuGuarded() — exercising the default
h.SetFprBits(1, 0x40800000u); // 4.0
h.SetFprBits(2, 0x3f800003u); // 1.0 + 3ulp
h.LoadProgram({ee::SUB_S(3, 1, 2)});
h.Run();
h.ExpectFpr(3, 0x403ffffeu); // bare == interp; masked (guard on) would be 0x403fffff
}
tests/ctest/core/recompilers/ee_rec_fpu_tests.cpp
-2
View File
@@ -1548,7 +1548,6 @@ TEST(EeRecFpu, CompareSurvivesInterposedGuardedAddCfc1)
// design (see CompareThenCfc1SeesFreshConditionBit above).
EeRecTestHarness h;
h.EnableCop1();
h.EnableFpuGuarded();
h.SetFpr(1, 2.0f);
h.SetFpr(2, 1.0f);
h.SetFpr(4, 1.0f); // exp 127
@@ -1571,7 +1570,6 @@ TEST(EeRecFpu, CompareSurvivesInterposedGuardedAddBc1)
// (must-take on C clear) falls through instead.
EeRecTestHarness h;
h.EnableCop1();
h.EnableFpuGuarded();
h.SetFpr(1, 2.0f);
h.SetFpr(2, 1.0f);
h.SetFpr(4, 1.0f);
tests/ctest/core/recompilers/harness/EeRecTestHarness.cpp
-12
View File
@@ -61,9 +61,6 @@ EeRecTestHarness::~EeRecTestHarness()
if (fpu_mul_hack_changed_)
EmuConfig.Gamefixes.FpuMulHack = prev_fpu_mul_hack_;
if (fpu_guarded_changed_)
EmuConfig.Cpu.Recompiler.fpuGuardedAddSub = prev_fpu_guarded_;
}
void EeRecTestHarness::SetGpr64(u32 reg_idx, u64 value)
@@ -118,15 +115,6 @@ void EeRecTestHarness::EnableFpuMulHack()
EmuConfig.Gamefixes.FpuMulHack = true;
}
void EeRecTestHarness::EnableFpuGuarded()
{
if (!fpu_guarded_changed_)
{
prev_fpu_guarded_ = EmuConfig.Cpu.Recompiler.fpuGuardedAddSub;
fpu_guarded_changed_ = true;
}
EmuConfig.Cpu.Recompiler.fpuGuardedAddSub = true;
}
void EeRecTestHarness::SetStatusBits(u32 mask) { cpuRegs.CP0.n.Status.val |= mask; }
// EE vtlb_memWrite on a direct RAM hit bypasses Cpu->Clear — upstream relies
tests/ctest/core/recompilers/harness/EeRecTestHarness.h
-8
View File
@@ -69,12 +69,6 @@ public:
void EnableFpuFullMode();
void EnableFpuMulHack();
// Enables the off-by-default fpuGuardedAddSub Recompiler option so the JIT
// emits the PS2 FPU add/sub guard-bit masking (single-precision fast path).
// Off by default in production, so any test that asserts a masked add/sub
// result must opt in. Restored to its previous value in the dtor.
void EnableFpuGuarded();
// ---- Memory ----
void WriteU8 (u32 addr, u8 value);
@@ -349,8 +343,6 @@ private:
bool prev_fpu_full_mode_ = false;
bool fpu_mul_hack_changed_ = false;
bool prev_fpu_mul_hack_ = false;
bool fpu_guarded_changed_ = false;
bool prev_fpu_guarded_ = false;
};
} // namespace recompiler_tests