Fix: SQRT.S clamps its operand in the arm64 fast path

recSQRT_S_xmm was the one emitter in iFPU-arm64.cpp that never clamped its
source. fpuClampInput has twelve call sites covering ADD/SUB/MUL/DIV/RSQRT and
the six accumulator forms; SQRT called it zero times. An exponent-255 Ft is an
ordinary large PS2 float, but it reaches the host as Inf, so Fsqrt returned Inf
and fpuClampResult flattened it to 0x7F7FFFFF -- two binades from the
interpreter's sqrt(fpuDouble(Ft)).

Found by the hardware capture landed in 47d910efa6, rows 44/45:

  sqrt +EEMAX : console 5fb504f3  interp 5f7fffff  jit 7f7fffff
  sqrt 2^128  : console 5f800000  interp 5f7fffff  jit 7f7fffff

Unlike the six operand-clamp rows beside them, these did not close under
CHECK_FPU_EXTRA_OVERFLOW -- there was no gate to turn on. That is what made it a
defect rather than the clamp-mode axis.

The gate is CHECK_FPU_OVERFLOW (eeClampMode >= 1, ON by default), not the
arithmetic family's CHECK_FPU_EXTRA_OVERFLOW. x86 recSQRT_S_xmm clamps at that
same lower threshold (iFPU.cpp:1777), and SQRT is alone in it: x86 gates RSQRT's
operand clamp on CHECK_FPU_EXTRA_OVERFLOW (recRSQRThelper1/2, iFPU.cpp:1835/1853),
which recRSQRT_S_xmm already matched, and every other x86 clamp reaches the FPU
through fpuFloat/fpuFloat2 under the same higher gate. Matching x86 rather than
DIV.S is what aligns all three engines in the mode games actually run in.
Direction per the standing rule: the interpreter was the side nearer the console,
so the recompiler moved. At eeClampMode 0 nothing is emitted, exactly as before.

