Recompiler: PADDUH/PSUBUH, PABSW/PABSH and PSLLVW/PSRLVW/PSRAVW produce wrong values

Open
  • Is something wrong?
#397 1 comment 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 5 days

Nobody has claimed this yet.

Assessment

Difficulty
4/5
Estimated time
3-5 days
Newbie friendliness
55/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
cpp
Domain
backend, compilers

Research direction

Start in ps2xRecomp/src/lib/mmi_translation_helpers.cpp and inspect the listed MMI translations, then read the corresponding macros in ps2xRuntime/include/ps2_runtime_macros.h. Compare generated results with the EE behavior described in the issue, including saturation, absolute-value edge cases, operand selection, and the two-word variable-shift result. Done means all seven operations match the documented behavior across the edge cases described.

Written by the indexing model from the issue text.

Description

What happens

Seven MMI ops translate to code that gives different results from the documented EE behaviour (which PCSX2's interpreter also follows). Nothing warns or crashes; the values are just wrong.

op EE behaviour PS2Recomp emits (at e5636981) example
PADDUH rd, rs, rt 8 x u16 add, saturating at 0xFFFF _mm_add_epi16(rs, rt) (wraps) 0xFFFF + 0x0001 gives 0x0000 (EE: 0xFFFF)
PSUBUH rd, rs, rt 8 x u16 subtract, saturating at 0 _mm_sub_epi16(rs, rt) (wraps) 0x0000 - 0x0001 gives 0xFFFF (EE: 0x0000)
PABSW rd, rt 4 x |rt.w|, with 0x80000000 -> 0x7FFFFFFF PS2_PABSW(GPR_VEC(ctx, rs)); rs is 0 in the encoding rd is 0 for every input
PABSH rd, rt 8 x |rt.h|, with 0x8000 -> 0x7FFF PS2_PABSH(GPR_VEC(ctx, rs)) rd is 0 for every input
PSLLVW / PSRLVW / PSRAVW rd, rt, rs shift rt's words 0 and 2 by (rs's words 0 and 2) & 31; the result is two sign-extended doublewords PS2_PSxxVW(GPR_VEC(ctx, rs), GPR_VEC(ctx, rt)): shifts all 4 words of rs by rt see below

Variable-shift example (PSLLVW), with rs.w = {1, 0, 4, 0} and rt.w = {0x40000000, 0, 1, 0}:

  • EE: d0 = sign-extend(0x40000000 << 1) = 0xFFFFFFFF80000000, d1 = sign-extend(1 << 4) = 0x10, so rd.w = {0x80000000, 0xFFFFFFFF, 0x10, 0}.
  • Generated code (each word of rs shifted by the same word of rt, & 31): {1 << (0x40000000 & 31), 0 << 0, 4 << 1, 0 << 0} = {1, 0, 8, 0}.

PSRLVW and PSRAVW have the same two problems (operands swapped, 4 independent words instead of 2 sign-extended doublewords).

The byte and word siblings (PADDUB/PADDUW/PSUBUB/PSUBUW) already saturate correctly, so it's only the halfword forms.

Where it comes from

In ps2xRecomp/src/lib/mmi_translation_helpers.cpp:

In ps2xRuntime/include/ps2_runtime_macros.h:

  • L718-L719: PS2_PABSW/PS2_PABSH are _mm_abs_epi32/_mm_abs_epi16, which leave 0x80000000/0x8000 unchanged instead of clamping.
  • L776-L778 and _mm_custom_sllv_epi32 from L780: shift all 4 words of the first argument independently.

How I found it

I translated small test programs with ps2_recomp (e5636981, Release), built the generated C++ against ps2xRuntime's headers with a minimal stand-in for the runtime class, ran it, and compared the results with a small hand-written reference model of each op, cross-checked against PCSX2's interpreter (pcsx2/MMI.cpp). Each op was tested in a straight-line kernel of 8 load/op/store blocks over my own input data, built with the ps2dev EE gcc 15.2.0 (-march=r5900), with 32 input sets including edge values. One block looks like this:

  lq     $10, 0($8)        # rs
  lq     $11, 16($8)       # rt
  padduh $12, $10, $11
  sq     $12, 0($9)

The seven ops above differed on all 32 input sets. Changing only the emitted expression for each op (the fixes below) made all 32 match.

Suggested fix (just a sketch)

  • PADDUH / PSUBUH: _mm_adds_epu16 / _mm_subs_epu16.
  • PABSW / PABSH: read rt, and clamp, for example _mm_min_epu32(_mm_abs_epi32(rt), _mm_set1_epi32(0x7FFFFFFF)) and _mm_min_epu16(_mm_abs_epi16(rt), _mm_set1_epi16(0x7FFF)) (SSE4.1). Fixing only the operand still leaves the 0x80000000 / 0x8000 lanes wrong.
  • PSLLVW / PSRLVW / PSRAVW: shift rt by rs, using words 0 and 2 only, and sign-extend each 32-bit result to 64 bits. For PSLLVW: 
    _mm_set_epi64x(
    (int64_t)(int32_t)((uint32_t)_mm_extract_epi32(rt, 2) << (_mm_extract_epi32(rs, 2) & 31)),
    (int64_t)(int32_t)((uint32_t)_mm_extract_epi32(rt, 0) << (_mm_extract_epi32(rs, 0) & 31)))
    PSRLVW is the same with a logical >> on the uint32_t; PSRAVW uses an arithmetic >> on the int32_t. Both parts are needed: swapping the operands alone, or fixing the shape alone, still gives wrong results.

Related

  • Open PRs #291 (MMI permutation ops) and #322 (HI/LO family) don't touch these seven ops; #322's head (792e313d) still has the variable-shift issue.
  • I couldn't find an existing issue or PR that names these ops. The only open PRs I found that touch sllv/srlv/srav (#283 and #135) change the scalar shift instructions, not these MMI ops.

What I haven't checked

  • Not tested on hardware. The expected values come from the EE's documented behaviour, cross-checked against PCSX2's interpreter.
  • Only the register-to-register forms, loaded and stored with lq/sq, in small kernels. I haven't run a full game through it or measured how often games use these ops.

I can attach the test ELFs, input data and expected outputs if that helps. Happy to split this into separate issues or open a PR, whichever you prefer.

Thanks for PS2Recomp!

Dominant language
C++
Stars
3.3k
Forks
149
Avg merge
8h 46m
Merged PRs (30d)
1

Getting set up

This project ships no dev container, Dockerfile or contributing guide, so setting up is up to you: start from its README, and see our first-contribution guide for the general steps.

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from ran-j/PS2Recomp

All issues in ran-j/PS2Recomp

Similar issues

More C++ issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.