msf_gif icon indicating copy to clipboard operation
msf_gif copied to clipboard

Use of _mm_srli_epi32 with non-compile time constant second argument may be incorrect

Open NeRdTheNed opened this issue 10 months ago • 1 comments

Note: this issue was found when I modified msf_gif to use SIMDe to compile the SSE2 code for other platforms. For a non-modified version of msf_gif, this issue would only affect compilers which are pedantic when compiling SSE2 intrinsics, and I am unsure if it would actually affect any users of msf_gif in practice. Feel free to disregard this issue as out of scope / very limited impact.

When I was trying to compile a program using a modified version of msf_gif to use SIMDe to compile the SSE2 intrinsics with Clang for AArch64 macOS, a number of errors were raised related to some uses of _mm_srli_epi32:

./msf_gif.h:303:43: error: argument to '__builtin_neon_vshrq_n_v' must be a constant integer
                __m128i k2 = _mm_or_si128(_mm_srli_epi32(k, rbits), _mm_slli_epi32(_mm_srli_epi32(k, bbits), 16));
                             ~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
./simde/x86/sse2.h:22513:35: note: expanded from macro '_mm_srli_epi32'
  #define _mm_srli_epi32(a, imm8) simde_mm_srli_epi32(a, imm8)
                                  ^
./simde/x86/sse2.h:22492:11: note: expanded from macro 'simde_mm_srli_epi32'
          vshrq_n_u32(simde__m128i_to_neon_u32(a), ((imm8) & 31) | (((imm8) & 31) == 0))))
          ^
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/clang/14.0.0/include/arm_neon.h:25282:24: note: expanded from macro 'vshrq_n_u32'
  __ret = (uint32x4_t) __builtin_neon_vshrq_n_v((int8x16_t)__s0, __p1, 50); \
                       ^
./simde/x86/sse2.h:20821:48: note: expanded from macro '_mm_or_si128'
  #define _mm_or_si128(a, b) simde_mm_or_si128(a, b)
(similar errors omitted)

In simd-everywhere/simde#267, it was noted that the second argument for _mm_srli_epi32 should be a compile time constant according to Intel documentation (as it should be an immediate imm8), but some compilers will accept the use of a non-compile time constant argument. msf_gif calls _mm_srli_epi32 with a non-compile time constant argument in a few places. SIMDe assumes this argument is a constant, which results in this error on some platforms. Through testing on compiler explorer, this seems to affect compilation through SIMDe on most variants of ARM and a few other architectures. I am unsure if this affects any compiler targeting x86_64.

NeRdTheNed avatar Feb 15 '25 23:02 NeRdTheNed

Yep, this one slipped past me as I didn't get any warnings for it on the compilers that I test with. I'll have to take a look at what code the compilers are generating and figure out what's best/fastest to do here instead. It's low priority but I do want to get this fixed at some point.

notnullnotvoid avatar Feb 17 '25 04:02 notnullnotvoid