Skip to content

simd: consolidate common load/store and abs intrinsics into simd.h - #201

Merged
skal65535 merged 2 commits into
mainfrom
skal/simd
Sep 23, 2026
Merged

skal65535 merged 2 commits into
mainfrom
skal/simd

Conversation

@skal65535

@skal65535 skal65535 commented Sep 20, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Consolidate common SSE2 / SSSE3 / AVX2 / NEON load, store, and absolute value vector intrinsics into a new centralized header src/simd.h, eliminating raw load/store intrinsics across all .cc implementation files.

Details

  • Centralized SIMD Primitives (src/simd.h):
    • LOAD_16(src), LOAD_ALIGNED_16(src), STORE_16(val, dst), STORE_ALIGNED_16(val, dst)
    • LOAD_64(src), STORE_64(val, dst)
    • LOAD_32(src), LOAD_ALIGNED_32(src), STORE_32(val, dst), STORE_ALIGNED_32(val, dst)
    • ABS_16(v) (SSSE3 _mm_abs_epi16 with SSE2 3-op fallback, NEON vabsq_s16)
    • ABS_32(v) (_mm256_abs_epi16)
  • Macro Condensation for NEON:
    • Factored NeonVector16 / NeonVector8 type reinterpretation operators and Store16Neon / Store8Neon overloads into concise macro generators (SJPEG_NEON16_OP, SJPEG_NEON8_OP, SJPEG_STORE_NEON).
  • Codebase Clean-Up:
    • Removed duplicate local macro definitions across src/colors_rgb.cc, src/quantize.cc, src/histogram_avx2.cc, and src/yuv_convert.cc.
    • Standardized all STORE_16 calls to (value, destination).
    • Replaced raw _mm_loadu_*, _mm_storeu_*, _mm256_loadu_*, _mm256_storeu_*, and _mm_loadl_epi64 calls across src/histogram.cc, src/quantize_avx2.cc, src/riskiness_avx2.cc, src/yuv_convert_avx2.cc, src/fdct.cc, and src/fdct_avx2.cc.
    • Switched QuantizeBlockSSE2 and QuantizeErrorSSE2 in src/quantize.cc to ABS_16, enabling automatic SSSE3 hardware acceleration.
  • Drive-by Fix:
    • Added Encoder::SetRDO(bool) setter in src/enc.cc and used it in src/api.cc to avoid direct private member access, with unit test coverage in tests/unit_test.cc.
  • Build System:
    • Added src/simd.h to both CMakeLists.txt and Makefile.

@skal65535 skal65535 self-assigned this Sep 20, 2026
@skal65535
skal65535 requested a review from jzern September 20, 2026 06:48
skal65535 added a commit that referenced this pull request Sep 22, 2026
Shared SIMD infra belongs in simd.h (PR #201), not sjpegi.h. Revert
the cross-file sharing and keep HorizontalSumS32/S16 duplicated
locally in riskiness_avx2.cc and jpeg_tools.cc for now; both still
drop the redundant u16 gray-count variant. TODO markers point at
simd.h for the follow-up once #201 lands.
Comment thread src/simd.h
Comment thread src/simd.h
Comment thread src/simd.h
Comment thread src/simd.h Outdated
Comment thread src/simd.h Outdated
Introduces src/simd.h with shared SSE2/AVX2/NEON helpers:
- LOAD_16/32, STORE_16/32, LOAD_64/STORE_64 unaligned/aligned macros
- ABS_16/ABS_32 (16-bit absolute value)
- HorizontalSumS32/HorizontalSumS16 (vector-to-scalar reduction)

Replaces the ad hoc load/store/abs intrinsics duplicated across
quantize.cc, histogram.cc, fdct.cc, yuv_convert.cc and their AVX2
variants with these shared macros. The horizontal-sum helpers aren't
consumed yet -- riskiness_avx2.cc and jpeg_tools.cc's NEON ScoreRow
keep local copies pending a follow-up switch-over.
* simd.h: rename Neon-suffixed helpers to _NEON (drift from _SSE2)
* simd.h: drop dead !__cplusplus fallback (never included from .c)
* colors_rgb_avx2.cc: use LOAD_32/LOAD_ALIGNED_32/STORE_32
* riskiness_avx2.cc: use LOAD_16, reuse HorizontalSumS32/S16
@skal65535
skal65535 merged commit c38b145 into main Sep 23, 2026
13 checks passed
Comment thread src/simd.h
struct NeonVector16 {
uint8x16_t v;
operator uint8x16_t() const { return v; }
#define SJPEG_NEON16_OP(T, S) \

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A little too fancy for msvc since all vectors are typedef'd to a union:

typedef union __declspec(intrin_type) _ADVSIMD_ALIGN(16) __n128
{
     unsigned __int64   n128_u64[2];
     unsigned __int32   n128_u32[4];
     unsigned __int16   n128_u16[8];
     unsigned __int8    n128_u8[16];
     __int64            n128_i64[2];
     __int32            n128_i32[4];
     __int16            n128_i16[8];
     __int8             n128_i8[16];
     poly64_t           n128_p64[2];
     poly32_t           n128_p32[4];
     poly16_t           n128_p16[8];
     poly8_t            n128_p8[16];
     float              n128_f32[4];
    double              n128_f64[2];

    struct
    {
        __n64  low64;
        __n64  high64;
    } DUMMYNEONSTRUCT;

} __n128;
typedef __n128   uint8x16_t;
src\simd.h(147,3): error C2535: 'NeonVector16::operator uint8x16_t(void) const': member function already defined or
 declared
  (compiling source file '../src/jpeg_tools.cc')
      C:\src\external\sjpeg\src\simd.h(144,3):
      see declaration of 'NeonVector16::operator uint8x16_t'

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

=> Is #211 fixing the issue?

@skal65535
skal65535 deleted the skal/simd branch October 4, 2026 06:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants