IPU: dither a whole row per deinterleaving load

ipu_dither has had an SSE2 path and a scalar reference since forever, and
arm64 took the reference. The compiler closes half of that gap on its own —
with dithering off the loop is simple enough that clang vectorises it, and
measured here the scalar and NEON versions come out cycle-identical. With
dithering on it closes none of it: the clamp is written as std::max/std::min
around a table lookup, the destination is a 5/5/5/1 bitfield, and between
them the vectoriser gives up entirely. That arm ran at about 36 instructions
per pixel.

The NEON version is not a transliteration of the SSE2 one. x86 needs six
unpacks to split a row into channels because it has no deinterleaving load;
NEON has VLD4, so a whole 16-pixel row arrives already split one register per
channel and the shuffle chain simply does not exist. The dither tables are
the reference's coefficients with the sign folded into the choice of
operation, which lets saturating byte arithmetic supply the clamp for free —
the same trick the SSE2 path uses, and the reason both agree with the
reference bit for bit.

Measured on an M2 Max P-core, 2M macroblocks, two runs each:

  dither on   reference  18.76G instructions / 3.372G cycles
              NEON        1.29G instructions / 0.293G cycles   (11.5x)
  dither off  reference   1.08G instructions / 0.247G cycles
              NEON        1.13G instructions / 0.247G cycles   (even)

Function size drops from 476 to 208 bytes.

The tests are the point of the commit as much as the code is. Three
implementations of one function existed and nothing had ever compared them,
which is a bad shape here: a wrong result does not crash, it tints an FMV,
and nobody reports that. The transform depends on nothing but a pixel's four
bytes and its position modulo four in each axis, so the suite sweeps every
byte value through every one of the sixteen dither cells rather than
sampling. It holds whichever path the host selected to the reference, so it
gates the SSE2 arm on x86 exactly as it gates NEON here.

Proven to discriminate by mutation: transposing the r and b channels fails
three of four cases (correctly not the sweep that holds the channels equal),
perturbing one dither cell by one fails two, and dropping saturation fails
all four.

