From 50c730cadf044ae0ff51a58bcf043cf74718a125 Mon Sep 17 00:00:00 2001 From: Unchiga Date: Wed, 19 Aug 2026 07:58:58 -0700 Subject: [PATCH 1/2] Forward a pending load into an LWL/LWR merge LWL/LWR take their destination register as both the merge base and the destination, and they read that base late enough to receive the forwarded result of an immediately preceding load. They are therefore not subject to the MIPS-I load delay: after lw $t0, 12($a3) lwr $t0, 10($a3) the LWR merges into the word the LW just fetched, not the stale $t0. The load-delay pair emitter defers a load's writeback into a psx_ldd_ temporary, so an LWL/LWR successor was merging into cpu->gpr[rt], which still held the pre-load value. The merge operand now names the deferred temporary when the successor is an LWL/LWR writing the same register, and the ordinary writeback is suppressed there because the merge has already consumed it. Only the consumer side needs this. LWL/LWR as the producer never defer, so their writeback is already immediate. Verified against a full regeneration of a title that exercises the pair: 16 forwarding sites, and every emitted merge operand identical to the reference output built with this fix in place. Co-Authored-By: Claude Opus 5 --- recompiler/include/code_generator.h | 29 +++++++++++++++++++++++++++++ recompiler/src/code_generator.cpp | 25 ++++++++++++++++++++++--- 2 files changed, 51 insertions(+), 3 deletions(-) diff --git a/recompiler/include/code_generator.h b/recompiler/include/code_generator.h index 803a2687..2bff2727 100644 --- a/recompiler/include/code_generator.h +++ b/recompiler/include/code_generator.h @@ -479,6 +479,35 @@ private: std::string translate_lhu(uint32_t instr); std::string translate_lwl(uint32_t instr); std::string translate_lwr(uint32_t instr); + // + // LWL/LWR take their destination register as BOTH input (the merge base) + // and output. Unlike every other load, they read that input late enough to + // receive the forwarded result of an immediately preceding load, so they + // are NOT subject to the load delay: after + // + // lw $t0, 12($a3) + // lwr $t0, 10($a3) + // + // the LWR merges into the value the LW just fetched, not the stale $t0. + // When the pair emitter defers a load's writeback into psx_ldd_, it + // sets these so the LWL/LWR merge operand names that temporary instead of + // cpu->gpr[rt]. Empty temp = no forwarding in effect. + // + // (The mirror case -- LWL/LWR as the PRODUCER -- needs nothing: those never + // defer, so their writeback is already immediate.) + void set_lwlr_merge_forward(uint32_t rt, const std::string& temp) { + lwlr_merge_reg_ = rt; + lwlr_merge_temp_ = temp; + } + void clear_lwlr_merge_forward() { lwlr_merge_temp_.clear(); } + // Name to use for an LWL/LWR merge operand on register rt. + std::string lwlr_merge_operand(uint32_t rt) { + if (!lwlr_merge_temp_.empty() && rt == lwlr_merge_reg_) + return lwlr_merge_temp_; + return reg_name(rt); + } + uint32_t lwlr_merge_reg_ = 0xFFFFFFFFu; + std::string lwlr_merge_temp_; std::string translate_swl(uint32_t instr); std::string translate_swr(uint32_t instr); std::string translate_sb(uint32_t instr); diff --git a/recompiler/src/code_generator.cpp b/recompiler/src/code_generator.cpp index 073c5e22..fb61e20b 100644 --- a/recompiler/src/code_generator.cpp +++ b/recompiler/src/code_generator.cpp @@ -432,7 +432,7 @@ std::string CodeGenerator::translate_lwl(uint32_t instr) { return fmt::format("(void)psx_lwl(cpu, {}, {}, 0, 0x{:X}u);", addr, reg_name(rt), mask); } return fmt::format("{} = psx_lwl(cpu, {}, {}, {}, 0x{:X}u);", - reg_name(rt), addr, reg_name(rt), rt, mask); + reg_name(rt), addr, lwlr_merge_operand(rt), rt, mask); } std::string CodeGenerator::translate_lwr(uint32_t instr) { @@ -452,7 +452,7 @@ std::string CodeGenerator::translate_lwr(uint32_t instr) { return fmt::format("(void)psx_lwr(cpu, {}, {}, 0, 0x{:X}u);", addr, reg_name(rt), mask); } return fmt::format("{} = psx_lwr(cpu, {}, {}, {}, 0x{:X}u);", - reg_name(rt), addr, reg_name(rt), rt, mask); + reg_name(rt), addr, lwlr_merge_operand(rt), rt, mask); } std::string CodeGenerator::translate_swl(uint32_t instr) { @@ -1894,7 +1894,22 @@ std::string CodeGenerator::translate_basic_block( if (!is_cf) { if (cycle_per_insn) emit_pre_icache(addr, config_.indent); if (cycle_per_insn) emit_pre_timing(instr, config_.indent); + // If this is the successor half of a deferred load pair AND it is an + // LWL/LWR merging into that same register, hardware forwards the + // pending load into the merge (see set_lwlr_merge_forward). Point + // the merge operand at the deferred temporary before translating. + const uint32_t succ_op = instr >> 26; + const bool succ_is_lwlr = (succ_op == 0x22u || succ_op == 0x26u); + const bool forward_to_lwlr = + delayed_load_active && addr == delayed_load_addr + 4u && + succ_is_lwlr && get_rt(instr) == delayed_load_dest; + if (forward_to_lwlr) { + set_lwlr_merge_forward( + delayed_load_dest, + fmt::format("psx_ldd_{:08X}", delayed_load_addr)); + } std::string emitted = translate_instruction(addr, instr); + if (forward_to_lwlr) clear_lwlr_merge_forward(); int load_dest = simple_load_dest(instr); bool defer_load = false; if (!delayed_load_active && load_dest > 0 && addr + 4u <= block.end_addr && @@ -1918,7 +1933,11 @@ std::string CodeGenerator::translate_basic_block( } ss << emitted << "\n"; if (delayed_load_active && addr == delayed_load_addr + 4u) { - if (writes_gpr(instr, delayed_load_dest)) { + if (forward_to_lwlr) { + ss << config_.indent << fmt::format( + "/* psx_ldd_{:08X} forwarded into the LWL/LWR merge above */\n", + delayed_load_addr); + } else if (writes_gpr(instr, delayed_load_dest)) { ss << config_.indent << fmt::format( "(void)psx_ldd_{:08X}; /* successor write wins */\n", delayed_load_addr); From c77f8adcc0d038de74f05e5f6e8e063cbe1ad97d Mon Sep 17 00:00:00 2001 From: Matthew Stanley <1379tech@gmail.com> Date: Thu, 20 Aug 2026 15:08:37 -0700 Subject: [PATCH 2/2] recompiler: forward a pending load into LWL/LWR in the full-function emitter too The CFG emitter now points an LWL/LWR merge operand at the deferred psx_ldd_ temp, but full_function_emitter.cpp (BIOS / resident functions) had the identical defect and was left untouched. There, insn_reads_gpr already reports LWL/LWR as reading rt (it is the merge base), so a `lw rt, ...` followed by `lwl/lwr rt, ...` was recorded as a dependent pair and the load's writeback deferred. The successor then merged cpu->gpr[rt] -- still the PRE-load value -- and because LWL/LWR architecturally writes rt, the flush-vs-discard rule turned the deferred writeback into `(void)psx_ldd_`. So the fetched word was dropped entirely and the merge combined a stale register: silently wrong code, no diagnostic. This mirrors the CFG fix: record such pairs in ldd_lwlr_forward, exclude them from the flush/discard normalisation, rewrite the successor's single psx_old_rt merge-base initializer to read the temp, and emit a note instead of a writeback. If strict_translator's emitted shape ever stops matching, the emitter throws rather than quietly producing a stale merge. Not currently live: neither generated BIOS contains a `(void)psx_ldd_` site today, so this is latent-bug hardening. It matters because the interpreter DOES forward here (dirty_ram_interp.c's s_ld_pend LWL/LWR exemption, added after it wedged Tomba 2 at boot), so leaving the emitter unfixed keeps native and interpreted execution in disagreement -- the divergence class that produced the OpenBIOS memcard corruption. Also adds recompiler/tests/test_lwlr_load_delay_forward.py, registered in recompiler/CMakeLists.txt. It synthesises a PS-EXE with the dependent lw/lwr pair and asserts the merge operand names the temp and that the deferred load is not discarded. Verified to FAIL on master (merge operand cpu->gpr[8], plus a discard) and pass with the fix. Co-Authored-By: Claude Opus 5 (1M context) --- recompiler/CMakeLists.txt | 6 + recompiler/src/full_function_emitter.cpp | 61 +++++++++ .../tests/test_lwlr_load_delay_forward.py | 122 ++++++++++++++++++ 3 files changed, 189 insertions(+) create mode 100644 recompiler/tests/test_lwlr_load_delay_forward.py diff --git a/recompiler/CMakeLists.txt b/recompiler/CMakeLists.txt index 71c16cc0..f48b4d4a 100644 --- a/recompiler/CMakeLists.txt +++ b/recompiler/CMakeLists.txt @@ -403,6 +403,12 @@ if(BUILD_TESTING) NAME overlay_shim_compile_contract COMMAND ${Python3_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/tests/test_overlay_shim_compile.py) + add_test( + NAME lwlr_load_delay_forward + COMMAND ${Python3_EXECUTABLE} + ${CMAKE_CURRENT_SOURCE_DIR}/tests/test_lwlr_load_delay_forward.py + --recompiler $ + WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/..) add_test( NAME dirty_text_continuation_guards COMMAND ${Python3_EXECUTABLE} diff --git a/recompiler/src/full_function_emitter.cpp b/recompiler/src/full_function_emitter.cpp index c15f1da1..10d36377 100644 --- a/recompiler/src/full_function_emitter.cpp +++ b/recompiler/src/full_function_emitter.cpp @@ -498,6 +498,17 @@ bool FullFunctionEmitter::emit_function( }; /* load addr -> {dest reg, flush writeback (vs discard)} */ std::map> ldd_sites; + /* Load addrs whose dependent successor is an LWL/LWR merging into that + * same rt. LWL/LWR read their destination late enough to receive the + * forwarded result of an immediately preceding load, so hardware merges + * into the value the load just fetched -- they are NOT subject to the + * load delay. The merge base must therefore name psx_ldd_, and the + * ordinary writeback is suppressed because the merge already consumed it. + * Without this the load was deferred AND then discarded (the successor + * writes rt), so the fetched word vanished and the merge combined the + * stale pre-load register -- silently wrong code. Mirrors the CFG + * emitter's set_lwlr_merge_forward path. */ + std::set ldd_lwlr_forward; std::string unmodeled_load_delay; for (const auto& [la, lw_raw] : addr_to_raw) { int dest = simple_load_dest(lw_raw); @@ -578,6 +589,16 @@ bool FullFunctionEmitter::emit_function( } continue; } + { + const uint32_t nop_ = nx->second >> 26; + const uint32_t nrt_ = (nx->second >> 16) & 31u; + if ((nop_ == 0x22u || nop_ == 0x26u) && + nrt_ == static_cast(dep_reg)) { + ldd_lwlr_forward.insert(la); + ldd_sites[la] = {dest, false}; + continue; + } + } ldd_sites[la] = {dest, true}; } if (!unmodeled_load_delay.empty()) { @@ -613,6 +634,16 @@ bool FullFunctionEmitter::emit_function( #define out func_out for (auto& [la, s] : ldd_sites) { + if (ldd_lwlr_forward.count(la)) { + /* The LWL/LWR merge consumes the temp; no writeback and no + discard. Do NOT let the successor-writes-rt rule below turn + this into a discard. */ + std::fprintf(stderr, + "[load-delay] modeled pair: load 0x%08X rt=%d succ 0x%08X " + "LWL/LWR-forward (func 0x%08X)\n", + la, s.first, la + 4, func.entry_addr); + continue; + } uint32_t nw = addr_to_raw.at(la + 4); if (insn_writes_gpr(nw, static_cast(s.first))) { auto nsite = ldd_sites.find(la + 4); @@ -675,6 +706,12 @@ bool FullFunctionEmitter::emit_function( auto emit_ldd_flush = [&](uint32_t succ_addr) { auto it = ldd_sites.find(succ_addr - 4); if (it == ldd_sites.end()) return; + if (ldd_lwlr_forward.count(it->first)) { + out += fmt::format( + " /* psx_ldd_{:08X} forwarded into the LWL/LWR merge above */\n", + it->first); + return; + } if (it->second.second) { out += fmt::format(" cpu->gpr[{}] = psx_ldd_{:08X}; /* load-delay writeback */\n", it->second.first, it->first); @@ -1215,6 +1252,30 @@ bool FullFunctionEmitter::emit_function( out += fmt::format(" /* load-delay pair: gpr[{}] writeback deferred past 0x{:08X} */\n", ldd_sites[addr].first, addr + 4); out += fmt::format(" {}\n", tr.c_code_deferred); + } else if (ldd_lwlr_forward.count(addr - 4u)) { + /* This LWL/LWR consumes the pending load from addr-4: point its + * merge base at the deferred temp. strict_translator emits that + * base as a single psx_old_rt initializer reading the GPR, which + * still holds the PRE-load value here. If that emitted shape ever + * changes, fail the build rather than silently emit a stale merge. */ + const int fwd_rt = ldd_sites.at(addr - 4u).first; + const std::string from = + fmt::format("uint32_t psx_old_rt = cpu->gpr[{}];", fwd_rt); + const std::string to = + fmt::format("uint32_t psx_old_rt = psx_ldd_{:08X};", addr - 4u); + std::string fwd = tr.c_code; + const size_t at = fwd.find(from); + if (at == std::string::npos) { + throw std::runtime_error(fmt::format( + "cannot forward pending load 0x{:08X} into the LWL/LWR at " + "0x{:08X} (func 0x{:08X}): merge-base initializer not found", + addr - 4u, addr, func.entry_addr)); + } + fwd.replace(at, from.size(), to); + out += fmt::format( + " /* LWL/LWR merge takes the forwarded psx_ldd_{:08X} */\n", + addr - 4u); + out += fmt::format(" {}\n", fwd); } else { out += fmt::format(" {}\n", tr.c_code); } diff --git a/recompiler/tests/test_lwlr_load_delay_forward.py b/recompiler/tests/test_lwlr_load_delay_forward.py new file mode 100644 index 00000000..3e55818e --- /dev/null +++ b/recompiler/tests/test_lwlr_load_delay_forward.py @@ -0,0 +1,122 @@ +#!/usr/bin/env python3 +"""Recompiler codegen regression test: a pending load forwards into LWL/LWR. + +LWL/LWR take their destination register as BOTH the merge base and the +destination, and they read that base late enough to receive the forwarded +result of an immediately preceding load. MIPS-I therefore exempts them from +the load delay: after + + lw $t0, 12($a3) + lwr $t0, 10($a3) + +the LWR merges into the word the LW just fetched, not the stale $t0. The +interpreter already models this (dirty_ram_interp.c, the s_ld_pend LWL/LWR +exemption -- without it Tomba 2 wedged at boot), so an emitter that does not +forward puts native and interpreted execution in disagreement. + +The load-delay pair emitter defers a load's writeback into psx_ldd_. +Before the fix the LWL/LWR successor merged cpu->gpr[rt], which still held the +pre-load value, AND the deferred writeback was then discarded because the +successor architecturally writes rt -- so the loaded word vanished entirely. + +Asserts the merge operand names the deferred temp and that no discard is +emitted for that site. + +Usage: python test_lwlr_load_delay_forward.py [--recompiler ] +Exit 0 = PASS. +""" +import argparse, glob, os, re, struct, subprocess, sys, tempfile + +LOAD = 0x80010000 + +# lw $t0, 12($a3) opcode 0x23 rs=7 rt=8 imm=0x000C +LW_T0_A3_12 = 0x8CE8000C +# lwr $t0, 10($a3) opcode 0x26 rs=7 rt=8 imm=0x000A +LWR_T0_A3_10 = 0x98E8000A +JR_RA = 0x03E00008 +NOP = 0x00000000 + + +def w(words): + return b"".join(struct.pack("c\-\.]+)\s*,", src) + if not merge: + failures.append("no psx_lwr call found in the emitted body") + elif merge.group(1) != temp: + failures.append("psx_lwr merge operand is %r, expected the forwarded " + "temp %r (a stale merge base drops the pending load)" + % (merge.group(1), temp)) + + # ...and the deferred writeback must NOT be discarded: the merge consumed it. + if re.search(r"\(void\)%s\s*;" % re.escape(temp), src): + failures.append("deferred load %s is discarded; the LWL/LWR merge " + "already consumed it, so the load would vanish" % temp) + + if failures: + for f in failures: + print("FAIL: " + f) + raise SystemExit(1) + + print("LWL/LWR load-delay forwarding test passed (merge base = %s)" % temp) + + +if __name__ == "__main__": + main()