SIMD extract functions return unexpected types

[nemequ]

This might be intentional, if so sorry for the noise. I really hope not, though.

Most of the integer wasm_*_extract_lane function-like macros return int or, in the case of i64, long long. This causes a bit of awkwardness when attempting to use them with -Wimplicit-int-conversion active. For example,

int8_t e = wasm_i8x16_extract_lane(v, 0);

Triggers a diagnostic.

It's straightforward enough to work around with a cast, but I feel like the cast should go in the extract_lane macros in wasm_simd128.h (or maybe even the return type of the builtins it uses), not on the caller side.

CC @tlively since it's a SIMD thing.

[tlively]

This is intentional, but is also open for discussion. The idea when first implementing the intrinsics was to match the types of the underlying WebAssembly instructions as closely as possible. WebAssembly doesn't have i8 or i16 types, so the instructions return i32 and the intrinsics reflect that.

[nemequ]

That makes sense to me in theory, but is there any practical benefit? I'm having trouble thinking of examples where it solves a problem.

It may be consistent with the WASM types, but it's not consistent with C-level APIs. For example, wasm_i8x16_make takes a bunch of int8_t parameters, not ints. I know the load and store functions take void* so they can be anything, but if you want to load a vector of 16 8-bit values they're probably going to be represented in memory as an array of 16 × int8_t (or maybe char, signed char, etc.). They're definitely not going to be an array of 16 × int.

And, of course, you're much less likely to get a warning from the compiler about implicitly converting to a wider type than to a narrower type. I know clang (at least) has a double-promotion warning which is triggered by assigning a float to a double, but I don't think there is anything similar for assigning, for example, an int8_t to an int. So if wasm_i8x16_extract_lane returned an int8_t you could still assign the result to an int if you wanted to.

I guess it just feels like with the current API I'm fighting against the type system, when it should be helping me find bugs.

// Possible warning (depending on flags)
int8_t e = wasm_i8x16_extract_lane(v, 0);
// Fix that with a cast.
int8_t e = (int8_t) wasm_i8x16_extract_lane(v, 0);
// That's ugly, but at least it works.  Unfortunately, it is too broad
// and creates a dangerous pattern:
int8_t e = (int8_t) wasm_i16x8_extract_lane(v, 0);
// Even without the cast, this generates (or doesn't) the same
// diagnostic as the first one
int8_t e1 = wasm_i16x8_extract_lane(v, 0);

These are exactly the sort of issues the type system can help with. I don't see a reason not to let it.

[tlively]

Yeah, definitely makes sense to me. I'll file an issue on the SIMD repo to see if anyone else has thoughts about this, but I'd be in favor of making this change.

[stale]

This issue has been automatically marked as stale because there has been no activity in the past year. It will be closed automatically if no further activity occurs in the next 30 days. Feel free to re-open at any time if this issue is still relevant.