Fix post-#91 master: clang-cl build, two missed guest-call sites, dropped SPU park - #92
Open
sp00nznet wants to merge 11 commits into
Open
Fix post-#91 master: clang-cl build, two missed guest-call sites, dropped SPU park#92sp00nznet wants to merge 11 commits into
sp00nznet wants to merge 11 commits into
Conversation
Two pre-existing breakages that only MSVC tolerated. Both were found and fixed locally in the Rubber Ducky port's checkout months ago (its README lists them under "research-branch fixes carried in this port") but never pushed upstream, so a fresh clang-cl build of master still fails. 1. cellSpurs.c, rsx_commands.c and sys_timer.c call getenv() without <stdlib.h>. clang-cl makes the implicit declaration an error; MSVC only warns (C4013) and then silently truncates the returned char* to int -- a real 64-bit bug on the MSVC path too, not just a clang pedantry. 2. spu_workload.c re-declared spu_run_lifted_job_abi as a local extern, which conflicts with the static inline that spu_lifted_job.h (already included at the top of the file) provides. Dropped the redeclaration. Verified: clang-cl + Ninja Release build of ps3recomp_runtime is clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
#86 widened ppu_guest_call / the g_ps3_guest_caller hook from r3-r6 to r3-r10 and updated every call site that existed on its base. Two sites added later by the integration chain were not on that base and were left at five arguments: runtime/ppu/ppu_hle.cpp:407 ppu_guest_call(iopd, desc, 1, 8, 0x398) lbp/main.cpp:174 harness_guest_caller + its local typedef/extern The lbp one is the worse of the two: it re-declared ps3_guest_caller_fn locally with five parameters and installed a five-parameter function as the hook, so a nine-argument call through g_ps3_guest_caller was an ABI mismatch rather than a compile error. Neither showed up in the #91 verification because ppu_hle.cpp and lbp/main.cpp are per-title files, not part of the ps3recomp_runtime library or the sync_stress target. Found by building the Rubber Ducky port against master. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…dation eeff394 added spu_context.park_on_empty_inmbox: a rchcnt(SPU_RdInMbox) that finds the inbound mailbox empty halts the SPU instead of returning 0, so a persistent-worker raw SPU can run SYNCHRONOUSLY -- full init + ready-mailbox handshake, then park -- with no async host thread racing the PPU. 6dd8524 ("fold: align runtime/spu with sagemono's consolidated structure") dropped both the field and the rchcnt behaviour. That silently un-fixed the Rubber Ducky AsyncCopy SPU, whose determinism it was written for (that port went from a run-to-run-varying abort to a stable ~5160-line run because of it), and left the port failing to compile against master. Restored into the consolidated spu_channels.c/spu_context.h. The same commit's ppu_loader diagnostics (RD_FORCE_ADDR read override) are still missing; left out here since they are debug-only. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
sp00nznet
changed the title
…fix) 6bc76ae restored three of the fixes the faithful-adopt merge dropped when it took sagemono's ppu_lifter.py wholesale (~835 lines of the ydkj side discarded). This is a fourth, from 27982e0. discover_jump_tables anchors on an indexed `lwzx rEnt,rIdx,rBase`. gcc's 64-bit offset-table idiom materialises the element address first: sldi rIdx, idx, 2 # rldicr rIdx,idx,2,61 lwz rBase, d(r2) add rT, rIdx, rBase lwz rEnt, 0(rT) # <-- no lwzx extsw rEnt, rEnt add r0, rEnt, rBase mtctr r0 ; bctr With no lwzx in the window, discovery bailed and the switch lifted to a bare ps3_indirect_call -- at runtime an "unresolved indirect call" that returns without dispatching. 27982e0 synthesised the equivalent lwzx operands from the `add rT,idx,base; lwz rEnt,0(rT)` pair so the existing base/offset/count logic applies unchanged; that block is restored verbatim. On Rubber Ducky (8530 functions, unchanged detection): jump-table dispatchers 204 -> 252 (+48, matching 27982e0's original +48) case targets 2042 -> 2458 and _jsCountFloatsInCgType's 110-case switch -- in the middle of the Cg shader loader that port is blocked on -- decodes again. Also decode lvxl/stvxl as lvx/stvx (identical apart from a cache LRU hint); they were emitting a silent no-op `.word` at 4 sites in the same binary. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Owner
Author
Result on Rubber Ducky: the Cg shader wall is goneThe 4th dropped fix (
Lift: 204 → 252 dispatchers, 2042 → 2458 case targets (detection unchanged at 8530 functions, so this is purely codegen). Run, same binary and assets:
It now runs to the 150s bound instead of aborting. New park point is |
… restore RD_FORCE_ADDR Three fixes found driving the Rubber Ducky demo past its Cg wall. 1. spu_dispatch_frame_by_queue seeded the per-frame work-descriptor EA into the SPU's inbound mailbox, on the assumption that a persistent-worker sim SPU reads it with `rdch SPU_RdInMbox`. At least one image uses `rchcnt SPU_RdInMbox` the OTHER way round: the solver's send-result helper (LS 0xC370) returns EBUSY when the inbox is NON-empty and only falls through to `wrch WrOutMbox; stop 0x110` when it is EMPTY. Seeding made that check fail every pass, so the worker never signalled completion and spun -- 3,000,000+ interpreted steps with no progress, and the PPU (which dispatches synchronously on its own thread) deadlocked against it. Default is now no seed; RD_SPU_FRAME_MBOX=1 restores the old behaviour for an image that really does rdch its work EA. 2. group_start reported "no fallback for any of N thread(s)" whenever it spawned no host threads -- but `instant` counts BOTH no-fallback threads AND threads the interpreter ran to completion synchronously. A perfectly working interpreted run therefore logged as "no fallback", which is how the Rubber Ducky sim SPUs looked broken while they were in fact running. Count the two separately and say which. 3. The frame path never logged a completion line, so a hung re-run was indistinguishable from one that returned. Mirrored the group_start print. Also restores eeff394's RD_FORCE_ADDR/RD_FORCE_VAL read override in vm_read32 (dropped by the same consolidation as park_on_empty_inmbox), and gives the read32 [HOTREAD] spin print the guest cia/lr that the read8 one already had -- that is what named TsimpleTexture::load and _jsFlipCallbackFunction here. Verified on Rubber Ducky: the solver's per-frame dispatch now completes (stop=0x110, 531 insns) instead of spinning, a second sim SPU thread comes up behind it, and the demo advances past TclMultiSim::initialize. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The transient-open retry exists for a real Windows failure mode (an AV/indexer share lock on a file that DOES exist), but it had no gate: any failed read-only open burned 3000 x Sleep(1) before giving up. Sleep(1) is 1-15ms, so a path that simply does not exist cost 3-45 SECONDS of dead wall-clock per probe. That is what made the Rubber Ducky demo look wedged at "Loading cviewer scene": libxml2 probes /etc/xml/catalog there, which is absent by design, and every probe stalled the guest's main thread for tens of seconds. ENOENT is never transient. A share-lock/handle-pressure failure reports EACCES/EBUSY, so the recovery this loop was written for still works. Also adds an (YDKJ_MBOXTRACE-gated) trace of the SPU->PPU mailbox delivery DECISION, not just successful deliveries -- a per-title hook that drops a mailbox class was otherwise indistinguishable from an SPU that never wrote one. Verified on Rubber Ducky: the scene finishes loading (all wall/tub/towel textures + lightmaps), "init done", and the demo enters its frame loop. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
NV4097_SET_VERTEX_DATA_ARRAY_FORMAT packs [3:0]=type, [7:4]=size,
[15:8]=stride, [31:16]=frequency. Size 0 means "this array is off"; the parser
was keying `enabled` off `type != 0` instead.
PSGL writes a bare type word (0x00000002 = float32, size 0, stride 0) into every
array slot it is NOT using, so 13 of the 16 slots came back enabled with zero
components and zero stride. The input layout was then built from a mostly
degenerate attribute set.
Confirmed against the Rubber Ducky demo's own FIFO (YDKJ_RSXTRACE): slots 0/2/13
carry real formats (0x00012032 = float32 x3, stride 32) while 1,3,4,5,6,7...
carry 0x00000002. With the fix the layouts the draws actually use are
attrib[0] float32 x3 stride=12, attrib[2] float32 x3 stride=12,
attrib[13] float32 x3 stride=52
and the fetched positions are real mesh coordinates (-1.45, 0.0059, 1.49 ...)
rather than a degenerate set.
Also makes three fixed debug caps env-tunable, because each was spent entirely on
boot-time traffic and so never showed the draws worth looking at:
YDKJ_RSXTRACE=<N> method trace depth (was a fixed 250)
RSX_VTXDBG=<N> vertex-layout prints (was a fixed 5)
VTX_POS=<N> first fetched position per draw (new; distinguishes
"vertex array resolved to the wrong memory" from
"the shader is wrong", which look identical otherwise)
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Follow-on to a0bca37, from bisecting a title that draws valid geometry to a correct render target and produces zero fragments. TCONST_MAX=<N> raises the transform-constant trace cap (was a fixed 64, all spent on boot-time uploads, so the constants the real draws use were never seen). With it: this title's MVP lands at c260..c263, well outside the slot 12..30 window the default filter watches. VP_MVP=<N> dumps the per-draw constant bank AS THE SHADER WILL SEE IT, read back from the mapped vp_cb slot. A snapshot taken from a stale rsx_state is indistinguishable from a broken vertex program without this -- both give zero fragments. Neither changes behaviour; both are off unless the env var is set. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`render_frame` gates the ENTIRE vertex-program draw pass on s_d3d.vp_ready:
if (s_d3d.vp_ready && s_d3d.draw_count > 0) { ... draw every is_vp record ... }
but vp_ready is only set by compile_vp(), whose trigger ran at frame end and read
s_d3d.current_rsx_state->vp_ucode_bytes. By then that state no longer carries the
microcode, so for a title that only ever compiles per-draw vertex shaders the base
pipeline was never built -- and every is_vp draw was silently dropped. Draws
recorded, constants snapshotted, per-draw VS compiled and cached, and not one of
them submitted to the GPU. From outside this is indistinguishable from a broken
vertex program: valid vertex data, valid MVP, correct render target, zero
fragments.
Build it from vp_get_vs() instead, which runs at RECORD time with a live state
that definitely has microcode. Also drops the trigger's "microcode size differs
from last compile" clause: while !vp_ready there is nothing to be stale against.
On the Rubber Ducky demo this is the difference between a boot that stalls after
two simulation frames and a continuous frame loop -- 8096 presents carrying 287
draws each, where before the pass never executed.
Adds VP_BYPASS=1 (replace the guest transform with a direct attribute-0 -> clip
space map) to separate "the transform is wrong" from "the geometry never reaches
raster", which otherwise look identical.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A mis-decoded NV4097_SET_COLOR_MASK makes geometry rasterize correctly and write nothing, which from the outside is identical to the draw never happening. Env- gated override so that hypothesis can be tested in one run instead of inferred. (Tested on the Rubber Ducky demo: not the cause there -- its draws record cmask 0xF from color_mask 0x01010101 -- but the check cost one run instead of a reading session, and the next blank-output title gets it for free.) Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ckbuffer current_rt_off() classified any surface whose colour offset was not one of the registered cellGcmSetDisplayBuffer offsets as OFFSCREEN. But SetDisplayBuffer only registers the buffers a FLIP may point at; a guest is free to render into a different surface of the same size and flip to it later, and PSGL does exactly that -- the Rubber Ducky demo registers 0xDF80000 / 0xDBC0000 and renders every frame to 0xE340000. The consequence was total, and invisible from every downstream check: [PRESENTGATE] records=29 onscreen=0 offscreen=29 has_display=0 -> render_frame=SKIPPED has_display gates render_frame(), which is what submits ALL recorded geometry. With every record misfiled as offscreen it was never called, so not one draw reached the GPU -- while draw records, vertex uploads, constant snapshots and per-draw shader compiles all completed normally. Vertex data, input layout, MVP constants, depth state, cull mode and colour mask each verify as correct in that state, which is what makes this so hard to see: the whole pipeline is right and nothing is drawn. With RT_DISPLAY_BY_SIZE=1 the same run submits its draws (`[VPSUBMIT] draw[0] verts=128 pso=guest-fp rt=0`) and the demo's geometry rasterizes to the backbuffer for the first time. Env-gated rather than unconditional: keying "is this the display" off the clip size is a heuristic, and a title that renders a full-size intermediate surface would be misfiled the other way. The real fix is to track the surface the guest flips to; this makes the failure reproducible and gives titles a way through in the meantime. Also adds VP_SUBMIT=<N>: prints the present-time gate decision and the first N DrawInstanced calls the VP pass actually issues. "Records exist" and "draws were submitted" are different claims -- conflating them cost this investigation several wrong turns. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Four fixes needed to build and run a real title (the Rubber Ducky port) against
masterafter #91. Two are build breaks, two are behavioural regressions thefold itself introduced.
1. clang-cl build breaks (pre-existing)
getenvwithout<stdlib.h>inlibs/spurs/cellSpurs.c,libs/video/rsx_commands.c,runtime/syscalls/sys_timer.c. clang-cl errors on the implicit declaration; MSVConly warns (
C4013) and then silently truncates the returnedchar*toint—a live 64-bit bug on the MSVC path, not clang pedantry.
externforspu_run_lifted_job_abiinruntime/spu/spu_workload.c,conflicting with the
static inlineinspu_lifted_job.hthat the same filealready includes.
Both were fixed locally in the Rubber Ducky checkout months ago (its README lists
them under "Research-branch fixes carried in this port") and never pushed upstream.
2. Two guest-call sites #86's r3–r10 widening missed
#86widenedppu_guest_call/ theg_ps3_guest_callerhook to nine arguments andupdated every call site on its base. Two sites added later by the integration chain
were not on that base:
runtime/ppu/ppu_hle.cpp:407—ppu_guest_call(iopd, desc, 1, 8, 0x398)lbp/main.cpp:174—harness_guest_callerplus a local 5-paramps3_guest_caller_fntypedef and
extern. This one is worse than a compile error: it installed afive-parameter function as the hook, so every nine-argument call through
g_ps3_guest_callerwas an ABI mismatch.These did not show up in #91's verification because
ppu_hle.cppandlbp/main.cppare per-title files — not part of
ps3recomp_runtimeor thesync_stresstarget.3.
park_on_empty_inmboxdropped by the runtime/spu consolidationeeff394addedspu_context.park_on_empty_inmbox, letting a persistent-worker rawSPU run synchronously (full init + ready-mailbox handshake, then park) instead of on
an async host thread racing the PPU.
6dd8524("fold: align runtime/spu withsagemono's consolidated structure") dropped both the field and the
rchcntbehaviour,silently un-fixing the determinism it was written for. Restored into the consolidated
spu_channels.c/spu_context.h.The same commit's
RD_FORCE_ADDRread-override diagnostic inppu_loader.cppisstill missing; left out here since it is debug-only.
Verification
ps3recomp_runtime.liblinks clean under clang-cl 21.1.8 (Ninja, Release).reaching the same point as its last known-good build.
🤖 Generated with Claude Code