ipu_dither_reference loses its __ri so that a symbol survives into Release
for the tests to call.
This commit is contained in:
Brian Degenhardt
2026-08-14 21:25:30 -07:00
parent a3bf73bf7a
commit c8b51438cc
5 changed files with 302 additions and 3 deletions
+5
View File
@@ -144,6 +144,11 @@ alignas(16) extern tIPU_BP g_BP;
MULTI_ISA_DEF(
extern void ipu_dither(const macroblock_rgb32& rgb32, macroblock_rgb16& rgb16, int dte);
// The scalar oracle ipu_dither()'s vector paths are written against. Exposed
// so the tests can hold whichever path this host selected to it; the emulator
// only ever reaches it through ipu_dither()'s own fallback arm.
extern void ipu_dither_reference(const macroblock_rgb32& rgb32, macroblock_rgb16& rgb16, int dte);
void IPUWorker();
)
+73 -3
View File
@@ -10,22 +10,29 @@
MULTI_ISA_UNSHARED_START
void ipu_dither_reference(const macroblock_rgb32 &rgb32, macroblock_rgb16 &rgb16, int dte);
#if defined(_M_X86)
void ipu_dither_sse2(const macroblock_rgb32 &rgb32, macroblock_rgb16 &rgb16, int dte);
#endif
#if defined(ARCH_ARM64)
void ipu_dither_neon(const macroblock_rgb32 &rgb32, macroblock_rgb16 &rgb16, int dte);
#endif
__ri void ipu_dither(const macroblock_rgb32 &rgb32, macroblock_rgb16 &rgb16, int dte)
{
#if defined(_M_X86)
ipu_dither_sse2(rgb32, rgb16, dte);
#elif defined(ARCH_ARM64)
ipu_dither_neon(rgb32, rgb16, dte);
#else
ipu_dither_reference(rgb32, rgb16, dte);
#endif
}
__ri void ipu_dither_reference(const macroblock_rgb32 &rgb32, macroblock_rgb16 &rgb16, int dte)
// Deliberately not inlineable: this is the semantic oracle the vector paths are
// written against, so the tests need a symbol to call. (__ri collapses to
// __forceinline in Release, which would leave nothing to link to.)
void ipu_dither_reference(const macroblock_rgb32 &rgb32, macroblock_rgb16 &rgb16, int dte)
{
if (dte) {
// I'm guessing values are rounded down when clamping.
@@ -121,4 +128,67 @@ __ri void ipu_dither_sse2(const macroblock_rgb32 &rgb32, macroblock_rgb16 &rgb16
#endif
#if defined(ARCH_ARM64)
// dither_coefficient[] above with the sign folded into the choice of operation:
// a positive cell goes in the add table, a negative one goes in the sub table at
// its magnitude, and the other table holds zero for that lane. Saturating byte
// arithmetic then gives the reference's clamp to [0, 255] for free.
//
// One row of the source matrix covers four pixel columns and the pattern repeats
// every four, so each entry is that row's four cells laid out four times — lane
// k is the cell for pixel k, which is what a deinterleaved row wants.
alignas(16) static const u8 dither_add_matrix[4][16] = {
{0, 0, 0, 1, 0, 0, 0, 1, 0, 0, 0, 1, 0, 0, 0, 1},
{2, 0, 3, 0, 2, 0, 3, 0, 2, 0, 3, 0, 2, 0, 3, 0},
{0, 1, 0, 0, 0, 1, 0, 0, 0, 1, 0, 0, 0, 1, 0, 0},
{3, 0, 2, 0, 3, 0, 2, 0, 3, 0, 2, 0, 3, 0, 2, 0},
};
alignas(16) static const u8 dither_sub_matrix[4][16] = {
{4, 0, 3, 0, 4, 0, 3, 0, 4, 0, 3, 0, 4, 0, 3, 0},
{0, 2, 0, 1, 0, 2, 0, 1, 0, 2, 0, 1, 0, 2, 0, 1},
{3, 0, 4, 0, 3, 0, 4, 0, 3, 0, 4, 0, 3, 0, 4, 0},
{0, 1, 0, 2, 0, 1, 0, 2, 0, 1, 0, 2, 0, 1, 0, 2},
};
__ri void ipu_dither_neon(const macroblock_rgb32 &rgb32, macroblock_rgb16 &rgb16, int dte)
{
const uint8x16_t alpha_test = vdupq_n_u8(0x40);
for (int i = 0; i < 16; ++i) {
// NEON deinterleaves on the load, so a whole 16-pixel row arrives already
// split one register per channel. The SSE path needs six unpacks to reach
// the same place because x86 has no equivalent load.
uint8x16x4_t px = vld4q_u8(&rgb32.c[i][0].r);
if (dte) {
const uint8x16_t add = vld1q_u8(dither_add_matrix[i & 3]);
const uint8x16_t sub = vld1q_u8(dither_sub_matrix[i & 3]);
px.val[0] = vqsubq_u8(vqaddq_u8(px.val[0], add), sub);
px.val[1] = vqsubq_u8(vqaddq_u8(px.val[1], add), sub);
px.val[2] = vqsubq_u8(vqaddq_u8(px.val[2], add), sub);
}
const uint8x16_t r = vshrq_n_u8(px.val[0], 3);
const uint8x16_t g = vshrq_n_u8(px.val[1], 3);
const uint8x16_t b = vshrq_n_u8(px.val[2], 3);
const uint8x16_t a = vceqq_u8(px.val[3], alpha_test);
// r:5 g:5 b:5 a:1, least significant field first. The alpha compare widens
// to 0x00FF, and 0x00FF << 15 truncates to exactly the 0x8000 top bit.
const uint16x8_t lo = vorrq_u16(
vorrq_u16(vmovl_u8(vget_low_u8(r)), vshlq_n_u16(vmovl_u8(vget_low_u8(g)), 5)),
vorrq_u16(vshlq_n_u16(vmovl_u8(vget_low_u8(b)), 10), vshlq_n_u16(vmovl_u8(vget_low_u8(a)), 15)));
const uint16x8_t hi = vorrq_u16(
vorrq_u16(vmovl_high_u8(r), vshlq_n_u16(vmovl_high_u8(g), 5)),
vorrq_u16(vshlq_n_u16(vmovl_high_u8(b), 10), vshlq_n_u16(vmovl_high_u8(a), 15)));
vst1q_u16(reinterpret_cast<u16 *>(&rgb16.c[i][0]), lo);
vst1q_u16(reinterpret_cast<u16 *>(&rgb16.c[i][8]), hi);
}
}
#endif
MULTI_ISA_UNSHARED_END
+3
View File
@@ -20,6 +20,9 @@ endif()
# GS vertex front-end kernel oracle (arch-neutral).
add_subdirectory(gs)
# IPU colour-conversion oracle (arch-neutral: each host tests its own path).
add_subdirectory(ipu)
set(multi_isa_sources
GS/swizzle_test_main.cpp
+17
View File
@@ -0,0 +1,17 @@
# IPU colour-conversion oracle suite. ipu_dither() compiles to a different
# implementation per architecture and the three were never compared to each other;
# these hold whichever one this host selected to the scalar reference.
add_pcsx2_test(ipu_dither_tests
${CMAKE_CURRENT_SOURCE_DIR}/../StubHost.cpp
ipu_dither_tests.cpp
)
target_include_directories(ipu_dither_tests PRIVATE
${CMAKE_CURRENT_SOURCE_DIR}/../../../../pcsx2
)
target_link_libraries(ipu_dither_tests PUBLIC
PCSX2_FLAGS
PCSX2
common
)
+204
View File
@@ -0,0 +1,204 @@
// SPDX-FileCopyrightText: 2026 ARMSX2 Contributors
// SPDX-License-Identifier: GPL-3.0+
// Whichever dither path this host selects must be bit-identical to the scalar one.
//
// ipu_dither() has three implementations -- a scalar reference, an SSE2 path and a
// NEON path -- and the emulator picks one at compile time from the architecture.
// Nothing ever compared them. That is a bad shape for a function like this: it is
// the last step of MPEG colour conversion, so an error does not crash, it tints an
// FMV slightly, and nobody files that as a bug.
//
// The transform is per-pixel and depends on nothing but the pixel's four bytes and
// its position modulo four in each axis, so the whole input domain is small enough
// to cover directly rather than sampled. The sweeps below walk every byte value
// through every one of the sixteen dither cells; the randomised case exists on top
// of that only to catch a path that crosses channels, which a sweep holding r, g
// and b equal would not see.
//
// Both dither states matter. dte=0 is not a trivial passthrough -- it still packs
// 8888 down to 1555 and still derives alpha from a compare against 0x40 -- so a
// path that only got the dithered arm right would be half broken in exactly the
// mode most games use.
#include "IPU/IPU_MultiISA.h"
#include "GS/MultiISA.h"
#include "gtest/gtest.h"
#include <array>
#include <cstring>
#include <random>
namespace
{
constexpr int kDim = 16;
// A value no real conversion can produce: every output pixel has its top bit
// set only when alpha matched, so an all-ones block would require every pixel
// to be white and opaque at once. Seeding with it turns "this path skipped a
// pixel" into a mismatch instead of a silent pass on stale memory.
void PoisonOutput(macroblock_rgb16& rgb16)
{
std::memset(&rgb16, 0xFF, sizeof(rgb16));
}
// Returns the index of the first differing pixel, or -1 when the two agree.
int FirstMismatch(const macroblock_rgb16& a, const macroblock_rgb16& b)
{
u16 a_words[kDim * kDim];
u16 b_words[kDim * kDim];
std::memcpy(a_words, &a, sizeof(a_words));
std::memcpy(b_words, &b, sizeof(b_words));
for (int i = 0; i < kDim * kDim; i++)
{
if (a_words[i] != b_words[i])
return i;
}
return -1;
}
// Runs the host's selected path and the reference over the same input and
// reports the first pixel they disagree on, with enough context to place it in
// the dither matrix.
void ExpectMatchesReference(const macroblock_rgb32& rgb32, int dte, const char* what)
{
macroblock_rgb16 got;
macroblock_rgb16 want;
PoisonOutput(got);
PoisonOutput(want);
MULTI_ISA_SELECT(ipu_dither)(rgb32, got, dte);
MULTI_ISA_SELECT(ipu_dither_reference)(rgb32, want, dte);
const int bad = FirstMismatch(got, want);
if (bad < 0)
return;
const int row = bad / kDim;
const int col = bad % kDim;
const auto& src = rgb32.c[row][col];
u16 got_words[kDim * kDim];
u16 want_words[kDim * kDim];
std::memcpy(got_words, &got, sizeof(got_words));
std::memcpy(want_words, &want, sizeof(want_words));
ADD_FAILURE() << what << ": dte=" << dte << " first mismatch at row " << row
<< " col " << col << " (dither cell [" << (row & 3) << "][" << (col & 3) << "])"
<< "\n source rgba = " << int(src.r) << ", " << int(src.g) << ", "
<< int(src.b) << ", " << int(src.a)
<< "\n got = 0x" << std::hex << got_words[bad]
<< "\n want = 0x" << want_words[bad] << std::dec;
}
} // namespace
// Every byte value, through every dither cell, on the colour channels. Holding the
// three channels equal is what makes this a clean sweep of the dither arithmetic;
// channel independence is the randomised test's job.
TEST(IPUDither, ColourSweepMatchesReference)
{
for (int v = 0; v <= 255; v++)
{
macroblock_rgb32 rgb32;
for (int i = 0; i < kDim; i++)
{
for (int j = 0; j < kDim; j++)
{
rgb32.c[i][j].r = static_cast<u8>(v);
rgb32.c[i][j].g = static_cast<u8>(v);
rgb32.c[i][j].b = static_cast<u8>(v);
// Alternate the two alpha outcomes so neither is ever untested.
rgb32.c[i][j].a = ((i + j) & 1) ? 0x40 : 0x00;
}
}
ExpectMatchesReference(rgb32, 1, "colour sweep");
ExpectMatchesReference(rgb32, 0, "colour sweep");
}
}
// Alpha is a compare against 0x40, not a range, so the interesting inputs are the
// neighbours of that value as much as the extremes. Sweeping the whole byte covers
// both without having to guess.
TEST(IPUDither, AlphaSweepMatchesReference)
{
for (int v = 0; v <= 255; v++)
{
macroblock_rgb32 rgb32;
for (int i = 0; i < kDim; i++)
{
for (int j = 0; j < kDim; j++)
{
// Distinct per channel, so an alpha bug cannot hide behind a
// colour that happens to match.
rgb32.c[i][j].r = static_cast<u8>(j * 16);
rgb32.c[i][j].g = static_cast<u8>(i * 16);
rgb32.c[i][j].b = static_cast<u8>((i + j) * 8);
rgb32.c[i][j].a = static_cast<u8>(v);
}
}
ExpectMatchesReference(rgb32, 1, "alpha sweep");
ExpectMatchesReference(rgb32, 0, "alpha sweep");
}
}
// The saturating arithmetic only shows its edges where a cell pushes a value past
// a limit, and the cells reach +3 and -4. Pinning the exact boundary values means a
// path that clamps with the wrong operation fails here rather than on one unlucky
// random block.
TEST(IPUDither, SaturationEdgesMatchReference)
{
static constexpr std::array<u8, 10> kEdges = {0, 1, 2, 3, 4, 251, 252, 253, 254, 255};
for (const u8 lo : kEdges)
{
for (const u8 hi : kEdges)
{
macroblock_rgb32 rgb32;
for (int i = 0; i < kDim; i++)
{
for (int j = 0; j < kDim; j++)
{
rgb32.c[i][j].r = lo;
rgb32.c[i][j].g = hi;
rgb32.c[i][j].b = static_cast<u8>((j & 1) ? lo : hi);
rgb32.c[i][j].a = ((i + j) & 1) ? 0x40 : 0x3F;
}
}
ExpectMatchesReference(rgb32, 1, "saturation edges");
ExpectMatchesReference(rgb32, 0, "saturation edges");
}
}
}
// The sweeps all hold something constant across the block. This one holds nothing
// constant, which is what catches a path that reads the right bytes into the wrong
// channel -- a deinterleave that transposes r and b survives every test above.
TEST(IPUDither, RandomMacroblocksMatchReference)
{
std::mt19937 rng(20260814u); // fixed seed: a failing case must be reproducible
std::uniform_int_distribution<int> byte(0, 255);
for (int iter = 0; iter < 256; iter++)
{
macroblock_rgb32 rgb32;
for (int i = 0; i < kDim; i++)
{
for (int j = 0; j < kDim; j++)
{
rgb32.c[i][j].r = static_cast<u8>(byte(rng));
rgb32.c[i][j].g = static_cast<u8>(byte(rng));
rgb32.c[i][j].b = static_cast<u8>(byte(rng));
// Bias alpha towards the one value the compare cares about,
// otherwise it is almost never hit at random.
rgb32.c[i][j].a = (byte(rng) < 128) ? 0x40 : static_cast<u8>(byte(rng));
}
}
ExpectMatchesReference(rgb32, iter & 1, "random macroblock");
}
}