-
Notifications
You must be signed in to change notification settings - Fork 246
Update JS integer lowering #256
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
Conversation
Updates JS integer lowering to match the component model explainer (assuming that WebAssembly/component-model#52 is merged). The main differences are that out-of-range numbers now wrap instead of throwing, and that non-integers are now rounded down instead of throwing.
) * Make `Docs::contents` `None` for regular comments. There was already a check to make it `None` for something with no comments whatsoever, but it was still `Some` if all of the comments on something were regular comments. This fixes that so that it only becomes `Some` once an actual doc comment is found. * fmt
|
|
||
| Intrinsic::ToInt32 => self.src.js(" | ||
| export function to_int32(val) { | ||
| return val >> 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should one-liners like this perhaps be inlined instead of going through an intrinsic? I'm not sure of the perf implications of using this indirection.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I put together some quick benchmarks, and it does seem to be slightly faster inline:
u32RoundtripBefore 7.65 µs/iter (7.15 µs … 41.26 µs) 7.19 µs 21.81 µs 23.23 µs
u32RoundtripAfter 7.42 µs/iter (6.84 µs … 67.78 µs) 7.12 µs 21.64 µs 22.67 µs
summary
u32RoundtripAfter
1.03x faster than u32RoundtripBefore
s32RoundtripBefore 7.34 µs/iter (6.84 µs … 79.61 µs) 6.89 µs 21.56 µs 22.68 µs
s32RoundtripAfter 6.83 µs/iter (6.75 µs … 7.12 µs) 6.89 µs 7.12 µs 7.12 µs
summary
s32RoundtripAfter
1.07x faster than s32RoundtripBefore
u64RoundtripBefore 66.25 µs/iter (59.18 µs … 676.27 µs) 61.51 µs 133.4 µs 141.81 µs
u64RoundtripAfter 64.96 µs/iter (57.94 µs … 323.74 µs) 61.22 µs 131.25 µs 143.86 µs
summary
u64RoundtripAfter
1.02x faster than u64RoundtripBefore
s64RoundtripBefore 64.33 µs/iter (57.48 µs … 295.28 µs) 60.06 µs 130.5 µs 140.59 µs
s64RoundtripAfter 63.77 µs/iter (57.17 µs … 404.7 µs) 59.76 µs 130.23 µs 136.73 µs
summary
s64RoundtripAfter
1.01x faster than s64RoundtripBefore
(the 'after' benchmarks are with the conversions inlined)
It's not a massive difference, though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah ok that seems negligible enough that having a uniform representation here is probably worth it, thanks for measuring though!
|
|
||
| Intrinsic::ToBigInt64 => self.src.js(" | ||
| export function to_int64(val) { | ||
| return BigInt.asIntN(64, val); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar to above it may be better to do this inline instead of as an intrinsic perhaps?
…ust (bytecodealliance#257) This was triggering some warnings when running tests.
Values are now coerced to numbers instead of throwing.
This updates to `wasmtime` v0.38.0. Because module linking support has been removed from `wasmtime` as of v0.36.0, and the component model implementation is not yet mature enough to use as a replacement (see bytecodealliance/wasmtime#4303 for details), it is necessary to temporarily disable the Spidermonkey tests. I've done this by commenting-out the broken code in `instantiate_smw`, prefixed its name with and underscore, and renamed the relevant host.rs files to ignore_host.rs to hide them from the test infrastructure. Once the component model implementation is mature enough and the broader ecosystem has been updated to use it, we can update and restore the Spidermonkey tests. fixes bytecodealliance#259 Signed-off-by: Joel Dice <[email protected]>
* Implement `std::fmt::Write` for `Source` * Rework C backend to use `write!` instead of `format!` * Add `uwrite!` and `uwriteln!` * Properly link to `writeln!` in docs
* Implement parsing for the `future` type. Future has one type. In the ABI, it's represented as an i32 index. Lifting and lowering is not yet implemented. * Fix future syntax in a test. * Add `future` and `stream` to WIT.md. * Update types.wit.result with the new expected results.
) Part of the work in commit 2430448 seems to have been reversed at some point. `print_typedef_enum` will still generate an impl of `std::error::Error` if the name of the enum contains "errno", but this isn't as simple of a fix because `core` doesn't have an `Error` trait. Signed-off-by: Klim Tsoutsman <[email protected]>
Closes bytecodealliance#274 Signed-off-by: Klim Tsoutsman <[email protected]>
…tecodealliance#278) A couple of keywords were already escaped, but not all of them; this changes it to escape all keywords.
* fix(gen-rust-wasm): get rid of unused unit warning * Revert "fix(gen-rust-wasm): get rid of unused unit warning" This reverts commit c2f47bd. * fix: clippy lint warning in generated code
…nce#276) WIT.md had an outdated definition of identifiers, which only allowed ASCII and included the old method of putting identifiers in quotes to escape keywords. This removes that definition in favour of the up-to-date definition further down in the file.
* Implement syntax support for resource subtyping.
This implements a basic syntax for [resource subtyping], using the
`implements` keyword:
```
resource file implements preopen {
...
}
```
I'm not attached to this specific syntax or keyword name, but it is
useful to have something that we can start using to mock up wit files.
[resource subtyping]: WebAssembly/component-model#60
* rustfmt
* Fix compilation in a test.
This commit refactors encoding in `wit-component` to only export types from the default interface and stop exporting types from instance types used in imports. It also makes "type only" encodings more explicit with `InterfaceEncoder` that will encode the types from a single interface only.
- The parser would previously panic on empty doc comments (`/**/`), since it would interpret the `/**` as the start of a doc comment, leaving the block comment with `/` instead of `*/` at the end and tripping an assertion. - The parser would panic on `.md` files, since it assumed they were `.wit.md` files and tried to strip the nonexistent `.wit`. - The parser would panic if the input path did not end in a file. I found these while trying out `cargo fuzz`.
…iance#294) * tests: fix warnings in union test case. * wit-bindgen-rust: use derived name for return area type. This commit ensures that the return area type name is based on the name of the interface being generated. It prevents type name collisions when multiple interfaces are exported when using the `export!` macro generated by the "stand alone" mode (i.e. for `cargo component`).
) * wit-component: emit function type encodings first. This commit ensures that components encoded with `wit-component` have the function types from the exports encoded before the instance types for instance imports. Previously, if an instance import defined a type it would be inserted into a map tracking duplicate types and not exported; when the default interface was later processed with a matching type, the type index was reused without adding an export for the type. By encoding export function types first, it guarantees that the default exported interface has its named types exported. * wit-component: add test case to ensure default types are exported.
…ecodealliance#296) * wit-bindgen-rust: fix lowering and lifting of zero-length lists. This commit fixes the Rust bindings generated for lifting and lowering of lists to account for lists of zero-length. It is undefined behavior to call `std::alloc::alloc` with a zero-sized layout. Additionally, as the behavior of the `canonical_abi_realloc` implemented in `wit-bindgen` is to return a non-null dummy pointer for a zero-sized alloc, the bindings were calling `std::alloc::dealloc` with a pointer that was not allocated with `std::alloc::alloc` (also undefined behavior). To fix this, the bindings now account for zero-length lists and skip allocation and deallocation where appropriate. * Update `list` runtime tests to pass empty lists and strings.
* Bump version * Update all crate versions to 0.2.0 Co-authored-by: Kyle Brown <[email protected]>
* initial work * Fix formatting * Fix references to Rust library for guest code Improve documentation Other minor fixes * Fix wastime -> host_wasmtime_rust references Fixed feature flags * Fix demo deploy update readme improve help text * Update readme Co-authored-by: Kyle Brown <[email protected]> Co-authored-by: Kyle Brown <[email protected]>
|
Apologies for not merging this at the time I'm ot sure why I didn't earlier. My merge with |
Updates JS integer lowering to match the component model explainer (assuming that WebAssembly/component-model#52 is merged).
The main differences are that out-of-range numbers now wrap instead of throwing, and that non-integers are now rounded down instead of throwing.