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

Abierto
  • ¿Algo no está bien?
#397 1 comentario 0 reacciones 0 asignados Ver en GitHub

Los mantenedores suelen responder en 5 días

Nadie ha tomado este issue todavía.

Evaluación

Dificultad
4/5
Tiempo estimado
3-5 días
Aptitud para principiantes
55/100
Tipo de issue
Error
Claridad
Bien especificado
Estado de actividad
Activo
Stack tecnológico
cpp
Área
backend, compilers

Línea de trabajo

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.

Escrito por el modelo de indexación a partir del texto del issue.

Descripción

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!

Lenguaje dominante
C++
Estrellas
3.3k
Forks
149
Merge medio
8 h 46 min
PR fusionados (30 d)
1

Preparar el entorno

Este proyecto no incluye contenedor de desarrollo, Dockerfile ni guía de contribución, así que la configuración corre por tu cuenta: empieza por su README y consulta nuestra guía para la primera contribución para los pasos generales.

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Más de ran-j/PS2Recomp

Todos los issues de ran-j/PS2Recomp

Issues similares

Más issues de C++

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.