Optimized ABI performance for Option<[all 32-bit primitives]> (#4183)

Author: RunDevelopment

CHANGELOG.md

@@ -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.

crates/cli-support/src/js/binding.rs

@@ -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,
@@ -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
@@ -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 {
@@ -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 => {
@@ -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(),
@@ -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) {
@@ -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"));
+        }
+        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();
@@ -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));

crates/cli-support/src/js/mod.rs

@@ -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,
@@ -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);
 

crates/cli-support/src/wit/incoming.rs

@@ -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 => {
@@ -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],
+        );
+    }
 }

crates/cli-support/src/wit/outgoing.rs

@@ -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(
@@ -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()],
+        );
+    }
 }

crates/cli-support/src/wit/standard.rs

@@ -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.
@@ -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