fix(vu1): give both halves of a reordered instruction pair the pre-pair VF and Q - #200
Draft
smmathews wants to merge 2 commits into
Draft
fix(vu1): give both halves of a reordered instruction pair the pre-pair VF and Q#200smmathews wants to merge 2 commits into
smmathews wants to merge 2 commits into
Conversation
vuLowerVfReadWriteMasks stopped at MFP and reported no read or write for the entire EFU opcode block (ESADD through EEXP), even for ELENG, ERLENG and ERCPR, which this interpreter already implements as reading VF[fs]. Add the thirteen EFU cases so the pair reorder predicate can see the hazard; WAITP and the two unallocated EFU slots stay excluded since they take no vector operand. The read is reported regardless of whether an opcode has an execution body today, so implementing one later cannot silently reopen the gap the table exists to close.
Both halves of a VU1 instruction pair issue concurrently, so both must
observe the register state that predates the pair, and a VF write
collision resolves in favour of the upper half for the whole register,
not just the fields it names (VU User's Manual, section 3.4.3,
"Priority for Writing to a Register"). The lowerBeforeUpper branch of
run() previously ran the lower half first only to satisfy the
lower-reads-upper direction of the hazard; the upper half still saw
whatever the lower half had just written, and any lower write that
collided with the upper half's destination survived on the fields the
upper half did not name.
Snapshot the VF registers the upper half's fs, ft and destination touch
before the lower half runs, restore them for the upper half, and
re-apply the lower half's write afterward everywhere except the upper
half's own destination register. Snapshot and restore Q the same way:
DIV, SQRT and RSQRT write Q from the lower half and the upper half's
q-form operations read it, so the reorder fed the upper half a Q the
lower half had just produced. No upper instruction writes Q, so Q is
restored unconditionally and needs no priority arbitration. Q is the
only register outside the VF file where this arises - no lower
operation writes ACC or I, and P and R are read only by lower
operations, of which a pair holds exactly one.
Section 3.4.1 ("Hazards") exempts ACC, I, Q, P and R from hazard
generation, but that exemption is about interlocks, not about which
value a concurrently issued instruction observes; the same paragraph
points at WAITQ/WAITP for synchronising Q and P. Backed further by
section 3.4.2 ("Upper Instruction and Lower Instruction", concurrent
issue) and section 3.4.4 ("FMAC Pipeline", no intra-pair dependency
stalls).
Also classify the four R-register lower operations the hazard table
still left at its default arm: RNEXT and RGET write VF[ft], RINIT and
RXOR read VF[fs]. None has an execution body in this tree, so nothing
observable changes today; leaving them unclassified would have let an
implementation of RGET or RNEXT bypass both the reorder predicate and
the write-priority machinery. RGET's own instruction page is where the
manual states that whole-register discard rule for a lower operation.
That completes the lower-special selector space: every selector the
manual allocates that names a VF operand is now reported, and the ones
that name only integer registers or no operand are pinned absent.
One direction of this change removes a write that previously landed: a
reordered pair whose lower half wrote the upper half's destination now
has that write discarded in whole-register units, including fields the
upper half did not touch itself.
Inter-pair Q and P visibility latency is unchanged and remains out of
scope. This tree still makes a Q or P write visible immediately rather
than at the end of the FDIV or EFU pipeline.
Contributor
Author
|
(I understand you're re-working vu1, will leave in draft) |
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.
fix(vu1): give both halves of an instruction pair the pre-pair VF and Q values
Problem
decodeInstructionPairalready reorders a pair when the upper half writes aVF register the lower half reads or writes, so the lower half sees the
pre-pair value. The gaps below remain.
vuLowerVfReadWriteMasks(
ps2xRuntime/src/lib/vu/ps2_vu1_detail.h) fell through todefault: return;for the EFU block (ESADDthroughEEXP) and theR-register block. The gap is live:
ELENG,ERLENGandERCPRreadVF[fs]here, so a pair whose upper half wrote that register neverreordered, and the EFU operation read the overwritten value.
read the pre-pair register file, so no order of
execLower/execUpperserves both.
Qhas the same shape:DIV/SQRT/RSQRTwrite it from thelower half and
q-form upper operations read it, so the reordered branchfed the upper half a
Qthe lower half had just produced.Qis the onlyregister outside the VF file where this arises — no lower operation writes
ACCorI, andPandRare read only by lower operations, of which apair holds one.
resolved per output field, not per register.
Fix
ps2_vu1_detail.h— classify the EFU cases (vuSetRegBit(readMask, is))and the R-register cases:
RNEXT/RGETwriteVF[ft],RINIT/RXORreadVF[fs]. Every allocated lower-special selector naming a VF operand is nowreported. Opcodes are classified whether or not their execution arm does
anything, so implementing one later cannot reopen the gap.
WAITPand theunallocated slots
0x77/0x7Fname no vector operand and stay out.ps2_vu1_core.cpp— in thelowerBeforeUpperbranch, snapshot the VFregisters named by the upper half's
fs,ftand destination before thelower half runs. Restore them for the upper half, then re-apply the lower
half's write everywhere except the upper half's own destination.
Qissnapshotted and restored the same way, unconditionally: no upper instruction
writes it.
vuLowerVfWriteMaskandvuAddGuardedReg, besidevuSetRegBit.Hardware basis
VU User's Manual, section 3.4 ("Pipeline Operation");
EVIDENCE.mdcarriesthe full quotations.
ACC,I,Q,PandRgenerate no datahazards, and
VF00is a constant register outside hazard checks. The sectionadds: "It is possible to make the Q and P registers synchronize with each
other by using the WAITQ/WAITP instruction." The exemption covers
interlocks, not what a concurrently issued instruction observes.
concurrently; both pipelines stall on a hazard to either.
write collision, in register units: "the result of the Lower instruction is
discarded even when the Upper and Lower instructions write data to different
fields".
halves of one instruction.
DIV/SQRT/RSQRTwriteQback at thestage ending the FDIV pipeline, after the issuing pair has read its
registers.
RGET/RNEXTinstruction pages, Remarks — an upper-half write toVF[ft]in the same cycle discards this instruction's result regardless offield overlap.
Testing
Tests are in
ps2xTest/src/ps2_vu1_tests.cpp, in the existingMiniTest::Case("PS2VU1")block, built through the real encode/decode path.They cover both read directions and the
Qread, whole-register discardthrough the
fddestination in the shape of section 3.4.3's worked example(with
vf3where the manual writesVF23) and through the upper-specialftdestination, the destination guard alone, and an upper half whose
fsandftname one register the lower half writes. They also assert the classifier's
reads, writes and unclassified selectors, so the completeness claim above is
asserted, not inspected.
cmake -S . -B build -G Ninja -DCMAKE_BUILD_TYPE=Release -DCMAKE_C_FLAGS=-msse4.1 -DCMAKE_CXX_FLAGS=-msse4.1 cmake --build build -j 12 ./build/ps2xTest/ps2x_testsEVIDENCE.mdhas the selector tables, fail-before/pass-after values, themutation table, and the disclosures.
Risk and not in scope
whole-register units. A reordered pair whose lower half writes the upper
half's destination now loses that write on fields the upper half does not
name and previously left alone. It is the only place where this change
removes a value rather than reading one earlier.
predicate rejects, LOI pairs, and reordered pairs whose lower half writes
neither a VF register nor
Q. In the earlier-value half rather than thebit-identical one: a reordered
DIV,SQRTorRSQRTpair, whose upper halfnow reads the pre-pair
Q, and a pair this change newly reorders onto anELENG,ERLENGorERCPRlower half, whose EFU operation now reads thepre-pair
VF[fs]. Everything else this change reclassifies has an empty orabsent execution arm in this tree, the R-register block included, so its
classification changes nothing observable today.
DecodedInstructionPairis not widened andps2xRuntime/include/runtime/ps2_vu1.his untouched, so nothing outside theinterpreter is affected.
Q/Pvisibility latency is unchanged and still wrong. Thistree makes a
QorPwrite visible immediately; hardware makes it visibleonly at the end of the FDIV or EFU pipeline. That window is the same width
after this change as before it; closing it belongs with whichever change
introduces a latency model.
WAITQ/WAITPsequencing, no EFU opcode bodies.Nothing here reads or writes a cycle counter;
EVIDENCE.mdrecords why eachis out of scope and the state of every EFU execution arm.
ps2_vu1_core.cppandps2_vu1_tests.cpp, which other VU1 work inthis series also touches; expect to rebase this branch if that work lands
first. Within this file,
EVIDENCE.md§7 gives the command that checks no testname collides.
Evidence — probe output, mutation table, mirror sweep, and reproduction commands
Evidence
1. Defect reproduction
Probe over the lower-special selectors this change touches, run from the clone
root against the tree before this change (
HEAD~2) and after it. No projectbuild needed.
is = 3gives read bit0x00000008;it = 5gives write bit0x00000020.Before — every selector reports neither read nor write, including
0x72(
ELENG),0x73(ERLENG) and0x7A(ERCPR), which this interpreteralready executes as reading
VF[fs]:After —
RNEXT/RGETreport theVF[ft]write, the EFU selectors andRINIT/RXORreport theVF[fs]read,WAITPstill reports neither:2. Selector coverage
Selectors use the interpreter's own
(instr & 0x3) | ((instr >> 4) & 0x7C).Mnemonic lines are the manual's. "Execution arm" is the opcode's state in
ps2_vu1_lower.cpp, read back from the source by the command below. A selectorthis change reclassifies but which the output does not list has no arm at all.
EFU block — every operation takes
VF[fs]as its only vector operand andwrites
P:ESADD P, VF[fs]0x70VF[fs]return;ERSADD P, VF[fs]0x71VF[fs]return;ELENG P, VF[fs]0x72VF[fs]PfromVF[fs]ERLENG P, VF[fs]0x73VF[fs]PfromVF[fs]EATANxy P, VF[fs]0x74VF[fs]EATANxz P, VF[fs]0x75VF[fs]ESUM P, VF[fs]0x76VF[fs]ESQRT P, VF[fs]fsf0x78VF[fs]ERSQRT P, VF[fs]fsf0x79VF[fs]ERCPR P, VF[fs]fsf0x7AVF[fs]PfromVF[fs]ESIN P, VF[fs]fsf0x7CVF[fs]EATAN P, VF[fs]fsf0x7DVF[fs]return;EEXP P, VF[fs]fsf0x7EVF[fs]0x7Breturn;0x770x7FR-register block:
RNEXT.dest VF[ft]dest, R0x40VF[ft]return;RGET.dest VF[ft]dest, R0x41VF[ft]return;RINIT R, VF[fs]fsf0x42VF[fs]return;RXOR R, VF[fs]fsf0x43VF[fs]return;Remaining lower-special selectors: integer registers or no operand,
unclassified, absence asserted by
lower-special ops that name no VF operand report neither read nor write:0x3B0x3E0x3F0x680x690x6C3. Build and test
cmake -S . -B build -G Ninja -DCMAKE_BUILD_TYPE=Release -DCMAKE_C_FLAGS=-msse4.1 -DCMAKE_CXX_FLAGS=-msse4.1 cmake --build build -j 12 ./build/ps2xTest/ps2x_testsps2x_testsstatically linksps2_analyzer_lib, so-DPS2X_BUILD_ANALYZER=OFFmust never be passed to the configure step — it breaks the test link.
ctestregisters no tests in this project; the binary's own summary line is the source
of truth.
4. Fail-before / pass-after
Each row fails on the tree before this change with the base-tree value shown
and passes after. Base-tree values come from running the fixture with the tests
added and no part of the fix applied.
EFU lower op reads the pre-pair VF value the upper half overwritesstate().p52.013.0upper half reads the pre-pair value of an fs operand the lower half overwritesvf5(110, 220, 330, 440)(11, 22, 33, 44)upper half reads the pre-pair value of an ft operand the lower half overwritesvf5(100, 400, 900, 1600)(2, 8, 15, 40)upper half write discards the lower half write to the same register in whole-register unitsvf1(11, 22, 3, 777)(11, 22, 3, 4)upper-special ft destination discards the lower half write to that registervf4(1, 2, 7, 8)(1, 2, 9, 9)upper half destination is guarded even when it is not one of its own operandsvf1(110, 220, 7, 8)(110, 220, 3, 4)upper half reads the pre-pair Q value the lower half overwritesvf3(12, 12, 12, 12)(108, 108, 108, 108)upper half with fs and ft aliased to one register keeps the lower half's write to itvf5(200, 400, 600, 800)(2, 4, 6, 8)The remaining new tests, and what each does on the tree before this change:
EFU lower ops report their fs read and WAITP reports noneR-register lower ops report their VF operandEFU pair with no VF write leaves the reordered branch behaviourally unchangedERLENGreads the post-writevf3:state().pis0.019231(1over52) against an expected0.076923(1over13). Thevf3write is(12, 16, 48, 4)before and after.lower-special ops that name no VF operand report neither read nor write5. First commit stands alone
The classifier widening and its tests were built and run with
ps2_vu1_core.cppat its pre-change state. The build succeeded and every testpassed, including the EFU tests: the classifier change is observable end-to-end
through
state().pwithout any change torun().6. Mutation table
Each mutation was applied alone to the fixed tree, rebuilt, and run with the
§3 command; the listed tests fail and the rest of the suite stays green. The
tree was restored between rows with
git checkout <file> && touch <file>, soninja would not skip a stale object.
P2/P3andP6/P7are mirrors, so aone-sided fix is still caught.
P18,P19andP20each add a bit to an armfrom the instruction field that arm does not name; the classifier tests encode a
non-zero
isand a non-zeroittogether so that such a bit lands on a registerindex the assertion can see.
case 0x72: // ELENGfrom the EFU armEFU lower op reads the pre-pair VF value the upper half overwrites;EFU lower ops report their fs read and WAITP reports nonecase 0x7E: // EEXPfrom the EFU armEFU lower ops report their fs read and WAITP reports nonecase 0x7B:to the EFU armEFU lower ops report their fs read and WAITP reports nonevuAddGuardedReg(..., FS(decoded.upper), ...)callupper half reads the pre-pair value of an fs operand the lower half overwritesvuAddGuardedReg(..., FT(decoded.upper), ...)callupper half reads the pre-pair value of an ft operand the lower half overwritesif (guarded[i] != upperWriteReg)toif (true)upper half write discards the lower half write to the same register in whole-register units;upper-special ft destination discards the lower half write to that register;upper half destination is guarded even when it is not one of its own operandsiftoif (false)upper half reads the pre-pair value of an fs operand the lower half overwrites;upper half reads the pre-pair value of an ft operand the lower half overwrites;upper half with fs and ft aliased to one register keeps the lower half's write to itvuAddGuardedReg(..., upperWriteReg, ...)callupper half destination is guarded even when it is not one of its own operandsreturn ft;toreturn 0u;invuUpperVfWriteReg's upper-special switchupper-special ft destination discards the lower half write to that registerprePairsnapshot loop to immediately afterexecLowerupper half reads the pre-pair value of an fs operand the lower half overwrites;upper half reads the pre-pair value of an ft operand the lower half overwrites;upper half write discards the lower half write to the same register in whole-register units;upper-special ft destination discards the lower half write to that register;upper half destination is guarded even when it is not one of its own operands;upper half with fs and ft aliased to one register keeps the lower half's write to itcase 0x73: // ERLENGfrom the EFU armEFU pair with no VF write leaves the reordered branch behaviourally unchanged;EFU lower ops report their fs read and WAITP reports nonem_state.q = prePairQ;upper half reads the pre-pair Q value the lower half overwritesm_state.q = postLowerQ;upper half reads the pre-pair Q value the lower half overwritescase 0x41: // RGETfrom the R-register armR-register lower ops report their VF operandcase 0x42: // RINITfrom the R-register armR-register lower ops report their VF operandcase 0x6C:to the R-register read armlower-special ops that name no VF operand report neither read nor writeif (regs[i] == reg)invuAddGuardedRegtoif (false)upper half with fs and ft aliased to one register keeps the lower half's write to itvuSetRegBit(writeMask, it);to the EFU armEFU lower ops report their fs read and WAITP reports nonevuSetRegBit(readMask, is);to theRNEXT/RGETarmR-register lower ops report their VF operandvuSetRegBit(writeMask, it);to theRINIT/RXORarmR-register lower ops report their VF operand7. Duplicate test-name check
Produces no output, so no case in
ps2xTest/src/ps2_vu1_tests.cppis droppedby the
std::mapthat keys the harness's test registration.8. Mirror sweep
EFU lower op reads the pre-pair VF value the upper half overwrites, and the pre-existinglower side sees old VF value when upper writes the same registerupper half reads the pre-pair value of an fs operand the lower half overwritesand itsftvariantEFU lower op reads the pre-pair VF value the upper half overwritesWAITPand the unallocated EFU slots; behaviourally unreachable-by-proof, since none of them writes any state —WAITP's arm is a barereturn;and the unallocated slots have no arm at all, per §2 — so no state differs between ordersfsftfs == ft)upper half with fs and ft aliased to one register keeps the lower half's write to it. The de-duplication invuAddGuardedRegis what makes the restore replay one entry instead of two, and P17 is its mutationfd(ordinary upper ops)ft(ITOF*/FTOI*/ABS)upper-special ft destination discards the lower half write to that registervuUpperVfWriteRegreturns0, so the predicate never enters the changed branchABS.xyover a full-widthMOVEADD.xyoverMOVE.w, whose written fields are wholly disjoint; and thez/wfields ofADD.xyover a full-widthMOVE, which overlaps onx/yand is disjoint on the fields the assertions namerun()testsEFU pair with no VF write leaves the reordered branch behaviourally unchanged, plus the pre-existingSQhazard test staying greenrun()takenlowerBeforeUpperiBit)lowerBeforeUpperis computed as!iBit && ..., so an LOI pair can never enter the changed branchELENG,ERLENG,ERCPR)ELENGandERLENG;ERCPRasserted at the classifier only — openWAITPand the unallocated slotsvf0as a guarded registervuSetRegBitnever sets bit 0, andvuAddGuardedRegseparately rejectsreg == 0run()test aboveQupper half reads the pre-pair Q value the lower half overwritesIIisLOI, which is theiBitbranch and can never enter the reordered branchACCACC, so no lower half can clobber it inside a pairPPis written by EFU lower operations and read only by the lower operationMFP; a pair holds one lower instruction, and no upper instruction readsPRP: written and read only by lower operationsQ/Pvisibility latencyRNEXT/RGET(VF[ft]write)RINIT/RXOR(VF[fs]read)WAITPand the unallocated EFU slots are covered by axis I9. Hardware citations
Source: Sony VU User's Manual, version 6.0, section 3.4 ("Pipeline Operation")
except where noted. Each citation was read from a rendered page image rather
than extracted text.
special registers
ACC,I,Q,PandR; the same paragraph continues:"It is possible to make the Q and P registers synchronize with each other by
using the WAITQ/WAITP instruction." Separately,
VF00is a constant registerand is not subject to hazard checks. The exemption is about interlocks, not
about which value a concurrently issued instruction observes.
Lower instructions are issued concurrently, so both instruction pipelines
stall if hazards occur to either of them.
Lower instructions write the same register at the same time, priority is given
to the Upper instruction and the result of the Lower instruction is discarded.
This holds in register units: "the result of the Lower instruction is
discarded even when the Upper and Lower instructions write data to different
fields". Worked example:
ADD.xy VF01, VF01, VF23as the upper half pairedwith
MOVE.w VF01, VF09as the lower half.occur between the Upper instruction and the Lower instruction in the same
instruction.
DIVandSQRTreach their write-backstage only at the end of the FDIV pipeline, and
RSQRTat the end of alonger one.
Qis therefore not visible to the pair that issued the divide.RGETandRNEXTinstruction pages, Remarks — "When an Upperinstruction in the same cycle writes data to the VF[ft] register, the result
of this instruction is discarded with priority given to the Upper instruction,
regardless of whether the data is written to the same field or not."
10. Blast radius
Behaviour that changes, and becomes more faithful to the hardware model:
ELENG,ERLENG,ERCPRQand whose lower half writes it now feeds the upper half the pre-pairQDIV,SQRTorRSQRTshares a pair with aq-form upper operation and the pair also hazards on VFRow 3 is the only one that removes a write that previously landed. Before this
change, a reordered pair whose lower half wrote a field of the upper half's
destination that the upper half did not itself write kept that field from the
lower half's write. That field is now discarded with the rest of the register,
matching the §9 worked example. A microprogram whose observable output depended
on the old per-field behaviour changes output.
Behaviour provably unchanged:
execUpper; execLowerbranchlowerBeforeUpperis!iBit && ..., so LOI never enters the changed branchQpostLowerQequalsprePairQ, so the branch runs the previousexecLower; execUppersequence and nothing else. Stores andMTIRare in this set, and the pre-existingSQhazard test is an instance of it.DIV,SQRTandRSQRTwriteQ, so their pairs are not: they are changed row 4execLower; execUpperandexecUpper; execLowerleave the same machine state and the predicate's new answer is not observable. §2 gives the arm state of every selector this change reclassifies, across both the EFU and the R-register block; all of them are in this set exceptELENG,ERLENGandERCPR, which are changed row 1QandPvisibilityCost, on the hazard branch only: two extra classifier calls — the lower half's
VF write mask and the upper half's VF destination — and then, at most, one
16-byte register copy before the lower half runs, two between the halves (the
guarded register is captured and then rolled back), and one after the upper
half runs.
guardedCountreaches one and goes no higher: no arm ofvuLowerVfReadWriteMaskssets more than one write bit, andvuAddGuardedRegadmits only a register that bit names. Its de-duplication is what holds the
count there when more than one of the upper half's
fs,ftand destinationnames that register, which is the shape mutation P17 kills; and the
guarded[3]array is sized by its three call sites, not by this bound. Pairsthat do not hazard are untouched, and the decode cache is unaffected.
The write-bit bound holds over every 32-bit lower encoding:
11. What stays open
ERCPRend-to-end (axis I) is asserted at the classifier only.ELENGand
ERLENGcarry the end-to-end assertion for the EFU arm.Inter-pair
Q/Pvisibility latency is unchanged and still wrong. Thistree makes a
QorPwrite visible immediately rather than at the end ofthe FDIV or EFU pipeline, which is a latency model. Intra-pair ordering is
closed here, and the inter-pair window is the same width as before.
No
WAITQ/WAITPsequencing.DIV/SQRT/RSQRTwriteQimmediatelyand EFU operations write
Pimmediately, so there is no pending state forWAITQ/WAITPto interlock against and no test could pin a change.The plain upper-then-lower branch and the LOI branch have no mutation.
They are unedited code, and
decodeInstructionPairstill computeslowerBeforeUpperas!decoded.iBit && vuLowerShouldRunBeforeUpper(...);they are backed by the existing suite staying green under §3.
Opcodes with an empty or absent execution arm have no end-to-end
mutation. Their classification is asserted directly at the classifier, but
no mutation of their cases can be observed through
run()today.vuAddGuardedReg'slowerWritesfilter has no mutation row. Dropping the((lowerWrites >> reg) & 1u) == 0utest is behaviour-preserving here, so notest can distinguish it and none is added. It can only add registers the lower
half did not write; for those
postLowerequalsprePair, making the rollbackand the re-apply no-ops. The re-apply cannot land on the upper half's own write
either: the VF destinations
execUpperuses arem_state.vf[fd]forop <= 0x2Fandm_state.vf[ft]for the special selectors, matching whatvuUpperVfWriteRegreturns, and that register is excluded byif (guarded[i] != upperWriteReg). Check both statements withThe filter is an optimisation: it keeps the guarded set to the registers that
need guarding. If the classifier is ever found to under-report a lower-half VF
write, dropping the filter is the safe response, which is a further reason not
to pin it.
None of these is described as closed anywhere in this document or in
PR-BODY.md.