Skip to content

Lockstep: stop reporting a step-cap timeout as a control-flow divergence - #16

Open
KakarottoCake wants to merge 1 commit into
ExpansionPak:mainfrom
KakarottoCake:lockstep-step-cap-artefact
Open

Lockstep: stop reporting a step-cap timeout as a control-flow divergence#16
KakarottoCake wants to merge 1 commit into
ExpansionPak:mainfrom
KakarottoCake:lockstep-step-cap-artefact

Conversation

@KakarottoCake

Copy link
Copy Markdown

The problem

LockstepCheck single-steps the shadow interpreter up to m_ls_step_cap
instructions looking for the block's end PC. If it runs out of steps, reached
stays false and the block is reported as a CTRLFLOW divergence.

But at that point nothing has been compared. Running out of steps is the
verifier giving up, not evidence that the module went anywhere different. The
tool was reporting its own timeout as a fault in the code under test, and the
default cap of 512 is low enough that this dominated the output.

Concretely, on a full retail GameCube title, the same run:

step cap CTRLFLOW reports FP divergences
512 (old default) 631 463
20000 22 622

Two separate problems show up in that table.

It manufactures divergences. The 609 CTRLFLOW reports that disappear were
all sitting exactly at the cap. They were never divergences, and no change to
the recompiler could ever have fixed them — I spent real time chasing them
before working out where they came from.

It also hides divergences, which is worse. Any block longer than the cap
never reaches the register comparison at all, so its mismatches are silently
skipped. Raising the cap surfaced 159 additional real FP divergences that
the old default had been concealing. A verifier that quietly stops checking is
a more expensive bug than one that over-reports.

The change

Three small edits, all in the lockstep verifier — no emulator behavior is
touched:

  • m_ls_step_cap default 51220000. STATICRECOMP_LOCKSTEP_STEPCAP
    still overrides it, so anyone relying on the old value can set it back.
  • A cap timeout now increments a dedicated m_ls_cap_hits counter and returns,
    instead of formatting a CTRLFLOW report. This is placed immediately after
    reached is computed, alongside the existing early returns.
  • cap_hits= added to the [lockstep] summary: line, so the give-up count is
    visible rather than merely absent.

A cap timeout is now reported as what it is — the verifier declining to answer —
which is different information from both "clean" and "diverged".

I did not touch LS_UNDERCHARGE_GRACE, the loop-header logic, or anything in
the comparison itself.

Testing

I should be straightforward about the limits here. This is exercised in a
downstream Dolphin fork running static-recomp modules for a retail GameCube
title, which is where the numbers above come from; the same three edits applied
to a tree of yours. I have not built this specific branch against upstream
main locally
— a full build is a long job on this machine and I would rather
put the finding in front of you now than sit on it. The change is a counter, an
early return and a printf argument, so I would expect CI to be the fast check.
Happy to build it properly if you would like that before considering it.

If 20000 seems arbitrary — it is chosen to be comfortably above the longest
block observed in practice rather than tuned. If you would prefer the cap left
alone and only the reporting fixed, that is a one-line change and I am happy to
split it.

AI disclosure

Contributing.md asks for this, so: this patch was written with AI
assistance
, and I would rather you close it than have it merged under a wrong
impression.

It came out of debugging a downstream static-recomp runtime, where the false
CTRLFLOW reports sent me down a long dead end before the cause became clear.
The investigation, the measurements above and the patch were all AI-assisted.

I am aware of the rule against using LLMs for changes related to emulated
console behavior. I believe this one sits outside it — it changes only how the
differential verifier reports its own timeouts, and touches no decoding, no FP
semantics, no register or memory modelling, and nothing the guest can observe.
But that is your call to make, not mine, which is why the disclosure is up front
rather than in a footnote.

If the rule means no regardless, that is completely fair. The table above is the
part that matters and it is yours to use either way: the default step cap is
concealing real divergences, whoever fixes it.

The shadow interpreter walks at most m_ls_step_cap instructions looking for
the block's end PC. When it runs out, `reached` is false and the block is
reported as CTRLFLOW -- but nothing has been compared at that point, so there
is no evidence the module diverged at all. The verifier was reporting its own
give-up condition as a fault in the code under test.

At the old default of 512 this dominated the output. On a full retail GameCube
title the same run reported 631 CTRLFLOW divergences at cap 512 and 22 at cap
20000, and the 609 that disappeared were all sitting exactly at the cap.

The cap was also hiding real divergences, which is the worse half: any block
longer than 512 steps never reaches the register comparison, so its mismatches
are silently skipped. Raising the cap took FP divergences from 463 to 622 --
159 real reports the old default concealed.

So: raise the default to 20000, and count a cap timeout in its own counter
(cap_hits, surfaced in the summary line) instead of emitting a divergence for
it. STATICRECOMP_LOCKSTEP_STEPCAP still overrides it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant