Repository navigation
fix(recomp): order PS2 floats as sign-magnitude for compares, MIN/MAX and VU MAX/MINI - #321
Open
abilliontokens wants to merge 1 commit into
Open
abilliontokens wants to merge 1 commit into
abilliontokens wants to merge 1 commit into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Defect
The PS2 has no NaN or Inf:
DIV.Sby zero (and every overflow) yields an exponent-255 number pattern —0x7FFFFFFFis what #272'sps2_fdivreturns forx / 0— and any exponent-0 pattern is zero (the FPU flushes denormals). Three instruction groups were still translated to host IEEE semantics:MIN.S/MAX.Semittedstd::min/std::max. Fed a ±max pattern (a saturated division result), the host sees a NaN:std::max(NaN, 2)returns its second argument and a clamp likemax(min(x/0, 1), 0)lets the ±max through instead of clamping to 1/0. PCSX2'sMAX_S/MIN_Sorder PS2 floats as sign-magnitude and flush an exponent-0 selection to signed zero.C.EQ/C.LT/C.LE(and their decoding aliases) compared with==,<,<=, and the "unordered" forms ORed inisnan(). With a ±max operand every compare answers false on the host (NaN), so branches keyed on them take the wrong side. PCSX2 comparesfpuDoublekeys.MAX/MINIemitted_mm_max_ps/_mm_min_ps. SSE returns the second operand when either is NaN, and PS2 ±max lanes are NaN patterns on the host. PCSX2'sfp_max/fp_mincompare the raw lanes as integers, reversed for two negatives.Symptom
Stuntman (SLUS-20250): the car physics divides by a wheel constant that is 0 for the first mission's car;
±max → [0,1] clampproduced garbage instead of 1/0, the driven-wheel integrator span at 2.65e9 and the speedometer read 999 forever. A rigid-bodyc.lt.swith |ω| = 0 took the wrong branch ~9×/frame from frame 0. Both confirmed fixed in-title by an A/B diagnostic that logged every compare/min/max whose answer changed.Fix
ps2_fkeymaps a float to its sign-magnitude order key (exponent-0 ⇒ 0, exponent-255 ordered by magnitude);FPU_C_*compare keys and the "unordered" forms reduce to the four the R5900 implements (the decoder names a title'sc.lt.s/c.le.sC_OLT/C_OLE— noted in a comment).ps2_fmax/ps2_fminselect by key and flush an exponent-0 result to signed zero (ties keep the first operand, likestd::max);MAX.S/MIN.SemitFPU_MAX_S/FPU_MIN_S.ps2_vu_max_ps/ps2_vu_min_psdo the integer lane compare with the both-negative reversal; all six VUMAX/MINIemitter forms (plain, broadcast, immediate) call them.Generated code must be regenerated to pick this up. The new test file compiles with
/fp:precise(like generated code) so the tests cannot be folded away by/fp:fast.Evidence
PS2FpuOrdersuite (6 cases): exponent-255 ordered largest (by mantissa), exponent-0 equal to zero (±0 equal, denormals), MIN/MAX clamping ±max and flushing denormals, the title's own clamp sequence (0x001A94FC–0x001A9508raw words, both value and emitter checks), VU lane ordering with operands swapped so no SSE NaN rule can pass, and the six emitters. All fail onmain.ps2x_tests: Correction (2026-10-16): an earlier version of this description saidEE scheduler selects absolute priority then FIFOalso fails on pristinemain. That was wrong. Our local MSVC build was configured with-DCMAKE_CXX_FLAGS=-msse4.1, which replaces CMake's default flags and so drops/EHsc, and that test failed only in that build. With default flags the suite passes onmain: upstream CI is green oneb8688b(run 38574762065). Theps2x_teststotals measured in that build are withdrawn until re-run; every comparison above used the same build on both sides. TestSuiteRepair found the misconfiguration.