db-tu-dresden / TSL

Template SIMD Library (+Generator)
GNU General Public License v3.0
9 stars 8 forks source link

[NEON] Shift left requires constant integer #95

Closed lawben closed 4 months ago

lawben commented 4 months ago

When downloading the current tar.gz from v0.1.9-rc5 (and probably ones before that but I didn't check), I cannot compile the NEON stuff on Mac with LLVM/Clang 18. I get the following error multiple times.

cmake-build-release/_deps/tsl_gen-src/generate_tsl_neon-asimd/include/generated/definitions/binary/binary_neon.hpp:1117:104: error: argument to '__builtin_neon_vshlq_n_v' must be a constant integer
 1117 |                 return __extension__ ({ uint8x16_t __ret; uint8x16_t __s0 = data; __ret = (uint8x16_t) __builtin_neon_vshlq_n_v((int8x16_t)__s0, shift, 48); __ret; });

which is the expanded macro for:

[[nodiscard]] 
TSL_FORCE_INLINE 
static typename Vec::register_type apply(
    const typename Vec::register_type data, const unsigned int shift
) {
    return vshlq_n_u8(data, shift);  // <-- this is a macro in Clang
}

When looking at the NEON specs, this error is correct, as vshlq_n_u8 requires a const int as the second argument. I'm not sure how this should be handled, but probably this needs a vdup_n_* for the runtime value before the shift.

JPietrzykTUD commented 4 months ago

The most straightforward way (yet, probably not very efficient) would be to fallback to vshlq_u8:

/*...*/
return vshlq_u8(data, vdupq_n_u8(shift));

Another approach would be to change the interface and pass the shift value as an integral constant.

lawben commented 4 months ago

FYI: This also hold for vshrq_n_* (right shift). The common fix for this is to use a left shift with negative values.

JPietrzykTUD commented 4 months ago

This should be fixed with #97.

alexKrauseTUD commented 4 months ago

Thank you for pointing out the issue! As for the common fix: AFAIK left-shift using negative values is undefined behavior and not standard conforming.

I would thus not include the workaround and rather rely on the vdup for now until we come up with something better.

lawben commented 4 months ago

That may be true fur C++ as a language, but specifically in NEON, this is well-defined and documented:

If the signed integer value is negative, it results in a right shift.