mirror/psxrecomp
1
0
Fork 0
mirror of https://github.com/RetroPortingToolKit/psxrecomp synced 2026-10-27 10:38:46 -04:00
Code Issues Packages Projects Releases Wiki Activity

docs(gpu): correct the MMX6 widescreen comments; record what is safe to delete

Browse Source 
Comment-only. No compiled code changes -- the sole non-comment diff line moves
a trailing comment off the g_mmx6_hostside declaration into a block above it;
the declaration itself is byte-identical.

A survey of the mmx6-named widescreen symbols found the existing comments
misleading in a way that invites a dangerous cleanup, so the findings are
recorded in the source next to the code they describe.

1. "Compatibility entry points for already-generated MMX6 sources" (gpu.h) was
   wrong. These are not MMX6 compat shims and are not safe to delete. They are
   the widescreen-2D entry points the recompiler emits for ANY title with a
   [widescreen.bg2d] block, and live generated code in several non-MMX6 repos
   links them today:
     cols/startcol/startx/stream_left/stream_right
         -> CrashBash, MegaManX5, Tsumu (+ -release)
     undercap
         -> Ape, CrashBash, MMX4, MMX5, Tsumu, Vigilante8, THPS2
   Each forwards to its psx_ws_bg2d_* twin, which is the real implementation.
   Only the name is historical. Renaming is not runtime-only: the names come
   from code_generator.cpp, which is in codegen_hash_sources.cmake, so it rolls
   the overlay cache tag and forces regen + reshard of every title.

2. Host-side reveal (g_mmx6_hostside) is dead in every shipping build, which
   the previous "kept inert pending removal" note understated. Nothing sets it
   but the mmx6_reveal debug command, PSX_DEBUG_TOOLS is OFF for Release, and
   no generated source anywhere references a host-side symbol. Noted that
   removing it is the last step decoupling the generic psx_ws_bg2d_* API from
   MMX6 -- mmx6_hostside_active() is currently embedded in the ternaries of
   psx_ws_bg2d_cols/startcol/startx -- and listed the symbols that go with it.

3. BG buffer relocation is a parked FEATURE behind a load-bearing SYMBOL, a
   distinction the old comment did not draw. With g_mmx6_reloc == 0 both hooks
   return their argument unchanged, but MegaManX6Recomp/game.toml names the
   bufbase/cap instruction sites, so the emitter writes calls into generated
   code and psx_ws_mmx6_bg_bufbase is genuinely called by MMX6's generated C.
   The supportable cleanup is to gut the bodies to identity and keep the
   symbols, not to delete the functions.

Removal of (2) and (3) is deliberately deferred: it touches the renderer and
wants its own session with MMX6 revalidation.

