EE: scan past non-overlapping neighbours when hunting stale blocks

Both the stale-overlap walk at the tail of recRecompile and recClear
descend recBlocks and stop at the first block that ends before the range
they care about. recBlocks is ordered by startpc, not by end address, so
that stop is unsound: a long block at a low address can jump clean over a
short one lying between it and the range, and the scan quits before the
long one is ever examined.

The walk then misses a genuinely stale block, and recClear leaves it in
the cache — the straddler-from-below it is specifically written to remove.
Reachable whenever a block goes stale AFTER its neighbours compiled, which
is exactly the ordering the walk exists for: an earlier compile's own walk
would otherwise have cleared it.

Scan past a non-overlapping neighbour instead of stopping on it, bounded
by s_maxBlockBytes — the longest guest extent compiled since the last
reset, so the scan still terminates as soon as nothing below could reach.
recClear splits its pending remove range around a skipped survivor exactly
as it already does for s_pCurBlock.

recClear also drops its lowerextent floor clamp. A skipped survivor can
sit inside a removed straddler's extent, and raising the floor to that
survivor's end would leave the straddler's own start word pointing at its
removed stub — the fnptr assert StraddlerBlockRecClearResetsStartFnptr
pins. Resetting a survivor's LUT entry instead costs it one recompile.

This was inert until f4debc54c1: with recRAMCopy indexed at startpc/4 the
compare mismatched on essentially every overlapping pair, so the walk
always cleared before under-scanning could cost anything. A working
compare makes the blind spot load-bearing.

Final Fantasy X (SLUS-20312), 300 frames from an in-game savestate, with
the scan instrumented — behaviour is unchanged and the added work is
noise:

                    before    after
  recompiles          2642     2642
  blocks removed       745      745
  stale found            0        0
  walk iterations     5239    19377
  recClear iters       811     1113
  max block extent   880 B    880 B

~5 extra loop iterations per block compile, ~0.004% of EE-thread cycles;
EE-thread CPU time 0.58 s both ways. The high-water mark stays far under
the 4 KB the page-boundary rule implies, so the bound is tight in practice.

