mirror of
https://github.com/ARMSX2/ARMSX2.git
synced 2026-08-24 16:50:16 -07:00
EE: flush the source pins before QFSRV's raw adjacent-source load
recQFSRV has a fast path for Rs == Rt+1 that reads the contiguous 256-bit
{Rt:Rs} window straight out of cpuRegs.GPR with an unaligned raw Ldr. Its
comment claimed the window was "memory-coherent after the flushes above",
but those flushes are mmiFlushReg -> _deleteEEreg, which reconciles
const-prop and the scalar/NEON slots and never touches the pins.
Under lazy-dirty the pin is authoritative for UD[0] and armStoreEERegPtrRaw
elides the canonical store entirely for a pinned lane-0 write, so a pinned
source's lower half in memory is routinely stale mid-block. Nine GPRs are
pinned, which makes four adjacent pairs both-pinned -- ($at,$v0) ($v0,$v1)
($v1,$a0) ($a0,$a1) -- plus eight more with one pinned operand: exactly the
register range a funnel-shift memcpy loop uses. Failure mode is wrong data,
not a fault.
Every other raw quad-load site fixes this by merging the pin into lane 0
after the load, which cannot work here because the read straddles two guest
registers. Flush the two pins the window covers instead -- it covers exactly
r[Rt] and r[Rt+1], since sa <= 15 over their 32 bytes -- via a new
armFlushEEGPRPin. That keeps the fast path (0-2 extra Str) rather than
falling back to the ~10-instruction temp-buffer path, and the flushed pins
stay authoritative.
This was the last raw address-of-GPR read in pcsx2/arm64/; the GE-M2e sweep
in 3bc64ac11a covered the mergeable sites and missed this one. Also fixes
the comment, which is what made the hole look deliberate.
Tests: two red-on-unfixed cases dirtying a pinned Rt and a pinned Rs, plus a
non-adjacent green control that proves the divergence belongs to the fast
path. recompiler_tests 1442/1442.
This commit is contained in:
@@ -1350,13 +1350,27 @@ void recQFSRV()
|
||||
|
||||
// Adjacent-source fast path: when Rs == Rt+1 the 256-bit
|
||||
// {Rt:Rs} window already exists contiguously in the GPR array
|
||||
// (GPR.r[Rt] immediately precedes GPR.r[Rt+1]==GPR.r[Rs], 32 bytes), now
|
||||
// memory-coherent after the flushes above. Read the unaligned 128 bits
|
||||
// directly at &GPR.r[Rt] + sa and skip the two temp stores. sa is 0..15 so
|
||||
// the load stays within the two registers' 32 bytes. Gate on Rt != 0 to avoid
|
||||
// depending on GPR.r[0] holding zero in memory (the slow path Movi's it).
|
||||
// (GPR.r[Rt] immediately precedes GPR.r[Rt+1]==GPR.r[Rs], 32 bytes).
|
||||
// Read the unaligned 128 bits directly at &GPR.r[Rt] + sa and skip the two
|
||||
// temp stores. sa is 0..15 so the load stays within the two registers' 32
|
||||
// bytes. Gate on Rt != 0 to avoid depending on GPR.r[0] holding zero in
|
||||
// memory (the slow path Movi's it).
|
||||
if (_Rt_ != 0 && _Rs_ == _Rt_ + 1)
|
||||
{
|
||||
// The flushes above do NOT make this window memory-coherent: mmiFlushReg
|
||||
// is _deleteEEreg, which reconciles const-prop and the scalar/NEON slots
|
||||
// and never touches the pins. Under lazy-dirty the pin is authoritative
|
||||
// for UD[0] and armStoreEERegPtrRaw elides the canonical store, so a
|
||||
// pinned source's lower half in memory is routinely stale here. Unlike
|
||||
// every other raw quad-load site we cannot merge after the load — the
|
||||
// read straddles two guest registers — so flush the two pins the window
|
||||
// actually covers. It covers exactly r[Rt] and r[Rt+1]: sa <= 15 over
|
||||
// their 32 bytes. Four adjacent pairs are both-pinned — ($at,$v0)
|
||||
// ($v0,$v1) ($v1,$a0) ($a0,$a1) — and eight more have one pinned
|
||||
// operand, which is the register range a funnel-shift memcpy uses. (SM-010)
|
||||
armFlushEEGPRPin(_Rt_);
|
||||
armFlushEEGPRPin(_Rs_);
|
||||
|
||||
armLoadEERegPtr(RWSCRATCH, &cpuRegs.sa);
|
||||
// Clamp sa to 0..15 before indexing host memory. MTSA masks at the
|
||||
// write, cpuRegs.sa can't hold >= 16 and this is belt-and-braces.
|
||||
|
||||
@@ -343,6 +343,19 @@ static __fi void armFlushEEGPRPins()
|
||||
armAsm->Str(pin.host, armCpuRegMem(&cpuRegs.GPR.r[pin.gpr].UD[0]));
|
||||
}
|
||||
|
||||
// Flush ONE pin mirror back to canonical memory. Not a dirty-subset heuristic
|
||||
// (see the note above) — this is for the emitter that reads a STATICALLY KNOWN
|
||||
// guest-GPR memory window raw, where neither of the two normal coherence tools
|
||||
// applies: pin substitution (armLoadEERegPtr) can't serve an unaligned read,
|
||||
// and the post-load lane merge (armMergeEEResidentIntoQuad) can't fix a quad
|
||||
// that straddles two guest registers. The caller must name every register its
|
||||
// window covers. Emits nothing when gpr is not pinned.
|
||||
static __fi void armFlushEEGPRPin(int gpr)
|
||||
{
|
||||
if (const vixl::aarch64::Register* pin = armEEPinForGPR(gpr))
|
||||
armAsm->Str(*pin, armCpuRegMem(&cpuRegs.GPR.r[gpr].UD[0]));
|
||||
}
|
||||
|
||||
// Flush only the CALLER-saved pins. Required before any C call that is
|
||||
// followed by armReloadEEClobberedPins: the reload reads canonical memory,
|
||||
// which under lazy-dirty is stale until flushed — the pair would otherwise
|
||||
|
||||
@@ -1214,3 +1214,80 @@ TEST(EeRecMmiCoherence, SqFaultPathStoresResidentQuad)
|
||||
faulted |= vtlb_IsFaultingPC(a);
|
||||
EXPECT_TRUE(faulted) << "MMIO SQ did not take the fastmem-fault/backpatch path";
|
||||
}
|
||||
|
||||
// ============================================================================
|
||||
// Raw guest-GPR memory reads vs the lazy-dirty pins (SM-010)
|
||||
// ============================================================================
|
||||
//
|
||||
// The pins (kEEPinTable) mirror GPR.r[n].UD[0] for nine hot guest regs, and
|
||||
// under lazy-dirty the PIN is authoritative: _eeStoreGPRDestReg resolves a
|
||||
// pinned dest to the pin itself and armStoreEERegPtrRaw then emits NOTHING, so
|
||||
// GPR.r[n].UD[0] in memory is stale until a seam flushes (armFlushEEGPRPins).
|
||||
//
|
||||
// mmiFlushReg is NOT such a seam — it reconciles const-prop, the scalar slot
|
||||
// and the NEON slot (_deleteEEreg) and never touches the pins. So any MMI path
|
||||
// that reads guest-GPR memory RAW, rather than via mmiLoadReg (which merges
|
||||
// through armMergeEEResidentIntoQuad) or armLoadEERegPtr (which substitutes the
|
||||
// pin), reads a stale lower half.
|
||||
//
|
||||
// recQFSRV's adjacent-source fast path (Rs == Rt+1) is exactly that: it takes
|
||||
// &GPR.r[Rt] and Ldr's 128 unaligned bits across the two registers. Four
|
||||
// adjacent pairs are BOTH pinned — ($at,$v0) ($v0,$v1) ($v1,$a0) ($a0,$a1) —
|
||||
// and eight more have one pinned operand, which is precisely the register range
|
||||
// a funnel-shift memcpy loop uses.
|
||||
|
||||
// Rt is pinned ($a0 → x27) and written earlier in the same block, so its
|
||||
// canonical memory slot is stale when QFSRV's fast path reads it.
|
||||
TEST(EeRecMmiCoherence, QfsrvAdjacentFastPathSeesDirtyPinnedRt)
|
||||
{
|
||||
EeRecTestHarness h;
|
||||
h.SetMmiPair(reg::a0, 0x1122'3344'5566'7788ull, 0x99AA'BBCC'DDEE'FF00ull); // Rt
|
||||
h.SetMmiPair(reg::a1, 0xAABB'CCDD'1122'3344ull, 0x5566'7788'99AA'BBCCull); // Rs == Rt+1
|
||||
h.SetGpr64(reg::t0, 0xF0E0'D0C0'B0A0'9080ull);
|
||||
h.LoadProgram({
|
||||
ee::MTSAB(reg::zero, 4),
|
||||
OR (reg::a0, reg::t0, reg::zero), // pinned write: pin dirty, memory stale
|
||||
ee::QFSRV(reg::v0, reg::a1, reg::a0),
|
||||
});
|
||||
h.Run();
|
||||
// {Rs:Rt} >> 4 bytes with Rt.UD[0] = the value just written.
|
||||
h.ExpectMmiPair(reg::v0, 0xDDEE'FF00'F0E0'D0C0ull, 0x1122'3344'99AA'BBCCull);
|
||||
// A stale read yields the pre-write Rt bytes: UD[0] == 0xDDEEFF0011223344.
|
||||
}
|
||||
|
||||
// Same defect through the other operand: Rs is pinned ($a1 → x21) and dirty,
|
||||
// so the upper half of the funnel result comes from stale memory.
|
||||
TEST(EeRecMmiCoherence, QfsrvAdjacentFastPathSeesDirtyPinnedRs)
|
||||
{
|
||||
EeRecTestHarness h;
|
||||
h.SetMmiPair(reg::a0, 0x1122'3344'5566'7788ull, 0x99AA'BBCC'DDEE'FF00ull); // Rt
|
||||
h.SetMmiPair(reg::a1, 0xAABB'CCDD'1122'3344ull, 0x5566'7788'99AA'BBCCull); // Rs == Rt+1
|
||||
h.SetGpr64(reg::t0, 0xF0E0'D0C0'B0A0'9080ull);
|
||||
h.LoadProgram({
|
||||
ee::MTSAB(reg::zero, 4),
|
||||
OR (reg::a1, reg::t0, reg::zero), // pinned write to Rs
|
||||
ee::QFSRV(reg::v0, reg::a1, reg::a0),
|
||||
});
|
||||
h.Run();
|
||||
h.ExpectMmiPair(reg::v0, 0xDDEE'FF00'1122'3344ull, 0xB0A0'9080'99AA'BBCCull);
|
||||
// A stale read yields the pre-write Rs bytes: UD[1] == 0x1122334499AABBCC.
|
||||
}
|
||||
|
||||
// Green control: identical dirty-pin setup, but Rs != Rt+1 so the temp-buffer
|
||||
// path runs. That path loads through mmiLoadReg, which merges the dirty pin —
|
||||
// it must stay correct both before and after the fast path is gated, proving
|
||||
// the divergence above belongs to the fast path and not to the expectations.
|
||||
TEST(EeRecMmiCoherence, QfsrvNonAdjacentPathMergesDirtyPin)
|
||||
{
|
||||
EeRecTestHarness h;
|
||||
h.SetMmiPair(reg::a0, 0x1122'3344'5566'7788ull, 0x99AA'BBCC'DDEE'FF00ull); // Rt
|
||||
h.SetMmiPair(reg::a2, 0xAABB'CCDD'1122'3344ull, 0x5566'7788'99AA'BBCCull); // Rs != Rt+1
|
||||
h.SetGpr64(reg::t0, 0xF0E0'D0C0'B0A0'9080ull);
|
||||
h.LoadProgram({
|
||||
ee::MTSAB(reg::zero, 4),
|
||||
OR (reg::a0, reg::t0, reg::zero),
|
||||
ee::QFSRV(reg::v0, reg::a2, reg::a0),
|
||||
});
|
||||
h.Run();
|
||||
h.ExpectMmiPair(reg::v0, 0xDDEE'FF00'F0E0'D0C0ull, 0x1122'3344'99AA'BBCCull);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user