admin/ci-libretro-fceumm
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

filter: revert SSE2/NEON FIR kernel — non-bit-exact, audio regressions

Browse Source 
The SSE2/NEON FIR added in df8a6e8 was not bit-exact with the scalar
code it replaced:

  * It packed int32 samples to int16 with PACKSSDW saturation, on the
    assumption that NES audio stays inside int16.  At full mixer
    volume with several APU channels active, WaveHi samples can
    exceed int16 -- the pack then clips, changing the filter response
    in a way that is not just sub-LSB drift.

  * It applied the >> 6 once after the horizontal sum rather than
    per-term as the scalar does.  The scalar's per-term shift keeps
    the int32 accumulator inside ~5e8 worst-case; the delayed shift
    pushes the intermediate value to ~3e10 and wraps int32 on
    sustained loud audio, producing garbage samples ("Sound Quality
    High" sounds broken).

  * The (int16_t) narrowing cast on the 96 kHz coefficient tables
    silently wrapped values up to ~51000 to negative numbers, giving
    nonsense coefficients on that output rate.

There is no clean bit-exact int32 x int32 -> int32 SIMD on SSE2
baseline -- pmuludq is 2-wide unsigned 32x32 -> 64 with sign
correction needed for the signed case, and the realisable speedup
at scalar bit-exactness is marginal vs. the ~1.4% of frame budget
the scalar already costs.  Restoring the scalar code, which is
correct across every rate / quality / volume combination.
This commit is contained in:
libretroadmin
2026-06-14 14:36:58 +00:00
committed by U-DESKTOP-SPFP6AQ\twistedtechre
parent df8a6e8b26
commit 01b7446a66
1 changed files with 16 additions and 127 deletions
Download Patch File Download Diff File Expand all files Collapse all files

143
src/filter.c
View File

@@ -7,12 +7,6 @@
#include "fcoeffs.h"
#if defined(__SSE2__)
#include <emmintrin.h>
#elif defined(__ARM_NEON) || defined(__ARM_NEON__)
#include <arm_neon.h>
#endif
static uint32_t mrindex;
static uint32_t mrratio;
@@ -86,108 +80,6 @@ void SexyFilter(int32_t *in, int32_t *out, int32_t count) {
code to be higher, or you *might* overflow the FIR code.
*/
/* Int16 mirrors of the symmetric windowed-sinc coefficient tables built
* in MakeFilters. The source tables have max abs value ~21588, which
* comfortably fits in int16, so packing is lossless on the coefficient
* side. This lets the SIMD kernels below use pmaddwd / vmlal_s16,
* 4-8x faster than the scalar 32x32 path while preserving sub-LSB
* audio drift across the 484/1024-tap window. */
static int16_t coeffs16[NCOEFFS];
static int16_t sq2coeffs16[SQ2NCOEFFS];
/* FIR inner kernel. ncoeffs is NCOEFFS or SQ2NCOEFFS (compile-time
* constants both -- the compiler specialises this for each caller).
* The SSE2 and NEON paths apply the >> 6 once after the sum rather
* than per-term as the scalar does; the difference is per-term
* truncation rounding accumulated across the window, well under one
* LSB of the final 16-bit output sample. Sample magnitudes are
* packed with signed saturation (PACKSSDW / vqmovn_s32) -- typical
* NES audio stays comfortably within int16, and a peak that would
* saturate here would already be clipped at the subsequent SexyFilter
* stage that bounds output to [-32768, 32767]. */
static INLINE void fir_inner_kernel(
const int32_t *S, const int16_t *D16, uint32_t ncoeffs,
int32_t *out_acc, int32_t *out_acc2)
{
int32_t acc = 0, acc2 = 0;
uint32_t j;
#if defined(__SSE2__)
{
__m128i acc_v = _mm_setzero_si128();
__m128i acc2_v = _mm_setzero_si128();
for (j = 0; j + 8 <= ncoeffs; j += 8) {
__m128i a_lo = _mm_loadu_si128((const __m128i *)&S[j + 1]);
__m128i a_hi = _mm_loadu_si128((const __m128i *)&S[j + 5]);
__m128i b_lo = _mm_loadu_si128((const __m128i *)&S[j + 2]);
__m128i b_hi = _mm_loadu_si128((const __m128i *)&S[j + 6]);
__m128i s_a = _mm_packs_epi32(a_lo, a_hi);
__m128i s_b = _mm_packs_epi32(b_lo, b_hi);
__m128i co = _mm_loadu_si128((const __m128i *)&D16[j]);
acc_v = _mm_add_epi32(acc_v, _mm_madd_epi16(s_a, co));
acc2_v = _mm_add_epi32(acc2_v, _mm_madd_epi16(s_b, co));
}
/* Horizontal sum of the 4 int32 lanes in each accumulator. */
{
__m128i shuf = _mm_shuffle_epi32(acc_v, _MM_SHUFFLE(2,3,0,1));
__m128i s1 = _mm_add_epi32(acc_v, shuf);
shuf = _mm_shuffle_epi32(s1, _MM_SHUFFLE(1,0,3,2));
acc = _mm_cvtsi128_si32(_mm_add_epi32(s1, shuf));
shuf = _mm_shuffle_epi32(acc2_v, _MM_SHUFFLE(2,3,0,1));
s1 = _mm_add_epi32(acc2_v, shuf);
shuf = _mm_shuffle_epi32(s1, _MM_SHUFFLE(1,0,3,2));
acc2 = _mm_cvtsi128_si32(_mm_add_epi32(s1, shuf));
}
acc >>= 6;
acc2 >>= 6;
}
#elif defined(__ARM_NEON) || defined(__ARM_NEON__)
{
int32x4_t acc_v0 = vdupq_n_s32(0), acc_v1 = vdupq_n_s32(0);
int32x4_t acc2_v0 = vdupq_n_s32(0), acc2_v1 = vdupq_n_s32(0);
for (j = 0; j + 8 <= ncoeffs; j += 8) {
int32x4_t a0 = vld1q_s32(&S[j + 1]);
int32x4_t a1 = vld1q_s32(&S[j + 5]);
int32x4_t b0 = vld1q_s32(&S[j + 2]);
int32x4_t b1 = vld1q_s32(&S[j + 6]);
int16x8_t s_a = vcombine_s16(vqmovn_s32(a0), vqmovn_s32(a1));
int16x8_t s_b = vcombine_s16(vqmovn_s32(b0), vqmovn_s32(b1));
int16x8_t co = vld1q_s16(&D16[j]);
acc_v0 = vmlal_s16(acc_v0, vget_low_s16(s_a), vget_low_s16(co));
acc_v1 = vmlal_s16(acc_v1, vget_high_s16(s_a), vget_high_s16(co));
acc2_v0 = vmlal_s16(acc2_v0, vget_low_s16(s_b), vget_low_s16(co));
acc2_v1 = vmlal_s16(acc2_v1, vget_high_s16(s_b), vget_high_s16(co));
}
{
int32x4_t s_acc = vaddq_s32(acc_v0, acc_v1);
int32x4_t s_acc2 = vaddq_s32(acc2_v0, acc2_v1);
#if defined(__aarch64__)
acc = vaddvq_s32(s_acc);
acc2 = vaddvq_s32(s_acc2);
#else
int32x2_t p = vadd_s32(vget_low_s32(s_acc), vget_high_s32(s_acc));
acc = vget_lane_s32(vpadd_s32(p, p), 0);
p = vadd_s32(vget_low_s32(s_acc2), vget_high_s32(s_acc2));
acc2 = vget_lane_s32(vpadd_s32(p, p), 0);
#endif
}
acc >>= 6;
acc2 >>= 6;
}
#else
j = 0;
#endif
/* Scalar tail handles whatever 0..7 taps the SIMD block didn't,
* and the entire window on builds without SSE2 or NEON. */
for (; j < ncoeffs; j++) {
acc += (S[j + 1] * (int32_t)D16[j]) >> 6;
acc2 += (S[j + 2] * (int32_t)D16[j]) >> 6;
}
*out_acc = acc;
*out_acc2 = acc2;
}
int32_t NeoFilterSound(int32_t *in, int32_t *out, uint32_t inlen, int32_t *leftover) {
uint32_t x;
int32_t *outsave = out;
@@ -196,9 +88,14 @@ int32_t NeoFilterSound(int32_t *in, int32_t *out, uint32_t inlen, int32_t *lefto
if (FSettings.soundq == 2) {
for (x = mrindex; x < max; x += mrratio) {
int32_t acc, acc2;
int32_t *S = &in[(x >> 16) - SQ2NCOEFFS];
fir_inner_kernel(S, sq2coeffs16, SQ2NCOEFFS, &acc, &acc2);
int32_t acc = 0, acc2 = 0;
uint32_t c;
int32_t *S, *D;
for (c = SQ2NCOEFFS, S = &in[(x >> 16) - SQ2NCOEFFS], D = sq2coeffs; c; c--, D++) {
acc += (S[c] * *D) >> 6;
acc2 += (S[1 + c] * *D) >> 6;
}
acc = ((int64_t)acc * (65536 - (x & 65535)) + (int64_t)acc2 * (x & 65535)) >> (16 + 11);
*out = acc;
@@ -207,9 +104,14 @@ int32_t NeoFilterSound(int32_t *in, int32_t *out, uint32_t inlen, int32_t *lefto
}
} else {
for (x = mrindex; x < max; x += mrratio) {
int32_t acc, acc2;
int32_t *S = &in[(x >> 16) - NCOEFFS];
fir_inner_kernel(S, coeffs16, NCOEFFS, &acc, &acc2);
int32_t acc = 0, acc2 = 0;
uint32_t c;
int32_t *S, *D;
for (c = NCOEFFS, S = &in[(x >> 16) - NCOEFFS], D = coeffs; c; c--, D++) {
acc += (S[c] * *D) >> 6;
acc2 += (S[1 + c] * *D) >> 6;
}
acc = ((int64_t)acc * (65536 - (x & 65535)) + (int64_t)acc2 * (x & 65535)) >> (16 + 11);
*out = acc;
@@ -266,17 +168,4 @@ void MakeFilters(int32_t rate) {
else
for (x = 0; x < (NCOEFFS >> 1); x++)
coeffs[x] = coeffs[NCOEFFS - 1 - x] = tmp[x];
/* Build the int16 mirror used by the SIMD inner kernel. All
* source FIR tables have max abs value ~21588 (fits in int16
* losslessly), so a straight narrowing cast preserves every
* coefficient bit. Done once per filter rebuild -- 1024 entries
* is well under 1 us. */
if (FSettings.soundq == 2) {
for (x = 0; x < SQ2NCOEFFS; x++)
sq2coeffs16[x] = (int16_t)sq2coeffs[x];
} else {
for (x = 0; x < NCOEFFS; x++)
coeffs16[x] = (int16_t)coeffs[x];
}
}