Verified: gcc -fsyntax-only on gpu.c exits clean (an earlier draft embedded
"*/" inside a block comment and broke the parse -- caught before commit);
ctest 29/29 green.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Matthew Stanley
2026-07-27 11:46:05 -07:00
parent 73785490e7
commit 49b57f4178
2 changed files with 80 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files
runtime/include/gpu.h
+26 -1
View File
@@ -137,7 +137,32 @@ int psx_ws_bg2d_startx(int x);
int psx_ws_bg2d_stream_left(int x);
int psx_ws_bg2d_stream_right(int x);
int psx_ws_bg2d_undercap(int counter, int native_cap);
/* Compatibility entry points for already-generated MMX6 sources. */
/* MISNAMED, NOT MMX6-SPECIFIC, AND NOT SAFE TO DELETE (surveyed 2026-07-27).
*
* These read as "MMX6 compat shims" and the comment here used to say exactly
* that. They are not. They are the widescreen-2D entry points that the
* recompiler emits for ANY title with a [widescreen.bg2d] block, and live
* generated code in several non-MMX6 games links them today:
*
* psx_ws_mmx6_bg_cols/startcol/startx/stream_left/stream_right
* -> CrashBashRecomp, MegaManX5Recomp, TsumuRecomp (+ -release)
* psx_ws_mmx6_bg_undercap (below, in gpu.c)
* -> ApeEscapeRecomp, CrashBashRecomp, MegaManX4Recomp,
* MegaManX5Recomp, TsumuRecomp, Vigilante8PSXRecomp, THPS2
*
* Each one forwards to its psx_ws_bg2d_* twin above, which is the real
* implementation. Only the NAME is historical: this began as MMX6 widescreen
* work and was generalised without renaming the emitted symbols.
*
* Deleting or renaming them breaks widescreen in every repo listed above, and
* a rename cannot be runtime-only: the names are emitted by
* recompiler/src/code_generator.cpp, which is in codegen_hash_sources.cmake,
* so changing it rolls the overlay cache tag and forces a regen + reshard of
* every title. That is the reason this is still called mmx6 in 2026.
*
* Verify before touching, don't trust this comment: grep the generated trees
* of the game repos for psx_ws_mmx6_bg_undercap and see who turns up.
*/
int psx_ws_mmx6_bg_cols(int base);
int psx_ws_mmx6_bg_startcol(int col);
int psx_ws_mmx6_bg_startx(int x);
runtime/src/gpu.c
+54 -6
View File
@@ -934,10 +934,30 @@ static int ws_bg2d_left_cols(void) {
* and draw the reveal margins ourselves into the wide surface (gpu_ws_mmx6_emit_reveal).
* The stream hooks below stay active either way (they widen the tile RING, which the
* host-side decoder reads). Toggle via gpu_ws_mmx6_set_hostside(); 0 = old guest-widen. */
static int g_mmx6_hostside = 0; /* DEFAULT OFF: host-side reveal was the wrong approach
* (garbled tpage + stale early ring read). The buffer-
* relocation fix keeps the engine's correct guest-widen
* render instead. Code kept inert pending removal. */
/* DEAD IN EVERY SHIPPING BUILD -- scheduled for removal, deliberately deferred
* (surveyed 2026-07-27).
*
* Host-side reveal was the wrong approach (garbled tpage + stale early ring
* read); the buffer-relocation route kept the engine's correct guest-widen
* render instead. What the original "kept inert pending removal" note did not
* say is HOW inert:
* - nothing sets g_mmx6_hostside except the mmx6_reveal debug command
* (debug_server.c), and PSX_DEBUG_TOOLS defaults OFF for Release, so in a
* shipped build this is permanently 0 and unreachable;
* - zero generated sources in any game repo reference any host-side symbol.
*
* So mmx6_hostside_active() below is a compile-time-ish constant false, and
* every branch it guards -- including the ternaries in the supposedly generic
* psx_ws_bg2d_cols/startcol/startx -- always takes the else path.
*
* Removing it is safe and worthwhile: it is the last thing coupling the
* generic psx_ws_bg2d_* API to MMX6, so deleting it collapses those three
* functions to their real bodies. Left in place only because that touches the
* renderer and wants its own session with MMX6 revalidation, not because
* anything depends on it. Also delete gpu_ws_mmx6_emit_reveal,
* gpu_ws_mmx6_reveal_is_active, g_mmx6_clut and the mmx6_reveal handler when
* it goes. */
static int g_mmx6_hostside = 0;
void gpu_ws_mmx6_set_hostside(int on) { g_mmx6_hostside = on ? 1 : 0; }
static int mmx6_hostside_active(void) {
return g_mmx6_hostside && ws_native_wide_active() && ws_disp_w() <= 384
@@ -977,7 +997,12 @@ int psx_ws_bg2d_undercap(int counter, int native_cap) {
return counter < cap;
}
/* Compatibility for generated MMX6 sources predating the generic names. */
/* NOT MMX6-only, and NOT dead -- see the survey note on the declarations in
* runtime/include/gpu.h. Live generated code in CrashBash, MegaManX5 and Tsumu
* calls these, and seven repos call psx_ws_mmx6_bg_undercap below. The mmx6 in
* the name is historical; the recompiler emits these names for any title with
* a [widescreen.bg2d] block. Renaming means touching code_generator.cpp, which
* rolls the overlay cache tag for every game. */
int psx_ws_mmx6_bg_cols(int base) { return psx_ws_bg2d_cols(base); }
int psx_ws_mmx6_bg_startcol(int col) { return psx_ws_bg2d_startcol(col, 0x3fu); }
int psx_ws_mmx6_bg_startx(int x) { return psx_ws_bg2d_startx(x); }
@@ -1012,7 +1037,30 @@ void psx_ws_mmx6_bg_stage_init(void) {
#define MMX6_RELOC_CAP 1800 /* per-frame BG tile cap (was 1000; <= 2048) */
extern void psx_write_word(uint32_t addr, uint32_t val);
/* PARKED (default 0): relocation moves the BG packets to a larger free buffer and the
/* PARKED FEATURE, LOAD-BEARING SYMBOL -- do not "delete the dead code" here
* without reading this (surveyed 2026-07-27).
*
* The feature is off and does nothing: with g_mmx6_reloc == 0 both
* psx_ws_mmx6_bg_bufbase() and psx_ws_mmx6_bg_undercap() return their argument
* unchanged. Only the mmx6_reveal debug command can flip it, and the debug
* server is compiled out of Release.
*
* The FUNCTIONS, however, are called by real generated code and cannot simply
* be unlinked. MegaManX6Recomp/game.toml sets [widescreen.bg2d] bufbase_site
* and cap_site, config_loader feeds them to code_generator.cpp, and the
* emitter writes calls at those exact instruction addresses:
* psx_ws_mmx6_bg_bufbase -> called by MegaManX6Recomp/generated
* psx_ws_mmx6_bg_undercap -> called by SEVEN repos' generated code
* (Ape, CrashBash, MMX4, MMX5, Tsumu,
* Vigilante8, THPS2 -- see gpu.h note)
* Removing the symbols therefore means changing the emitter, and
* code_generator.cpp is in codegen_hash_sources.cmake, so that rolls the
* overlay cache tag and forces regen + reshard of every title.
*
* The supportable cleanup, when someone takes it on, is to gut the BODIES to
* plain identity and keep the symbols -- not to delete the functions.
*
* Why it is parked: relocation moves the BG packets to a larger free buffer and the
* prims DO render from there (census-confirmed), but the engine's OT chain / per-slot
* DR_TPAGE primitives are keyed to the original buffer layout, so relocated BG draws
* BLACK (wrong/absent texpage). Needs the OT-chain + DR_TPAGE relocation REd before it