Create SIMD macros for WebAssembly and use them in simd_macros.cc - #44
Conversation
trsonic
left a comment
There was a problem hiding this comment.
Approving. The change only takes effect when __wasm_simd128__ is defined, so no existing target is affected.
Two things worth doing, here or in a follow-up:
- Route Wasm through the existing generic loops instead of the SSE blocks (inline comment on
simd_macros.h). - Nothing in CI compiles this branch. Please state the Emscripten flags used and confirm that
simd_utils_testpasses under node. A single Wasm job inci-cmake.ymlwould settle both:emcmake cmakewith-msimd128in the C and C++ flags,-DCMAKE_CROSSCOMPILING_EMULATOR=node, static library only, then the samectest -L 'oar|obr'step.
| #if !defined(DISABLE_SIMD) && defined(__wasm_simd128__) | ||
| // Wasm SIMD is enabled. | ||
| // Define __SSE__ for the xmmintrin header and SIMD_WASM for simd_utils.cc | ||
| #define __SSE__ |
There was a problem hiding this comment.
This define exists only because the #if defined(SIMD_SSE) || defined(SIMD_WASM) blocks in simd_utils.cc call _mm_loadu_ps, _mm_storeu_ps and _mm_shuffle_ps directly. Those blocks split aligned and unaligned cases because SSE has separate instructions for them. v128.load accepts any alignment, so the split buys nothing on Wasm. Every function on the render path (AddPointwise, SubtractPointwise, MultiplyPointwise, ScalarMultiply) already has a generic #else loop that NEON uses today, and MultiplyPointwise, which this PR leaves untouched, runs through that loop on Wasm as-is.
Suggestion: keep the #ifdef SIMD_SSE guards as they were and handle only the three functions that have no #else. None of them is called outside the tests.
ReciprocalSqrtandSqrt: change#elif defined SIMD_NEONto#elif defined(SIMD_NEON) || defined(SIMD_WASM).ApproxComplexMagnitude: add aSIMD_WASMbranch that replaces the two_mm_shuffle_pscalls withwasm_i32x4_shuffle(a, b, 0, 2, 4, 6)andwasm_i32x4_shuffle(a, b, 1, 3, 5, 7), or a scalar fallback.
Then this branch needs no xmmintrin.h. SIMD_RECIPROCAL_SQRT becomes wasm_f32x4_div(wasm_f32x4_splat(1.0f), wasm_f32x4_sqrt(a)), which is what Emscripten's _mm_rsqrt_ps expands to, and the v128_t/__m128 mixing that currently relies on clang's lax vector conversions goes away.
As written, the empty define also collides with the __SSE__ 1 that -msse predefines, which triggers -Wmacro-redefined.
This change allows building OAR, specifically OBR sub-renderer, with Wasm and take advantage of the SIMD optimizations.