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

オープン
  • 何か問題がありますか？
#397 コメント 1 件 リアクション 0 件 担当者 0 名 GitHub で見る

メンテナーはふだん 5 日以内に返信

まだ誰も着手していません。

評価

難易度
4/5
見積もり時間
3〜5日
初心者へのやさしさ
55/100
issue の種類
バグ
明瞭さ
明確に書かれている
活発さ
活発
技術スタック
cpp
領域
backend, compilers

調査の方向性

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.

索引モデルが issue の本文から書いたものです。

説明

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!

主要言語
C++
スター
3.3k
フォーク
149
平均マージ
8時間 46分
マージ済み PR（30日）
1

環境構築

このプロジェクトには開発コンテナ、Dockerfile、コントリビューションガイドがありません。まず README を読み、一般的な手順ははじめてのコントリビューションガイドを参照してください。

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. issue 番号を参照したプルリクエストを送ります。

ran-j/PS2Recomp のほかの issue

ran-j/PS2Recomp の issue をすべて見る

似ている issue

C++ の issue をもっと見る

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。