Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WebAssembly SIMD Intrinsic API Documentation #108

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

rrwinterton
Copy link

This is the markdown description of the WebAssembly SIMD Intrinsic API's

@alexcrichton
Copy link
Collaborator

FWIW this all roughly matches what we're doing in Rust to expose SIMD intrinsics (documented here), although there's two main differences:

  • We're dropping the wasm_ prefix proposed here and instead scoping it in a wasm module (I think the prefix makes sense for C/C++ though)
  • We're not currently distinguishing between types like i8x16 and f32x4, but rather there's only one v128 type which everything takes. Given that all the underlying instructions operate over a single v128, I wonder if it'd be best for C/C++ to reflect the same and take v128 everywhere?

@tlively
Copy link
Member

tlively commented Apr 19, 2019

@alexcrichton I was thinking that having separate types might be helpful for users. I don't think it's very important that WebAssembly's underlying type system be mirrored exactly at the source level, even for intrinsics. On the other hand, clang by default silently allows conversions between vector types of the same size, so users have to opt-in to getting any benefit from the separate types. Perhaps having separate types is not worth the extra effort. I would be interested in hearing more opinions about this.

@alexcrichton
Copy link
Collaborator

Makes sense! We originally used separate types awhile back in Rust as well. I think that the most important capability to enable here is what wasm actually exposes, which is passing any vector anywhere. We do that in Rust by taking v128 everywhere (so the types all match up), but it sounds clang enables the same behavior with implicit conversions.

In that sense I don't think the type names in C/C++ would matter too much since the underlying functionality shouldn't change too much. This header file, though, will likely be used to model other languages which may be more strict about types than Clang specifically is today. For example we won't be able to follow this specification in Rust due to the lack of implicit type coercions. In that sense it seems like it'd be nice to have languages exposing SIMD for wasm be consistent amongst themselves!

##### **Arithmetic Operations Definition** <a name=arithmetic-operations-definition></a>

~~~
i8x16 wasm_i8x16_add(i8x16 a i8x16 b)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing comma

@Maratyszcza
Copy link

I suggest that wasm_v128_load and wasm_v128_store should use generic pointers ([const] void*) rather than pointers to v128 type. This is preferable because applications are likely to store data in arrays of standard data types (e.g. float, uint8_t, or complex<float>), and having pointer casts everywhere would make codes using WASM intrinsics less readable. If loads/stores operate with generic (void*) pointers, these casts are not needed as any pointer can be implicitly cast to [const] void*`.

@gnzlbg
Copy link

gnzlbg commented May 8, 2019

@Maratyszcza IIUC, the main problem I see with v128* in the loads and stores is that WASM supports unaligned loads and stores (the alignment is part of the memarg immediate) but this API only supports aligned loads and stores because dereferencing a v128* is only valid if it is properly aligned to a multiple of v128 alignment.

and having pointer casts everywhere would make codes using WASM intrinsics less readable.

I don't think this is just an inconvenience. Casting a float* to a v128*, and passing it to these APIs, would invoke undefined behavior if the float* is not aligned to a multiple of the alignment of v128 (because the API does not support unaligned loads and stores).

Basically, the v128* type documents that this APIs only do aligned loads/stores. If you wanted to do unaligned ones, you'd need to use a different API.

@tlively
Copy link
Member

tlively commented May 8, 2019

Thanks for your comments, everyone! Let's move conversation to the implementing PR for now to avoid duplication. Once the implementation is merged we can circle back and fix up the documentation to match.

@sunfishcode
Copy link
Member

The Wasm SIMD intrinsics implementation PR for Emscripten is now merged. Is it time to circle back and fix up the documentation to match here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants