fix(vu0): pin vf0 to (0,0,0,1) in every constructed R5900Context - #187
Draft
smmathews wants to merge 1 commit into
Draft
fix(vu0): pin vf0 to (0,0,0,1) in every constructed R5900Context#187smmathews wants to merge 1 commit into
smmathews wants to merge 1 commit into
Conversation
On real VU0 hardware vf0 is hardwired to (x,y,z,w) = (0,0,0,1), the vector-unit analogue of GPR r0. R5900Context's constructor memset the struct to zero and re-seeded only a short list of fields, leaving vf0 at (0,0,0,0) with w = 0. Pin vf0 in the constructor so every default-constructed context, including the callback, interrupt, and thread contexts built across the kernel, starts hardware-correct. The runtime's own top-level constructor needed a second fix. It default-constructs its embedded m_cpuContext, which runs the new constructor pin, then immediately memsets that same member back to zero and re-seeds only r0. That memset silently cleared the pin a second time, so the primary context that actually executes recompiled VU0 macro-mode code still started with vf0.w == 0, and stayed wrong until the first VU0 microprogram ran the state restore helper. Repin vf0 there too, right after the existing r0 re-seed, mirroring how r0 is already treated at that site. Adds two regression tests: one on a bare R5900Context asserting all four vf0 lanes, independent of any runtime, and one on a freshly constructed PS2Runtime asserting the same four lanes on its own m_cpuContext. The second test is the one that actually constrains the memset bypass; deleting the repin line makes only that test's w assertion fail while the bare-context test keeps passing, proving the two tests guard different sites.
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
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.
Problem
On VU0 hardware
vf0is hardwired to (x,y,z,w) = (0,0,0,1) and always reads back thatconstant. It is the vector-unit analogue of GPR
r0.R5900Context's constructor memsets the struct to zero and then re-seeds an explicit listof fields:
vu0_q,cop0_random,cop0_status,cop0_prid,in_delay_slot,branch_pc.vu0_vf[0]is not on that list, so a default-constructed context starts at(0,0,0,0), with
wreading 0 instead of the hardware constant 1. Recompiled VU0macro-mode math that uses
vf0.was the constant-1 homogeneous source — building atranslation or bias term, for example — then reads 0. There is no crash and no assertion,
only an arithmetic result off by whatever term depended on
vf0.wbeing 1.The runtime's primary context has a second problem.
PS2Runtime::PS2Runtime()default-constructs
m_cpuContext, thenmemsets that same member back to zero andre-seeds only
r[0]. That memset clears a constructor pin, so the context that executesrecompiled VU0 macro-mode code stays wrong until a VU0 microprogram finishes and
copyVu0StateToContextrepinsvf0after the fact.Fix
ps2xRuntime/include/ps2_runtime.h— pinvu0_vf[0] = _mm_set_ps(1.0f, 0.0f, 0.0f, 0.0f)in the
R5900Contextconstructor, next to the existingvu0_qseed._mm_set_pstakesits arguments in (w,z,y,x) order, so this is x=0, y=0, z=0, w=1.
ps2xRuntime/src/lib/ps2_runtime.cpp— repeat the pin inPS2Runtime::PS2Runtime(),immediately after the
r[0]re-seed that follows thememsetofm_cpuContext. Thememset drops the constructor's pin, so the repin is necessary for the runtime's own CPU
context. It mirrors the
r0idiom on the line above it.No signature, layout, or ABI change. No recompiler change.
Testing
Two tests in the
PS2RuntimeExpansionsuite, each asserting all four lanes ofvf0:R5900Context constructor pins vf0 to hardware (0,0,0,1)— a bareR5900Context ctx;,with no runtime and no microprogram. Its
wassertion fails without the header pin. Itnever touches
PS2Runtime, so it cannot catch the memset bypass.PS2Runtime's own CPU context has vf0 pinned to hardware (0,0,0,1)— readsruntime.cpu().vu0_vf[0]on a freshly constructedPS2Runtime. This is the test thatconstrains the memset: on a tree with the header pin but without the
ps2_runtime.cpprepin, its
wassertion fails while the bare-context test still passes.Risk and not in scope
vf0's lifetime. The recompiler still emits plain writes to vector register index 0, soa later VU0 macro-mode write can clobber
vf0mid-function. Write-discard is arecompiler/codegen concern and stays out of scope. Read this as "vf0 now starts
hardware-correct", not "vf0 is hardware-correct for the life of the context".
vf0pin incopyVu0StateToContextis left untouched. It runs at theend of a VU0 microprogram, so it does not cover a context that has not run one.
R5900Contextandits constructor by name in the header, and locate the
m_cpuContextmemsetandr[0]re-seed by name inside
PS2Runtime::PS2Runtime(). Do not use line numbers.Evidence: every runtime construction site of R5900Context, every assignment to vu0_vf[0], and every memset of a context — with commands
Construction sites in the runtime (test sources excluded):
The
{}and bare-declaration rows run the constructor, so the header pin covers them(callback, interrupt, thread, and IOP-host contexts). The four
= *ctxrows are copiesand take
vf0from their source, which is the correct copy behaviour.Assignments to
vu0_vf[0]in the runtime, after this change:Line 271 is the pre-existing pin inside
copyVu0StateToContext, whichPS2Runtime::executeVU0Microprogramcalls afterm_vu0.executereturns. Lines 155 and529 are this change.
Memsets that could clear a pinned context:
m_ctxinps2_gs_gpu.cppis the GS drawing-context array, not anR5900Context. Them_cpuContextmemset is the one the second pin answers.