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

Naming wasm32 intrinsics #562

Closed
alexcrichton opened this issue Sep 6, 2018 · 13 comments
Closed

Naming wasm32 intrinsics #562

alexcrichton opened this issue Sep 6, 2018 · 13 comments
Labels

Comments

@alexcrichton
Copy link
Member

Right now for the wasm32-unknown-unknown target the naming of intrinsics is a bit up in the air. We don't have a document like Intel's providing a specification of all the intrinsics, nor do we really have intrinsic function signature at all! What we've been doing so far is taking instructions that aren't natively supported by LLVM (like growing memory but not an atomic cmpxchg) and making a Rust function with inputs/outputs that look the same as the instruction itself (or similar at least).

This then begs the question, though, how do we actually name these functions? Again, unlike Intel, we don't have a specification from wasm itself about what to name these functions. Also, unlike x86_64, we don't have much prior art in Clang (I think they're still using __builtin_wasm32_$name?). To that end, we have a few options of how to name these intrinsics:

Replace . with _ in the instructions

  • memory.size => memory_size
  • memory.grow => memory_grow
  • atomic.wake => atomic_wake
  • i32.atomic.wait => i32_atomic_wait
  • i8x16.extract_lane_s => i8x16_extract_lane_s

Pros: easy to remember, clear translation from the spec, clear how to handle future instructions
Cons: can't group intrinsics together, maybe more difficult to discover

Replace . with :: in all instructions

(and use internal modules for a hierarchy

  • memory.size => memory::size
  • memory.grow => memory::grow
  • atomic.wake => atomic::wake
  • i32.atomic.wait => i32::atomic::wait
  • i8x16.extract_lane_s => i8x16::extract_lane_s

Pros: works well for atomic::wake and one-. intrinsics, can group everything nicely for docs/discovering
Cons: works badly for i32::atomic::wait (groups badly with atomic::wake), lots of odules for some (i32::atomic::wait)

Group in modules semi-arbitrarily

  • memory.size => memory::size
  • memory.grow => memory::grow
  • atomic.wake => atomic::wake
  • i32.atomic.wait => atomic::i32_wait
  • i8x16.extract_lane_s => i8x16::extract_lane_s

Pros: definitely good for grouping, documenting, and discovery
Cons: unclear if it will work well for all future instructions, we're making up names in a few cases


That's at least what I can think of now, I'm curious to hear what others think!

@tlively
Copy link

tlively commented Sep 7, 2018

Work on defining the intrinsics in Clang is ongoing. I'll check in on its status and try to get what progress we have so far posted somewhere ASAP. In the meantime, I would recommend not getting too fancy with name translations. There are a few edge cases in the naming conventions that may be awkward under some naming schemes. For example, consider v8x16.shuffle, which is the only instruction with the "v8x16" prefix. For discoverability, I recommend grouping instructions by operation rather than by type at the highest level, but I don't have strong opinions on what it should look like beyond that.

@gnzlbg
Copy link
Contributor

gnzlbg commented Sep 7, 2018

There are also the f64x2.convert_s/i64x2 instructions that mix . with / in their name and use two different types in their name as well..

For discoverability, I recommend grouping instructions by operation rather than by type at the highest level, but I don't have strong opinions on what it should look like beyond that.

I would prefer this as well, e.g., shuffle_v8x16, convert_s_f64x2_i64x2, extract_lane_s_i8x16, extract_lane_s_i16x8, etc.

Using underscores, I see two schemas: op_{input types...}_{output type} or op_{output type}_{input_types...}. I prefer op_{output type}[_{input_types...}] where we only include the input types when they are relevant, e.g., conversions, but we don't have to include them if they are the same as the output type.

In any case, independently of what we do here, somebody can write a crate that provides different syntax to call these in stable Rust once these are stabilized, so as long as the naming scheme is consistent I'd be fine with it.

@alexcrichton
Copy link
Member Author

@tlively oh good to know! This may also be a better issue for the tool-conventions repo as I suspect we all want to agree on what to name these intrinsics!

@tlively
Copy link

tlively commented Sep 19, 2018

FYI The current plan for Clang builtins can be found here:

https://docs.google.com/a/google.com/spreadsheets/d/e/2PACX-1vQH8QTunTdrkxBr7uWOIizZ1N8efvS4RxxnssW-LWtq3fSACq0OWQ1ffgcR6yy4IHYFsylelkT0IzSk/pubhtml

A lot of functionality including almost all arithmetic is already built in to Clang in a target-independent manner, so we won't have separate builtin functions for that. Other things like shuffles are also already covered by target-independent builtins. There are also a couple strange edge cases when it comes to typing. For example, bitcast has three vector arguments but their types really don't matter, so I think we will implement custom typing for that builtin that can accept any vector type with the right size. Perhaps in Rust it would make sense to have a v128 type or trait that can be used for that kind of interface.

@alexcrichton
Copy link
Member Author

Ah it looks like that's not a public document (I can't access it myself at least)

@tlively
Copy link

tlively commented Sep 19, 2018

@alexcrichton
Copy link
Member Author

Ah that does indeed work, thanks!

We've been experimenting with sealed traits on powerpc for intrinsics and those may be applicable here too! We strive to model everything at compile time to ensure we don't get LLVM errors or codegen errors, so it may be the case that these intrinsics end up looking slightly different in Rust than they do in C

@gnzlbg
Copy link
Contributor

gnzlbg commented Sep 19, 2018

So what @tlively shows for clang is pretty much what I expected these to look like in clang too. Here, we could do something similar, but we don't have to. Also, it would be interesting to figure out what GCC does here (is it the same as clang?).

There are also a couple strange edge cases when it comes to typing. For example, bitcast has three vector arguments but their types really don't matter, so I think we will implement custom typing for that builtin that can accept any vector type with the right size. Perhaps in Rust it would make sense to have a v128 type or trait that can be used for that kind of interface.

I just expected bitselect to work on v128 and that's it. Those who want to do anything more complicated than that are probably better off using packed_simd's mask select anyways (and in C++ they can just use C++20's std::simd<T>).

@alexcrichton
Copy link
Member Author

I've been picking this up again today to see how far we can get with it using an updated LLVM, and it reminded me that I should mention @tlively using __builtin_* for clang means that we'll probably diverge in Rust. The __builtin_* business is a C-ism which we I think would prefer to avoid inheriting in Rust, but I wonder if we can figure out a set of mappings from instruction to intrinsic that we can both expose?

@tlively
Copy link

tlively commented Oct 4, 2018

@alexcrichton that makes sense. Even in C, I think most end users are going to use some header file with a less verbose interface rather than the raw __builtin_* functions. Such a header file might be a nice place to unify C and Rust interfaces.

@alexcrichton
Copy link
Member Author

Sounds like a great idea to me! I wasn't sure if a dedicated header was planned, but that matches intrinsics on x86 exactly I believe (or at least close enough)

@alexcrichton
Copy link
Member Author

I've made a formal proposal now to stabilize memory-related intrinsics, which would also establish an underscore convention for naming here

@alexcrichton
Copy link
Member Author

Decided to use underscores now!

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

No branches or pull requests

3 participants