recompiler_tests 1438/1438. New gate OverlapWalkScansPastNonOverlappingNeighbor
builds the A/B/N geometry (long A, short B truncated against C, new N above
B's end), pokes A stale after B and C compiled, and asserts A is cleared;
it fails on the walk fix alone, so both halves are pinned. Upstream x86
carries the same stop in both places (iR5900.cpp:786, :2688).
This commit is contained in:
Brian Degenhardt
2026-07-25 12:40:18 -07:00
parent 450d5e5e28
commit 3ac41e05bd
2 changed files with 148 additions and 3 deletions
+38 -3
View File
@@ -51,6 +51,9 @@ namespace a64 = vixl::aarch64;
// =====================================================================================================
u32 maxrecmem = 0;
// Longest guest extent, in bytes, of any block compiled since the last reset.
// Bounds how far back the stale-overlap walk in recRecompile must scan.
static u32 s_maxBlockBytes = 0;
u32 pc;
int g_branch;
u32 target;
@@ -2686,6 +2689,13 @@ static void recClear(u32 addr, u32 size)
// removed blocks (a straddler can extend well past `end` or below
// `addr`). `ceiling` clamps the tail at the next surviving block's
// startpc so we never trample its interior.
//
// There is deliberately no matching floor clamp. A survivor skipped by the
// scan below can sit INSIDE a removed straddler's extent, and raising
// lowerextent to that survivor's end would leave the straddler's own start
// word still pointing at its removed stub — the fnptr-assert case
// StraddlerBlockRecClearResetsStartFnptr pins. Resetting a survivor's entry
// instead merely costs it a recompile.
u32 lowerextent = static_cast<u32>(-1);
u32 upperextent = 0;
u32 ceiling = static_cast<u32>(-1);
@@ -2721,8 +2731,19 @@ static void recClear(u32 addr, u32 size)
if (blockend <= addr)
{
lowerextent = std::max(lowerextent, blockend);
break;
// Not overlapping — but recBlocks is ordered by startpc, NOT by
// end address, so a block further down can still be long enough
// to reach [addr, end); this one is no proof the rest miss too.
// Keep it, splitting the pending remove range around it exactly
// as for s_pCurBlock, and carry on until even the longest block
// compiled since the last reset could not span the gap.
if (blockstart + s_maxBlockBytes <= addr)
break;
if (toRemoveLast != blockidx)
recBlocks.Remove(blockidx + 1, toRemoveLast);
toRemoveLast = --blockidx;
continue;
}
lowerextent = std::min(lowerextent, blockstart);
@@ -3076,6 +3097,7 @@ static void recResetRaw()
recBlocks.Reset();
maxrecmem = 0;
s_maxBlockBytes = 0;
memset(manual_page, 0, sizeof(manual_page));
memset(manual_counter, 0, sizeof(manual_counter));
@@ -4096,6 +4118,10 @@ StartRecomp:
pxAssert((pc - startpc) >> 2 <= 0xffff);
s_pCurBlockEx->size = (pc - startpc) >> 2;
// High-water mark of any compiled block's guest extent, for the
// stale-overlap walk's scan-back bound below. Reset with the block array.
s_maxBlockBytes = std::max(s_maxBlockBytes, pc - startpc);
if (!(pc & 0x10000000))
maxrecmem = std::max((pc & ~0xa0000000), maxrecmem);
@@ -4120,8 +4146,17 @@ StartRecomp:
continue;
if (oldBlock->startpc >= HWADDR(pc))
continue;
if ((oldBlock->startpc + oldBlock->size * 4) <= HWADDR(startpc))
// recBlocks is ordered by startpc, NOT by end address: a block
// starting lower can be longer and still reach us, jumping clean
// over a short one between it and [startpc, pc). So one
// non-overlapping neighbour is no proof the blocks below it miss
// too — skip it, and only stop once even the longest block ever
// compiled could not span the gap. (Upstream x86 breaks here —
// iR5900.cpp:2688 — and silently skips the stale block below.)
if (oldBlock->startpc + s_maxBlockBytes <= HWADDR(startpc))
break;
if ((oldBlock->startpc + oldBlock->size * 4) <= HWADDR(startpc))
continue;
// recRAMCopy is a byte array covering guest main RAM 1:1 — index
// it by guest address. Do NOT reintroduce the `/ 4` upstream x86
@@ -22,6 +22,7 @@ using namespace recompiler_tests;
using namespace mips;
extern bool recEeBlockHostInfo(u32 pc_query, uptr* fnptr, u32* host_size, uptr* lut_fnptr);
extern u32 recEeBlockGuestSize(u32 pc_query);
namespace {
constexpr u32 kProgramPc = RecompilerTestEnvironment::kProgramPc;
@@ -340,3 +341,112 @@ TEST(EeRecSmc, OverlapWalkIgnoresUnmodifiedNeighbors)
h.Run(EeRecTestHarness::RunMode::PreserveCache);
h.ExpectGpr64(reg::v0, 9);
}
namespace {
// A second RAM page, for the four-block skip geometry below.
constexpr u32 kSkipRoutinePc = 0x000B0000;
// v0 = head_imm, then nine increments, then JR ra. Entering at any word
// yields a distinct v0, so which compiled body actually ran is observable.
//
// +0x00 ADDIU v0, zero, head_imm
// +0x04 .. +0x24 ADDIU v0, v0, 1 (9 words)
// +0x28 JR ra
// +0x2C NOP extent = [S, S+0x30), 12 words
void SeedSkipRoutine(EeRecTestHarness& h, u16 head_imm)
{
h.WriteU32(kSkipRoutinePc + 0x00, ADDIU(reg::v0, reg::zero, head_imm));
for (u32 off = 0x04; off <= 0x24; off += 4)
h.WriteU32(kSkipRoutinePc + off, ADDIU(reg::v0, reg::v0, 1));
h.WriteU32(kSkipRoutinePc + 0x28, JR(reg::ra));
h.WriteU32(kSkipRoutinePc + 0x2C, NOP);
}
} // namespace
// The walk descends recBlocks by startpc and `break`s at the first block that
// ends before the new block starts. That assumes end addresses rise with start
// addresses — recBlocks guarantees no such thing. A long block at a LOW address
// can jump clean over a short one lying between it and the new block, and the
// break stops the walk before the long one is ever compared, so a genuinely
// stale block survives.
//
// Geometry (one 4K page, so no page-boundary split interferes):
//
// S+0x00 A ──────────────────────────────────────────┐ [S, S+0x30)
// S+0x10 B ────────┐ │ [S+0x10, S+0x18)
// S+0x18 C ────────┴──────────────────────┐ │ [S+0x18, S+0x30)
// S+0x20 N ───────────────────┐ │ │ [S+0x20, S+0x30)
// S+0x28 JR ra │ │
//
// B is short because the boundary scan stops at the first address that already
// holds a compiled block (`pblock->GetFnptr() != JITCompile`) and C is compiled
// first. Compiling N then walks: C (overlaps, matches) → B (does not reach N) →
// and must keep going to reach the stale A underneath it.
//
// A goes stale only AFTER B and C compiled, so no earlier walk clears it: that
// is what makes this reachable rather than merely theoretical.
TEST(EeRecSmc, OverlapWalkScansPastNonOverlappingNeighbor)
{
EeRecTestHarness h;
SeedSkipRoutine(h, 5);
// JALR through t1 (seeded per-Run) so every entry re-dispatches through
// the LUT and the caller block itself is compiled once, as in
// MidCompileOverlapClearKeepsLutAndLinkerCoherent.
h.LoadProgram({
OR(reg::t0, reg::ra, reg::zero),
ADDIU(reg::v0, reg::zero, 0x40),
JALR(reg::ra, reg::t1),
NOP,
OR(reg::ra, reg::t0, reg::zero),
});
// 1. A, from the head: snapshotted into recRAMCopy over [S, S+0x30).
h.SetGpr64(reg::t1, kSkipRoutinePc);
h.Run(EeRecTestHarness::RunMode::FreshCache);
h.ExpectGpr64(reg::v0, 5 + 9);
// 2. C, before B, so B has something to terminate against.
h.SetGpr64(reg::t1, kSkipRoutinePc + 0x18);
h.Run(EeRecTestHarness::RunMode::PreserveCache);
h.ExpectGpr64(reg::v0, 0x40 + 4);
// 3. B — truncated at C, so it ends below N and the walk will meet it.
h.SetGpr64(reg::t1, kSkipRoutinePc + 0x10);
h.Run(EeRecTestHarness::RunMode::PreserveCache);
h.ExpectGpr64(reg::v0, 0x40 + 6); // falls through C's words to the JR
// Fixture guard: without the short B between A and N there is nothing for
// the walk to break on, and the whole test passes green-but-inert.
ASSERT_EQ(recEeBlockGuestSize(kSkipRoutinePc), 12u)
<< "A did not span the routine — fixture geometry broken";
ASSERT_EQ(recEeBlockGuestSize(kSkipRoutinePc + 0x10), 2u)
<< "B was not truncated at C — fixture geometry broken";
ASSERT_LE(kSkipRoutinePc + 0x10 + 2 * 4, kSkipRoutinePc + 0x20)
<< "B reaches N — nothing for the walk to break on";
// 4. RAW poke of A's head only (the write class no protection path sees in
// the harness). B's and C's own words are untouched, so only A is stale.
*(u32*)PSM(kSkipRoutinePc) = ADDIU(reg::v0, reg::zero, 6);
// 5. Compile N. Its walk must reach past B and clear stale A.
h.SetGpr64(reg::t1, kSkipRoutinePc + 0x20);
h.Run(EeRecTestHarness::RunMode::PreserveCache);
h.ExpectGpr64(reg::v0, 0x40 + 2);
// THE PIN: A overlaps N and is stale, so the walk must have cleared it.
{
uptr afn = 0, alut = 0;
u32 asz = 0;
EXPECT_FALSE(recEeBlockHostInfo(kSkipRoutinePc, &afn, &asz, &alut))
<< "stale A survived: the walk stopped at non-overlapping B "
"instead of scanning past it";
}
// 6. Re-enter A's head. Cleared → recompiles from current memory (6+9);
// a stale survivor still executes the pre-poke body (5+9).
h.SetGpr64(reg::t1, kSkipRoutinePc);
h.Run(EeRecTestHarness::RunMode::PreserveCache);
h.ExpectGpr64(reg::v0, 6 + 9);
}