Both the interpreter and the arm64 recompiler modelled each PMADDW/PMSUBW
lane as two independent 32-bit accumulations. That loses the carry between
LO and HI, needs a truncating `/ 0xFFFFFFFF` where an arithmetic shift
belongs, grew a +0x70000000 lane-0 addend to compensate, and leaves LO
sign-extended from the wrapped intermediate rather than from the
architectural result. The x86 recompiler has always done it correctly
(recPMADDW: PMULDQ + PADDQ + PMOVSXDQ against the packed accumulator), so
the three engines disagreed; this brings the other two into line with it.
Scored against the 64 ps2autotests PMADDW/PMSUBW captures, which preset
HI/LO to nonzero values and so make the width of the accumulate observable:
two 32-bit halves + errata 54/64
packed 64-bit accumulate 64/64
The errata components are not separable. Reintroducing any one of them
alone into the corrected model scores far worse than leaving all three in:
packed + voodoo addend 52/64
packed + truncating divide 3/64
packed + carry dropped from HI 33/64
The truncating divide's off-by-one very nearly cancels the missing carry
on these operands, which is why the shipping code only lost 10 cases. A
partial fix here would have been much worse than none.
On arm64 the correct form is also the smaller one: Smull plus a 64-bit
add against the spliced accumulator replaces the lane splitting, the SDIV
by 0xFFFFFFFF and the whole conditional-addend block, and Rd.UD[dd] turns
out to be the raw result. Verified bidirectionally: reverting either
engine alone leaves 7 tests failing, since h.Run() auto-diffs the JIT
against the interpreter.
Tests: the six EeRecMmi vectors that pinned the errata are rewritten to
pin the carry, the borrow, the per-half sign extension and the absence of
the lane-0 addend; all six fail on the unpatched baseline. The
PMADDW/PMSUBW allowance in EeMmiConsoleConformance is deleted, so all 64
cases now assert normally.
Also corrects a comment in iMMI-arm64.cpp claiming MMI2_RECOMPILE is
never defined -- Config.h:1650 defines it unconditionally, which makes
that part of pcsx2/x86/iMMI.cpp the shipping x86 implementation rather
than dead reference code.
Idea by pstef.
EE interpeter: remove unused argument
rdd is neither used, nor needed. It appears it was there to pass the _Rd_ word to write to, but the writing was moved to PHMSBH() to have one "if (_Rd_)".
Add a note on undefined behavior
Math is correct but a shift of 64 bits is illegal in x86 because the cl register is masked
From the x86 spec:
The destination operand can be a register or a memory location. The count operand can be an immediate value or
the CL register. The count is masked to 5 bits (or 6 bits if in 64-bit mode and REX.W is used). The count range is
limited to 0 to 31 (or 63 if 64-bit mode and REX.W is used). A special opcode encoding is provided for a count of 1.
-Coverity CID 146817: In R5900::Interpreter::OpcodeImpl::MMI::PMFHL(): Code can never be reached because of a logical contradiction (CWE-561)
-This code is used by both the Interpreter and Recompiler, however it was probably never checked because nothing much uses it. Out of 248 games, it was called 0 times.
int src; if (src < 0xffff8000) {}
so solution is either use (int)0xffff8000 or -0x8000...
it also doesn't seem to print out warnings about this either D:
git-svn-id: http://pcsx2.googlecode.com/svn/trunk@3712 96395faa-99c1-11dd-bbfe-3dabce05a288
bmdhacks
SternXD
TheTechnician27
GovanifY
Stenzek
TellowKrinkle
kojin
Dylan Smit
refractionpcsx2
Huud
Gregory Hainaut
Jonathan Li
Miguel A. Colón Vélez
Sacha
ramapcsx2
cottonvibes