assemblyscript icon indicating copy to clipboard operation
assemblyscript copied to clipboard

Drop isize / usize type support for v128 generic intrinsics

Open MaxGraey opened this issue 1 year ago • 11 comments

Motivation

Many intrinsics support usize / isize only partially (only i32 / u32 variants or i64 but not u64):

builtin_v128_gt // supports only I64 part but not U64
builtin_v128_ge // supports only I64 part but not U64
builtin_v128_lt // supports only I64 part but not U64
builtin_v128_le // supports only I64 part but not U64

builtin_v128_load32_splat
builtin_v128_convert
builtin_v128_convert_low
builtin_v128_trunc_sat
builtin_v128_trunc_sat_zero
builtin_v128_extend_low
builtin_v128_extend_high

This leads to IB (implementation behavior) between wasm32 / wasm64 which may require refactoring during migration.

But situation ever worst. v128.bitmask<usize> may return different results for wasm32 and wasm64. This is already UB which we can't afford.

Also, this inconsistent with non-generic functions which doesn't have according types. I'm not even sure if they are useful at all, considering that usize / isize are most often used for address arithmetic, while simd operations very rarely (never?) try to be used for such operations.

So, propose just remove usize / isize support for v128 generic intrinsics.

  • [x] I've read the contributing guidelines
  • [x] I've added my name and email to the NOTICE file

MaxGraey avatar Sep 01 '22 10:09 MaxGraey

Would say the IB/UB here originates from the existence of isize/usize, not so much from anything using them. Would agree that it is untypical to use them with SIMD, but doesn't seem very different from anything else?

dcodeIO avatar Sep 01 '22 10:09 dcodeIO

but doesn't seem very different from anything else?

Generally it is different because unlike scalar operations simd operations do not have the full range of commands for usize / isize. In addition, such commands as bitmask completely change the picture already in runtime, which does not happen with scalar operations

MaxGraey avatar Sep 01 '22 10:09 MaxGraey

The inverse perspective is also interesting I think: If someone indeed wants to use sized types with instructions supporting to do so, with the feature restricted, they have to fall back to static type checks, which could reasonably be considered an arbitrary limitation because it works for everything else.

dcodeIO avatar Sep 01 '22 10:09 dcodeIO

If someone wants to use usize for Simd they will just have two implementations:

if (ASC_TARGET == 2) { // Wasm64
  c =  i64x2.add(a, b)
   ...
} else if (ASC_TARGET == 1) { // Wasm32
  c =  i32x4.add(a, b)
  ...
}

MaxGraey avatar Sep 01 '22 10:09 MaxGraey

Or they could just do v128.add<isize>(a, b), since this instruction supports it. Would say that using SIMD already requires quite a bit of knowledge that is way deeper than understanding what an isize is.

dcodeIO avatar Sep 01 '22 10:09 dcodeIO

Or they could just do v128.add(a, b)

As I said above it can lead to many problems. I also forgot to mention replace_lane and extract_lane, which can lead to both compile errors and runtime errors.

If we only support usize / iszie for operations like add / sub, etc., it would be problematic for DX. Because users will not understand why some commands are supported and some are not.

MaxGraey avatar Sep 01 '22 10:09 MaxGraey

Thinking about this more, I begin to wonder whether the "problem" here is rather that the generic variants are problematic in general? Like, the same argument could be made about some instructions supporting integers but not floats (v128.add_sat is particularly strange, or vice-versa, say v128.div), or, in the sense of this issue, that some operations don't have 64-bit int ops. Would you say that the generic variants don't make a lot of sense in the first place, and these should just be the inline asm instructions?

dcodeIO avatar Sep 01 '22 12:09 dcodeIO

In principle, yes. I would give up the generic options at all for simd, but that would be significant breaking change. For beginning, I would like just get rid of isize / usize support for SIMD. For that, I wouldn't even mark this PR as "breaking changes". I don't think anyone uses them anyway, so you could just delete them with some notation.

MaxGraey avatar Sep 01 '22 13:09 MaxGraey

Bringing this up because I think that there is a consistency argument to be made here. Similar considerations apply to the MVP generics, and it's not very different for the atomics than it is for SIMD. My question hence is what'd be the end goal here, respectively where we'd draw the line. Seems like a matter of opinion to me, and in such cases it might make sense to simply allow and leave it up to the developer.

dcodeIO avatar Sep 01 '22 13:09 dcodeIO

I would leave everything as it is for now, except for usize / isize in simd. Because if we remove generics for atomics and simd at all it will definitely break the code people already have. So I suggest merge this PR as is. And in the future raise the subject again when we will ready for next breaking changes

MaxGraey avatar Sep 01 '22 13:09 MaxGraey

I am interested in thinking this through first, though. At the very least, we should know where we are heading, since removing and then hoping for the best doesn't convince me given that right now it's at least consistent w.r.t. allowing anything that's possible.

dcodeIO avatar Sep 01 '22 13:09 dcodeIO

So what we decide for this PR? Close / Merge ?

MaxGraey avatar Sep 25 '22 17:09 MaxGraey

Would prefer to keep it the way it is for now until we know more, respectively have an idea to make this more consistent in the bigger picture, not less.

dcodeIO avatar Sep 25 '22 17:09 dcodeIO