One-sided, since Fabs has already made the operand non-negative -- the same
positive-only shape as x86's xMIN.SS. It is NOT Fminnm, which is what the first
cut of this used, and that was wrong: FPMinNum only prefers the number when the
other operand is a QUIET NaN, so a signalling operand goes down FPProcessNaNs
and comes back merely quieted, surviving the clamp. x86's MINSS returns src2 for
ANY NaN, and half of the EE's exponent-255 mantissa space is signalling, so
Fminnm covered only half the class the comment claims ("any Ft whose exponent
field is 255"). Measured exhaustively on this host over all 2^31 non-negative
operands, against a model of MINSS(x, +FLT_MAX):

    UMIN   mismatches vs MINSS: 0
    FMINNM mismatches vs MINSS: 4194303   (first at 7f800001)

End to end, sqrt(0x7F800001) came back 0x7F7FFFFF where the interpreter -- whose
fpuDouble switches on the exponent FIELD alone, mantissa irrelevant -- gives
0x5F7FFFFF. Same for 0xFF800001 and 0x7FBFFFFF. The capture's three SQRT rows
are a qNaN, an Inf and a finite number, so nothing in it could reach the
signalling half.

So the clamp is done in the integer domain instead: the operand is post-Fabs, so
bit 31 is clear, and over non-negative floats the IEEE ordering IS the unsigned
integer ordering. Umin against 0x7F7FFFFF clamps Inf, sNaN and qNaN alike and
passes every representable finite value -- exact MINSS agreement on every input,
at one instruction. Umin has no scalar form so it is a 2S vector op; only lane 0
carries the operand and the scalar Fsqrt that follows zeroes the rest.

Kept as a SQRT-local helper rather than folded into fpuClampResultPositive,
whose other caller recABS_S_xmm emits its clamp with no CHECK_FPU_* gate at all,
where the interpreter and the console both leave exponent-255 operands alone.
That is a separate pre-existing defect whose fix is to delete the clamp, not to
change which wrong answer it produces; editing the shared helper would have
moved ABS.S's output for an unfixed case. Verified unchanged: ABS.S/NEG.S still
diverge on the same 30 of 48 rows (EeFpuAbsNegClamp.DISABLED_DumpAllLegs).

FULL mode was checked and does not share the gap: DOUBLE::recSQRT_S_xmm widens
through ToDouble, which carries exponent 255 across exactly, and
EeRecFpuFull.SqrtPseudoInfExact already pins the true sqrt(2^128) = 0x5f800000.
Its inline note about the fast path was describing behavior the fast path did
not yet have; it does now, so the note is updated with the measured value.

Verified bidirectionally. On the unpatched emitter with these tests present,
SqrtClampsItsOperandLikeTheRestOfTheFamily fails on rows 44 and 45 in both clamp
modes (interp 5f7fffff vs jit 7f7fffff) and passes on row 46, and
EnginesAgreeExceptOnTheDocumentedRows fails because the two rows no longer
belong on the allowance list. SqrtClampCoversSignallingOperandsToo sweeps every
exponent-255 shape in both signs and fails on exactly the three signalling rows
under Fminnm. With the patch all enabled tests in the file pass, and the console
tally is unchanged at 20 match / 19 value-only / 3 flag-only / 15 both -- the JIT
moved onto the interpreter's answer without changing what the file says about
the hardware.

The tripwire is promoted to an enabled regression test that asserts the value as
well as the agreement -- agreement alone could be reached by degrading the
interpreter, which is the side nearer the console here.

Idea by pstef.
This commit is contained in:
bmdhacks
2026-08-02 22:24:24 -07:00
committed by Brian Degenhardt
parent 2951dd5fb0
commit d80101329e
3 changed files with 213 additions and 32 deletions
+65
View File
@@ -363,6 +363,40 @@ static void fpuClampResultPositive(const a64::VRegister& fpr)
armAsm->Fminnm(fpr, fpr, a64::s8);
}
// Positive OPERAND clamp, for a value that is about to be fed to another FP
// op rather than written back — currently SQRT.S's post-Fabs Ft.
//
// Fminnm would be the obvious choice and is wrong here. It is only NaN-eating
// for QUIET NaNs: FPMinNum prefers the number when the other operand is a
// quiet NaN, but a SIGNALLING operand goes down FPProcessNaNs first and comes
// back merely quieted, so it survives the clamp. x86's MINSS has no such
// split — it returns src2 for any NaN — and half of the EE's exponent-255
// mantissa space (4194303 of the 8388608 positive patterns) is signalling.
// Those are ordinary large PS2 floats, not errors; the interpreter's fpuDouble
// keys on the exponent FIELD and clamps all of them.
//
// So clamp in the integer domain. The input is post-Fabs, hence bit 31 is
// clear, and over non-negative floats the IEEE ordering IS the unsigned
// integer ordering: every pattern above 0x7F7FFFFF is exponent-255 (Inf, sNaN
// or qNaN alike) and must come down, everything at or below it is a
// representable finite value and must pass. One instruction, same as Fminnm,
// and exact agreement with MINSS on every input.
//
// Umin has no scalar form, so this is a 2S vector op. Only lane 0 carries the
// operand; lane 1 takes min(garbage, v8[1]) and is discarded by the scalar
// Fsqrt that follows, which zeroes bits [127:32] of its destination.
//
// Deliberately NOT folded into fpuClampResultPositive above. That helper's
// other caller, recABS_S_xmm, emits its clamp with no CHECK_FPU_* gate at all,
// where both the interpreter and the console leave exponent-255 operands
// untouched — a separate pre-existing defect whose fix is to DELETE the clamp,
// not to change which wrong answer it produces (see the DISABLED tripwire
// EeFpuAbsNegClamp.DISABLED_JitMatchesConsoleInEveryClampMode). Changing the
// shared helper would silently move ABS.S's output for that unfixed case.
static void fpuClampOperandPositive(const a64::VRegister& fpr)
{
armAsm->Umin(fpr.V2S(), fpr.V2S(), a64::v8.V2S());
}
// Sign-preserving operand clamp for FPU comparisons (C.cond.S).
//
// Mirrors the x86 JIT's fpuFloat3 (PMIN.SD vs 0x7f7fffff then PMIN.UD vs
@@ -1001,6 +1035,37 @@ static void recSQRT_S_xmm(int info)
// PS2 takes sqrt of |ft| → Fabs first.
armAsm->Fabs(armSRegister(EEREC_D), ft);
// Source-operand clamp. An exponent-255 Ft is an ordinary large PS2 float
// (the EE has no Inf/NaN), but the host reads it as Inf/NaN — so without
// this, Fsqrt returns Inf and fpuClampResult flattens it to +fMax, two
// binades from where the interpreter lands (SQRT_S routes Ft through
// fpuDouble unconditionally). Measured against the hardware capture:
// sqrt(0x7FFFFFFF) was interp 0x5F7FFFFF vs JIT 0x7F7FFFFF, and
// sqrt(0x7F800000) the same pair — autocases_fpuovf.h rows 44/45, pinned by
// EeFpuOverflowConsole.SqrtClampsItsOperandLikeTheRestOfTheFamily.
//
// The gate is CHECK_FPU_OVERFLOW (eeClampMode >= 1, ON by default), NOT the
// arithmetic family's CHECK_FPU_EXTRA_OVERFLOW (>= 2) that fpuClampInput
// carries. x86 recSQRT_S_xmm clamps at that same lower threshold
// (`if (CHECK_FPU_OVERFLOW) xMIN.SS(EEREC_D, g_maxvals[0])`, iFPU.cpp:1777),
// so matching it is what puts interp, x86 and arm64 on one answer in the
// mode games actually run in — the same lower-gate reasoning as
// fpuClampMinMaxOperand above. SQRT is alone in this: x86 recRSQRThelper1 /
// recRSQRThelper2 (iFPU.cpp:1835/1853) gate RSQRT's operand clamp on
// CHECK_FPU_EXTRA_OVERFLOW, which is what recRSQRT_S_xmm below already does,
// and every remaining x86 clamp reaches the FPU through fpuFloat/fpuFloat2
// under the same higher gate.
//
// One-sided, because Fabs has already made the operand non-negative: only
// the upper half of fpuClampResult could ever fire. That is exactly x86's
// positive-only xMIN.SS — and it has to match MINSS on NaNs too, which is
// why this is an integer Umin and not an Fminnm; see the comment on
// fpuClampOperandPositive for the signalling-NaN split that rules Fminnm
// out. Pinned by EeFpuOverflowConsole.SqrtClampCoversSignallingOperandsToo.
if (CHECK_FPU_OVERFLOW)
fpuClampOperandPositive(armSRegister(EEREC_D));
armAsm->Fsqrt(armSRegister(EEREC_D), armSRegister(EEREC_D));
fpuClampResult(armSRegister(EEREC_D));
@@ -44,6 +44,8 @@
#include <gtest/gtest.h>
#include <ios>
using namespace recompiler_tests;
using namespace mips;
using namespace mips::ee;
@@ -156,20 +158,14 @@ constexpr EngineDivergence kEngineDivergences[] = {
// question is deferred to the redesign (see the DISABLED tripwires in
// ee_fpu_fcr_console_conformance_tests.cpp).
// CLASS 2 -- a real gap, and the capture is what surfaced it. SQRT.S is the
// ONLY op in iFPU-arm64.cpp whose emitter never clamps its operand:
// fpuClampInput has twelve call sites covering ADD/SUB/MUL/DIV/RSQRT and
// the six accumulator forms, and recSQRT_S_xmm calls it zero times. So an
// exponent-255 Ft reaches Fsqrt as a host +Inf, sqrt(Inf) is Inf, and
// fpuClampResult flattens it to 0x7F7FFFFF -- while the interpreter's
// SQRT_S does sqrt(fpuDouble(Ft)) and lands two binades away. Unlike class
// 1 this does NOT close under CHECK_FPU_EXTRA_OVERFLOW, because there is no
// gate to turn on. The interpreter is the side nearer the console on both
// rows, so the direction is to give SQRT the clamp the rest of the family
// already has -- never to stop the interpreter clamping.
// Pinned by DISABLED_SqrtOperandClampIsMissing below.
{44, false, "sqrt +EEMAX -- recSQRT_S_xmm never clamps Ft"},
{45, false, "sqrt 2^128 -- same"},
// CLASS 2 used to live here: rows 44 and 45, sqrt of an exponent-255 Ft,
// where recSQRT_S_xmm was the one emitter in iFPU-arm64.cpp that never
// clamped its operand at all. That was a defect rather than a mode axis --
// it did not close under CHECK_FPU_EXTRA_OVERFLOW because there was no gate
// to turn on -- and it is fixed: SQRT now clamps under CHECK_FPU_OVERFLOW,
// matching x86 recSQRT_S_xmm. The rows are covered by
// SqrtClampsItsOperandLikeTheRestOfTheFamily below and by the general
// agreement test, so they must NOT be listed here any more.
};
constexpr int kEngineDivergenceCount =
static_cast<int>(sizeof(kEngineDivergences) / sizeof(kEngineDivergences[0]));
@@ -218,12 +214,15 @@ TEST(EeFpuOverflowConsole, EnginesAgreeExceptOnTheDocumentedRows)
}
// ---------------------------------------------------------------------------
// ENABLED. Splits the divergence list in two by measurement rather than by
// assertion in a comment: the operand-clamp rows close when the clamp is on,
// the SQRT rows do not, because SQRT has no clamp to turn on.
// ENABLED. Classifies the divergence list by measurement rather than by
// assertion in a comment: every listed row must close when the operand clamp
// is on. The else-branch is the liveness clause for any future entry that does
// NOT close -- a defect rather than the mode axis, which is what rows 44/45
// were before SQRT gained its clamp.
// ---------------------------------------------------------------------------
TEST(EeFpuOverflowConsole, OperandClampHealsEveryDivergenceExceptSqrt)
TEST(EeFpuOverflowConsole, OperandClampHealsEveryDocumentedDivergence)
{
ASSERT_GT(kEngineDivergenceCount, 0) << "nothing left to classify";
for (int i = 0; i < kEngineDivergenceCount; ++i)
{
const EngineDivergence& d = kEngineDivergences[i];
@@ -280,29 +279,141 @@ TEST(EeFpuOverflowConsole, DefaultClampModeSaturatesToFltMaxOnBothEngines)
}
// ---------------------------------------------------------------------------
// TRIPWIRE. SQRT.S is the only EE FPU op whose arm64 emitter never clamps its
// operand. Enable this after giving recSQRT_S_xmm the fpuClampInput call the
// other twelve emitters have, gated on CHECK_FPU_EXTRA_OVERFLOW the same way;
// then drop rows 44 and 45 from kEngineDivergences.
// REGRESSION TEST for the defect this capture surfaced. recSQRT_S_xmm was the
// one emitter in iFPU-arm64.cpp that never clamped its operand -- fpuClampInput
// had twelve call sites covering ADD/SUB/MUL/DIV/RSQRT and the six accumulator
// forms, and SQRT called it zero times. An exponent-255 Ft therefore reached
// Fsqrt as a host +Inf, sqrt(Inf) was Inf, and fpuClampResult flattened it to
// 0x7F7FFFFF, while the interpreter's sqrt(fpuDouble(Ft)) landed two binades
// away at 0x5F7FFFFF.
//
// Direction matters here: the interpreter is the side nearer the console on
// both rows (console 5fb504f3 / 5f800000, interp 5f7fffff, JIT 7f7fffff), so
// this is fixed by moving the recompiler, never by making the interpreter stop
// clamping.
// Before the fix this failed on rows 44 and 45 and passed on row 46 (whose Ft
// is representable and needs no clamp).
//
// Two things are asserted, not one. Agreement alone is a weak pin: it can be
// reached by degrading the interpreter, which is the side NEARER the console
// here (console 5fb504f3 / 5f800000 vs interp 5f7fffff vs old JIT 7f7fffff).
// So the expected value is spelled out as well -- the interpreter's answer,
// which is sqrt of the fpuDouble-clamped operand.
//
// The clamp is gated on CHECK_FPU_OVERFLOW (eeClampMode >= 1, on by default),
// matching x86 recSQRT_S_xmm's `if (CHECK_FPU_OVERFLOW) xMIN.SS(...)`, so the
// DEFAULT mode is checked first and the higher clamp mode second.
// ---------------------------------------------------------------------------
TEST(EeFpuOverflowConsole, DISABLED_SqrtOperandClampIsMissing)
TEST(EeFpuOverflowConsole, SqrtClampsItsOperandLikeTheRestOfTheFamily)
{
ASSERT_TRUE(EmuConfig.Cpu.Recompiler.fpuOverflow)
<< "the SQRT operand clamp is gated on CHECK_FPU_OVERFLOW; with the "
"option off the recompiler is not being asked to clamp at all";
// sqrt(0x7F7FFFFF), i.e. sqrt of the operand after fpuDouble/xMIN.SS has
// pulled an exponent-255 word down to +FLT_MAX. Both engines must produce
// this for any Ft whose exponent field is 255.
constexpr u32 kSqrtOfFastPathMax = 0x5F7FFFFFu;
int clamped_rows = 0, total_rows = 0;
for (int i = 0; i < kCaseCount; ++i)
{
const FpuOvfCase& c = kCases[i];
if (c.op != FO_SQRT)
continue;
++total_rows;
SCOPED_TRACE(::testing::Message() << "row " << i << ": " << c.what);
EXPECT_TRUE(Agree(RunCase(c, false, /*extra_overflow=*/true),
RunCase(c, true, /*extra_overflow=*/true)))
<< "SQRT.S still does not clamp its operand under "
"CHECK_FPU_EXTRA_OVERFLOW";
for (int extra = 0; extra < 2; ++extra)
{
SCOPED_TRACE(::testing::Message()
<< (extra ? "eeClampMode >= 2" : "default clamp mode"));
const Observed in = RunCase(c, false, extra != 0);
const Observed ji = RunCase(c, true, extra != 0);
EXPECT_TRUE(Agree(in, ji))
<< "interp " << std::hex << in.result << "/" << in.fcr31
<< " vs jit " << ji.result << "/" << ji.fcr31;
if ((c.ft & 0x7F800000u) == 0x7F800000u)
{
if (extra == 0)
++clamped_rows;
EXPECT_EQ(in.result, kSqrtOfFastPathMax) << "interp";
EXPECT_EQ(ji.result, kSqrtOfFastPathMax) << "jit";
}
}
}
EXPECT_GT(total_rows, 0) << "no SQRT rows in the capture; vacuous";
EXPECT_GT(clamped_rows, 0)
<< "no SQRT row feeds an exponent-255 operand any more, so the clamp "
"itself is never exercised; this test would pass vacuously";
}
// ---------------------------------------------------------------------------
// The same property as above, over the WHOLE exponent-255 class rather than
// the three patterns the capture happens to contain.
//
// The capture's SQRT rows feed 0x7FFFFFFF, 0x7F800000 and 0xFF7FFFFF. As host
// bit patterns those are a quiet NaN, an infinity and a finite number -- the
// one class it never feeds is a SIGNALLING NaN, and that is the class the
// arm64 clamp misses. Fminnm is not MINSS: MINSS returns src2 for ANY NaN,
// while FMINNM only prefers the number when the other operand is a QUIET NaN.
// A signalling operand goes down the FPProcessNaNs path instead and comes back
// quieted, unclamped. That is half the exponent-255 mantissa space --
// 4194303 of the 8388608 positive patterns -- passing straight through a clamp
// the comment above says covers "any Ft whose exponent field is 255".
//
// The interpreter has no such split: fpuDouble (FPU.cpp) switches on the
// exponent FIELD alone, so every exponent-255 operand becomes +-0x7F7FFFFF
// regardless of mantissa. It is also the side nearer the console. So this is
// asserted as a value, not just as agreement.
// ---------------------------------------------------------------------------
TEST(EeFpuOverflowConsole, SqrtClampCoversSignallingOperandsToo)
{
ASSERT_TRUE(EmuConfig.Cpu.Recompiler.fpuOverflow)
<< "the SQRT operand clamp is gated on CHECK_FPU_OVERFLOW";
constexpr u32 kSqrtOfFastPathMax = 0x5F7FFFFFu;
struct Operand
{
u32 ft;
const char* what;
};
// Every exponent-255 shape, both signs. The host classification is noted
// because it is the axis the defect splits on -- nothing about the EE
// itself distinguishes these, they are all just large floats.
static constexpr Operand kOperands[] = {
{0x7F800000u, "+2^128 (host +Inf)"},
{0xFF800000u, "-2^128 (host -Inf)"},
{0x7F800001u, "exp255 mant 1 (host +sNaN, smallest)"},
{0xFF800001u, "exp255 mant 1 (host -sNaN, smallest)"},
{0x7FBFFFFFu, "exp255 mant 0x3FFFFF (host +sNaN, largest)"},
{0x7FC00000u, "exp255 mant 0x400000 (host +qNaN, smallest)"},
{0x7FFFFFFFu, "+EEMAX (host +qNaN, largest)"},
{0xFFFFFFFFu, "-EEMAX (host -qNaN, largest)"},
};
int signalling = 0;
for (const Operand& o : kOperands)
{
const FpuOvfCase c{FO_SQRT, 0u, o.ft, 0u, 0u, 0u, false, o.what};
SCOPED_TRACE(::testing::Message() << o.what);
const Observed in = RunCase(c, false);
const Observed ji = RunCase(c, true);
EXPECT_EQ(in.result, kSqrtOfFastPathMax)
<< "[interp] fpuDouble keys on the exponent field alone";
EXPECT_EQ(ji.result, kSqrtOfFastPathMax)
<< "[jit] the operand reached Fsqrt unclamped -- Fminnm passes a "
"signalling NaN through where x86's MINSS would return +fMax";
EXPECT_EQ(in.fcr31, ji.fcr31) << "FCR31 diverges between engines";
const u32 mant = o.ft & 0x7FFFFFu;
if (mant != 0 && (mant & 0x400000u) == 0)
++signalling;
}
EXPECT_GE(signalling, 3)
<< "anti-vacuity: the operand pool must keep signalling-NaN patterns, "
"which are the only ones the Fminnm/MINSS split can act on";
}
// ---------------------------------------------------------------------------
@@ -561,7 +561,12 @@ TEST(EeRecFpuFull, SqrtPseudoInfExact)
h.SetFprBits(1, kPs2HugePos);
h.LoadProgram({SQRT_S(2, 1)});
h.RunJitNoDiff();
EXPECT_EQ(h.GetFprBitsJit(2), 0x5f800000u); // fast: sqrt(clamped FLT_MAX)
// Discriminator: ToDouble carries exponent 255 across exactly, so FULL gets
// the true sqrt(2^128). The single-precision fast body clamps the operand
// to +FLT_MAX first (recSQRT_S_xmm's CHECK_FPU_OVERFLOW xMIN, matching x86)
// and lands one ULP low at 0x5f7fffff — measured in
// EeFpuOverflowConsole.SqrtClampsItsOperandLikeTheRestOfTheFamily.
EXPECT_EQ(h.GetFprBitsJit(2), 0x5f800000u);
}
TEST(EeRecFpuFull, SqrtNegativeSetsIFlagAndUsesAbs)