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

Optimized ABI for Option<u32> and other 32-bit primitives #4183

Merged
merged 15 commits into from
Oct 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@
* Deprecate `autofocus`, `tabIndex`, `focus()` and `blur()` bindings in favor of bindings on the inherited `Element` class.
[#4143](https://github.com/rustwasm/wasm-bindgen/pull/4143)

* Optimized ABI performance for `Option<{i32,u32,isize,usize,f32,*const T,*mut T}>`.
[#4183](https://github.com/rustwasm/wasm-bindgen/pull/4183)

### Fixed

* Fixed methods with `self: &Self` consuming the object.
Expand Down
64 changes: 60 additions & 4 deletions crates/cli-support/src/js/binding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ pub struct JsBuilder<'a, 'b> {
/// JS functions, etc.
cx: &'a mut Context<'b>,

/// A debug name for the function being generated, used for error messages
debug_name: &'a str,

/// The "prelude" of the function, or largely just the JS function we've
/// built so far.
prelude: String,
Expand Down Expand Up @@ -121,6 +124,7 @@ impl<'a, 'b> Builder<'a, 'b> {
asyncness: bool,
variadic: bool,
generate_jsdoc: bool,
debug_name: &str,
) -> Result<JsFunction, Error> {
if self
.cx
Expand All @@ -138,7 +142,7 @@ impl<'a, 'b> Builder<'a, 'b> {
// If this is a method then we're generating this as part of a class
// method, so the leading parameter is the this pointer stored on
// the JS object, so synthesize that here.
let mut js = JsBuilder::new(self.cx);
let mut js = JsBuilder::new(self.cx, debug_name);
if let Some(consumes_self) = self.method {
let _ = params.next();
if js.cx.config.debug {
Expand Down Expand Up @@ -184,7 +188,12 @@ impl<'a, 'b> Builder<'a, 'b> {
)?;
}

assert_eq!(js.stack.len(), adapter.results.len());
assert_eq!(
js.stack.len(),
adapter.results.len(),
"stack size mismatch for {}",
debug_name
);
match js.stack.len() {
0 => {}
1 => {
Expand Down Expand Up @@ -422,9 +431,10 @@ impl<'a, 'b> Builder<'a, 'b> {
}

impl<'a, 'b> JsBuilder<'a, 'b> {
pub fn new(cx: &'a mut Context<'b>) -> JsBuilder<'a, 'b> {
pub fn new(cx: &'a mut Context<'b>, debug_name: &'a str) -> JsBuilder<'a, 'b> {
JsBuilder {
cx,
debug_name,
args: Vec::new(),
tmp: 0,
pre_try: String::new(),
Expand Down Expand Up @@ -463,7 +473,10 @@ impl<'a, 'b> JsBuilder<'a, 'b> {
}

fn pop(&mut self) -> String {
self.stack.pop().unwrap()
match self.stack.pop() {
Some(s) => s,
None => panic!("popping an empty stack in {}", self.debug_name),
}
}

fn push(&mut self, arg: String) {
Expand Down Expand Up @@ -920,6 +933,44 @@ fn instruction(
js.push(format!("isLikeNone({0}) ? {1} : {0}", val, hole));
}

Instruction::F64FromOptionSentinelInt { signed } => {
let val = js.pop();
js.cx.expose_is_like_none();
js.assert_optional_number(&val);

// We need to convert the given number to a 32-bit integer before
// passing it to the ABI for 2 reasons:
// 1. Rust's behavior for `value_f64 as i32/u32` is different from
// the WebAssembly behavior for values outside the 32-bit range.
// We could implement this behavior in Rust too, but it's easier
// to do it in JS.
// 2. If we allowed values outside the 32-bit range, the sentinel
// value itself would be allowed. This would make it impossible
// to distinguish between the sentinel value and a valid value.
//
// To perform the actual conversion, we use JS bit shifts. Handily,
// >> and >>> perform a conversion to i32 and u32 respectively
// to apply the bit shift, so we can use e.g. x >>> 0 to convert to
// u32.

let op = if *signed { ">>" } else { ">>>" };
js.push(format!("isLikeNone({val}) ? 0x100000001 : ({val}) {op} 0"));
RunDevelopment marked this conversation as resolved.
Show resolved Hide resolved
}
Instruction::F64FromOptionSentinelF32 => {
let val = js.pop();
js.cx.expose_is_like_none();
js.assert_optional_number(&val);

// Similar to the above 32-bit integer variant, we convert the
// number to a 32-bit *float* before passing it to the ABI. This
// ensures consistent behavior with WebAssembly and makes it
// possible to use a sentinel value.

js.push(format!(
"isLikeNone({val}) ? 0x100000001 : Math.fround({val})"
));
}

Instruction::FromOptionNative { ty } => {
let val = js.pop();
js.cx.expose_is_like_none();
Expand Down Expand Up @@ -1277,6 +1328,11 @@ fn instruction(
));
}

Instruction::OptionF64Sentinel => {
let val = js.pop();
js.push(format!("{0} === 0x100000001 ? undefined : {0}", val));
}

Instruction::OptionU32Sentinel => {
let val = js.pop();
js.push(format!("{0} === 0xFFFFFF ? undefined : {0}", val));
Expand Down
27 changes: 12 additions & 15 deletions crates/cli-support/src/js/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2655,6 +2655,16 @@ __wbg_set_wasm(wasm);"
ContextAdapterKind::Adapter => {}
}

// an internal debug name to help with error messages
let debug_name = match kind {
ContextAdapterKind::Import(i) => {
let i = builder.cx.module.imports.get(i);
format!("import of `{}::{}`", i.module, i.name)
}
ContextAdapterKind::Export(e) => format!("`{}`", e.debug_name),
ContextAdapterKind::Adapter => format!("adapter {}", id.0),
};

// Process the `binding` and generate a bunch of JS/TypeScript/etc.
let binding::JsFunction {
ts_sig,
Expand All @@ -2674,22 +2684,9 @@ __wbg_set_wasm(wasm);"
asyncness,
variadic,
generate_jsdoc,
&debug_name,
)
.with_context(|| match kind {
ContextAdapterKind::Export(e) => {
format!("failed to generate bindings for `{}`", e.debug_name)
}
ContextAdapterKind::Import(i) => {
let i = builder.cx.module.imports.get(i);
format!(
"failed to generate bindings for import of `{}::{}`",
i.module, i.name
)
}
ContextAdapterKind::Adapter => {
"failed to generates bindings for adapter".to_string()
}
})?;
.with_context(|| "failed to generates bindings for ".to_string() + &debug_name)?;

self.typescript_refs.extend(ts_refs);

Expand Down
30 changes: 22 additions & 8 deletions crates/cli-support/src/wit/incoming.rs
Original file line number Diff line number Diff line change
Expand Up @@ -276,13 +276,13 @@ impl InstructionBuilder<'_, '_> {
&[AdapterType::I32],
);
}
Descriptor::I8 => self.in_option_sentinel(AdapterType::S8),
Descriptor::U8 => self.in_option_sentinel(AdapterType::U8),
Descriptor::I16 => self.in_option_sentinel(AdapterType::S16),
Descriptor::U16 => self.in_option_sentinel(AdapterType::U16),
Descriptor::I32 => self.in_option_native(ValType::I32),
Descriptor::U32 => self.in_option_native(ValType::I32),
Descriptor::F32 => self.in_option_native(ValType::F32),
Descriptor::I8 => self.in_option_sentinel32(AdapterType::S8),
Descriptor::U8 => self.in_option_sentinel32(AdapterType::U8),
Descriptor::I16 => self.in_option_sentinel32(AdapterType::S16),
Descriptor::U16 => self.in_option_sentinel32(AdapterType::U16),
Descriptor::I32 => self.in_option_sentinel64_int(AdapterType::I32, true),
Descriptor::U32 => self.in_option_sentinel64_int(AdapterType::U32, false),
Descriptor::F32 => self.in_option_sentinel64_f32(AdapterType::F32),
Descriptor::F64 => self.in_option_native(ValType::F64),
Descriptor::I64 | Descriptor::U64 => self.in_option_native(ValType::I64),
Descriptor::Boolean => {
Expand Down Expand Up @@ -459,11 +459,25 @@ impl InstructionBuilder<'_, '_> {
);
}

fn in_option_sentinel(&mut self, ty: AdapterType) {
fn in_option_sentinel32(&mut self, ty: AdapterType) {
self.instruction(
&[ty.option()],
Instruction::I32FromOptionU32Sentinel,
&[AdapterType::I32],
);
}
fn in_option_sentinel64_int(&mut self, ty: AdapterType, signed: bool) {
self.instruction(
&[ty.option()],
Instruction::F64FromOptionSentinelInt { signed },
&[AdapterType::F64],
);
}
fn in_option_sentinel64_f32(&mut self, ty: AdapterType) {
self.instruction(
&[ty.option()],
Instruction::F64FromOptionSentinelF32,
&[AdapterType::F64],
);
}
}
24 changes: 16 additions & 8 deletions crates/cli-support/src/wit/outgoing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -257,15 +257,15 @@ impl InstructionBuilder<'_, '_> {
&[AdapterType::NamedExternref(name.clone()).option()],
);
}
Descriptor::I8 => self.out_option_sentinel(AdapterType::S8),
Descriptor::U8 => self.out_option_sentinel(AdapterType::U8),
Descriptor::I16 => self.out_option_sentinel(AdapterType::S16),
Descriptor::U16 => self.out_option_sentinel(AdapterType::U16),
Descriptor::I32 => self.option_native(true, ValType::I32),
Descriptor::U32 => self.option_native(false, ValType::I32),
Descriptor::I8 => self.out_option_sentinel32(AdapterType::S8),
Descriptor::U8 => self.out_option_sentinel32(AdapterType::U8),
Descriptor::I16 => self.out_option_sentinel32(AdapterType::S16),
Descriptor::U16 => self.out_option_sentinel32(AdapterType::U16),
Descriptor::I32 => self.out_option_sentinel64(AdapterType::S32),
Descriptor::U32 => self.out_option_sentinel64(AdapterType::U32),
Descriptor::I64 => self.option_native(true, ValType::I64),
Descriptor::U64 => self.option_native(false, ValType::I64),
Descriptor::F32 => self.option_native(true, ValType::F32),
Descriptor::F32 => self.out_option_sentinel64(AdapterType::F32),
Descriptor::F64 => self.option_native(true, ValType::F64),
Descriptor::Boolean => {
self.instruction(
Expand Down Expand Up @@ -576,11 +576,19 @@ impl InstructionBuilder<'_, '_> {
);
}

fn out_option_sentinel(&mut self, ty: AdapterType) {
fn out_option_sentinel32(&mut self, ty: AdapterType) {
self.instruction(
&[AdapterType::I32],
Instruction::OptionU32Sentinel,
&[ty.option()],
);
}

fn out_option_sentinel64(&mut self, ty: AdapterType) {
self.instruction(
&[AdapterType::F64],
Instruction::OptionF64Sentinel,
&[ty.option()],
);
}
}
10 changes: 10 additions & 0 deletions crates/cli-support/src/wit/standard.rs
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,14 @@ pub enum Instruction {
I32FromOptionEnum {
hole: u32,
},
/// Pops an `externref` from the stack, pushes either a sentinel value if it's
/// "none" or the integer value of it if it's "some"
F64FromOptionSentinelInt {
signed: bool,
},
/// Pops an `externref` from the stack, pushes either a sentinel value if it's
/// "none" or the f32 value of it if it's "some"
F64FromOptionSentinelF32,
/// Pops any externref from the stack and then pushes two values. First is a
/// 0/1 if it's none/some and second is `ty` value if it was there or 0 if
/// it wasn't there.
Expand Down Expand Up @@ -324,6 +332,8 @@ pub enum Instruction {
kind: VectorKind,
mem: walrus::MemoryId,
},
/// pops f64, pushes it viewed as an optional value with a known sentinel
OptionF64Sentinel,
/// pops i32, pushes it viewed as an optional value with a known sentinel
OptionU32Sentinel,
/// pops an i32, then `ty`, then pushes externref
Expand Down
Loading