fix(test): make sceGsSyncV field-parity assertions robust to vblank timing#182
Draft
smmathews wants to merge 1 commit into
Draft
fix(test): make sceGsSyncV field-parity assertions robust to vblank timing#182smmathews wants to merge 1 commit into
smmathews 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.
Problem
The interlaced field-parity test for
sceGsSyncVraces the free-running vsync-tick source intwo ways:
sceGsSyncVcall and the tick it wascomputed from are never observed together; the test only reads the returned field, never the
tick that produced it.
consumes exactly
base + 1, i.e. exactly one vblank elapses between the reset's base captureand the first wait.
On a loaded test runner, a vblank can land in that window between the reset and the first wait.
When it does, the true parity inverts relative to the hardcoded assumption, and the assertion
fails even though the field-parity logic itself is correct. This fails intermittently on loaded
Windows CI runners while passing reliably on Linux — a timing artifact, not a code regression.
Fix
sceGsSyncVnow records the vblank tick it consumed under its existing mutex. Two read-onlyaccessors,
lastGsSyncVConsumedTick()andgsSyncVBaseTick(), expose that consumed tick and theparity-anchor base to the test. The test now computes each call's expected field directly from
the measured
(base, tick)gap —(tick − base − 1) & 1, with thetick ≤ base ⇒ 0clausemirrored exactly — instead of assuming a fixed gap. It adds a phase-independent XOR cross-check
(
field0 ^ field1 == (tick1 − tick0) & 1) and a liveness check (tick1 > tick0), and keeps theoriginal literal even/odd anchor, but only asserts it under the nominal gaps where it is
guaranteed true.
Why this is race-free by construction
Every assertion is a pure function of the coherent triple
(base, tick0, tick1), all read backafter their respective calls return. Nothing re-samples a live counter, and nothing assumes a
fixed elapsed-tick count — whatever gap the loaded runner actually produced, the expectation is
derived from that same gap. The literal 0/1 anchor fires only when its precondition
(
tick0 == base + 1 && tick1 - tick0 == 1) is literally true, so it can never see a falsemismatch. The failing interleaving becomes impossible, not merely rarer.
Accessors' zero-behavior-change argument
The only substantive runtime edit is the tick-recording write in
sceGsSyncV. It is observationstate, not behavior:
sceGsSyncV's observable effects are exactly (a) the value written to thereturn register via
setReturnS32, and (b) the vblank wait it performs. Both are byte-identicalto before this change — the same consumed tick still flows into the existing field-parity
computation unchanged; no guest memory, no other register, no field math, and no wait timing is
touched. The recorded value is written to a variable read only by the two new accessors, which
are pure lock-and-return functions mirroring the mutex discipline already used by the existing
field-parity helper. This can be confirmed by diff review plus a full green test suite.
Testing
Configure and build per the canonical CI commands, then run the test binary from the repository
root (one of the runtime tests is working-directory sensitive):
To verify the rework actually catches regressions, the following single-line mutations to
getGsSyncVFieldForTick/sceGsSyncVinps2xRuntime/src/lib/Kernel/Stubs/GS.cppeach makeexactly the
sceGsSyncVinterlaced-parity test fail while the rest of the suite stays green:-1uphase offset from the field computation.& 1uto& 0u(field always even).Windows-msvc CI is the real arbiter that the failing interleaving is eliminated, since the
original flake is specific to loaded Windows CI runners and is not expected to reproduce on an
unloaded Linux box.