Use 64-bit integer representation in FFI turbocall

denoland · littledivy · May 30, 2024 · May 25, 2024 · May 25, 2024 · May 25, 2024
commit 6cf72cc701c9cbafa96468835ee534dda15e6baf
diff --git a/ext/ffi/00_ffi.js b/ext/ffi/00_ffi.js
@@ -51,7 +51,6 @@ const {
   TypedArrayPrototypeGetByteLength,
   TypeError,
   Uint8Array,
-  Int32Array,
   Uint32Array,
   BigInt64Array,
   BigUint64Array,
@@ -342,11 +341,6 @@ class UnsafeFnPointer {
   }
 }
 
-function isReturnedAsBigInt(type) {
-  return type === "u64" || type === "i64" ||
-    type === "usize" || type === "isize";
-}
-
 function isStruct(type) {
   return typeof type === "object" && type !== null &&
     typeof type.struct === "object";
@@ -517,7 +511,6 @@ class DynamicLibrary {
       const structSize = isStructResult
         ? getTypeSizeAndAlignment(resultType)[0]
         : 0;
-      const needsUnpacking = isReturnedAsBigInt(resultType);
 
       const isNonBlocking = symbols[symbol].nonblocking;
       if (isNonBlocking) {
@@ -553,27 +546,7 @@ class DynamicLibrary {
         );
       }
 
-      if (needsUnpacking && !isNonBlocking) {
-        const call = this.symbols[symbol];
-        const parameters = symbols[symbol].parameters;
-        const vi = new Int32Array(2);
-        const b = new BigInt64Array(TypedArrayPrototypeGetBuffer(vi));
-
-        const params = ArrayPrototypeJoin(
-          ArrayPrototypeMap(parameters, (_, index) => `p${index}`),
-          ", ",
-        );
-        // Make sure V8 has no excuse to not optimize this function.
-        this.symbols[symbol] = new Function(
-          "vi",
-          "b",
-          "call",
-          `return function (${params}) {
-            call(${params}${parameters.length > 0 ? ", " : ""}vi);
-            return b[0];
-          }`,
-        )(vi, b, call);
-      } else if (isStructResult && !isNonBlocking) {
+      if (isStructResult && !isNonBlocking) {
         const call = this.symbols[symbol];
         const parameters = symbols[symbol].parameters;
         const params = ArrayPrototypeJoin(

diff --git a/ext/ffi/dlfcn.rs b/ext/ffi/dlfcn.rs
@@ -52,17 +52,6 @@ impl DynamicLibraryResource {
   }
 }
 
-pub fn needs_unwrap(rv: &NativeType) -> bool {
-  matches!(
-    rv,
-    NativeType::I64 | NativeType::ISize | NativeType::U64 | NativeType::USize
-  )
-}
-
-fn is_i64(rv: &NativeType) -> bool {
-  matches!(rv, NativeType::I64 | NativeType::ISize)
-}
-
 #[derive(Deserialize, Debug)]
 #[serde(rename_all = "camelCase")]
 pub struct ForeignFunction {
@@ -242,10 +231,6 @@ fn make_sync_fn<'s>(
       // SAFETY: The pointer will not be deallocated until the function is
       // garbage collected.
       let symbol = unsafe { &*(external.value() as *const Symbol) };
-      let needs_unwrap = match needs_unwrap(&symbol.result_type) {
-        true => Some(args.get(symbol.parameter_types.len() as i32)),
-        false => None,
-      };
       let out_buffer = match symbol.result_type {
         NativeType::Struct(_) => {
           let argc = args.length();
@@ -261,35 +246,10 @@ fn make_sync_fn<'s>(
       };
       match crate::call::ffi_call_sync(scope, args, symbol, out_buffer) {
         Ok(result) => {
-          match needs_unwrap {
-            Some(v) => {
-              let view: v8::Local<v8::ArrayBufferView> = v.try_into().unwrap();
-              let pointer =
-                view.buffer(scope).unwrap().data().unwrap().as_ptr() as *mut u8;
-
-              if is_i64(&symbol.result_type) {
-                // SAFETY: v8::SharedRef<v8::BackingStore> is similar to Arc<[u8]>,
-                // it points to a fixed continuous slice of bytes on the heap.
-                let bs = unsafe { &mut *(pointer as *mut i64) };
-                // SAFETY: We already checked that type == I64
-                let value = unsafe { result.i64_value };
-                *bs = value;
-              } else {
-                // SAFETY: v8::SharedRef<v8::BackingStore> is similar to Arc<[u8]>,
-                // it points to a fixed continuous slice of bytes on the heap.
-                let bs = unsafe { &mut *(pointer as *mut u64) };
-                // SAFETY: We checked that type == U64
-                let value = unsafe { result.u64_value };
-                *bs = value;
-              }
-            }
-            None => {
-              let result =
-                // SAFETY: Same return type declared to libffi; trust user to have it right beyond that.
-                unsafe { result.to_v8(scope, symbol.result_type.clone()) };
-              rv.set(result);
-            }
-          }
+          let result =
+            // SAFETY: Same return type declared to libffi; trust user to have it right beyond that.
+            unsafe { result.to_v8(scope, symbol.result_type.clone()) };
+          rv.set(result);
         }
         Err(err) => {
           deno_core::_ops::throw_type_error(scope, err.to_string());

diff --git a/ext/ffi/turbocall.rs b/ext/ffi/turbocall.rs
@@ -9,7 +9,6 @@ use dynasmrt::dynasm;
 use dynasmrt::DynasmApi;
 use dynasmrt::ExecutableBuffer;
 
-use crate::dlfcn::needs_unwrap;
 use crate::NativeType;
 use crate::Symbol;
 
@@ -46,21 +45,18 @@ pub(crate) fn make_template(
   sym: &Symbol,
   trampoline: &Trampoline,
 ) -> fast_api::FastFunction {
-  let mut params = once(fast_api::Type::V8Value) // Receiver
+  let params = once(fast_api::Type::V8Value) // Receiver
     .chain(sym.parameter_types.iter().map(|t| t.into()))
     .collect::<Vec<_>>();
 
-  let ret = if needs_unwrap(&sym.result_type) {
-    params.push(fast_api::Type::TypedArray(fast_api::CType::Int32));
-    fast_api::CType::Void
-  } else if sym.result_type == NativeType::Buffer {
+  let ret = if sym.result_type == NativeType::Buffer {
     // Buffer can be used as a return type and converts differently than in parameters.
     fast_api::CType::Pointer
   } else {
     fast_api::CType::from(&fast_api::Type::from(&sym.result_type))
   };
 
-  fast_api::FastFunction::new(
+  fast_api::FastFunction::new_with_bigint(
     Box::leak(params.into_boxed_slice()),
     ret,
     trampoline.ptr(),
@@ -158,15 +154,9 @@ impl SysVAmd64 {
 
     let must_cast_return_value =
       compiler.must_cast_return_value(&sym.result_type);
-    let must_wrap_return_value =
-      compiler.must_wrap_return_value_in_typed_array(&sym.result_type);
-    let must_save_preserved_register = must_wrap_return_value;
-    let cannot_tailcall = must_cast_return_value || must_wrap_return_value;
+    let cannot_tailcall = must_cast_return_value;
 
     if cannot_tailcall {
-      if must_save_preserved_register {
-        compiler.save_preserved_register_to_stack();
-      }
       compiler.allocate_stack(&sym.parameter_types);
     }
 
@@ -177,22 +167,13 @@ impl SysVAmd64 {
       // the receiver object should never be expected. Avoid its unexpected or deliberate leak
       compiler.zero_first_arg();
     }
-    if must_wrap_return_value {
-      compiler.save_out_array_to_preserved_register();
-    }
 
     if cannot_tailcall {
       compiler.call(sym.ptr.as_ptr());
       if must_cast_return_value {
         compiler.cast_return_value(&sym.result_type);
       }
-      if must_wrap_return_value {
-        compiler.wrap_return_value_in_out_array();
-      }
       compiler.deallocate_stack();
-      if must_save_preserved_register {
-        compiler.recover_preserved_register();
-      }
       compiler.ret();
     } else {
       compiler.tailcall(sym.ptr.as_ptr());
@@ -555,12 +536,6 @@ impl SysVAmd64 {
     )
   }
 
-  fn must_wrap_return_value_in_typed_array(&self, rv: &NativeType) -> bool {
-    // V8 only supports i32 and u32 return types for integers
-    // We support 64 bit integers by wrapping them in a TypedArray out parameter
-    crate::dlfcn::needs_unwrap(rv)
-  }
-
   fn finalize(self) -> ExecutableBuffer {
     self.assmblr.finalize().unwrap()
   }
@@ -602,44 +577,15 @@ impl Aarch64Apple {
   fn compile(sym: &Symbol) -> Trampoline {
     let mut compiler = Self::new();
 
-    let must_wrap_return_value =
-      compiler.must_wrap_return_value_in_typed_array(&sym.result_type);
-    let must_save_preserved_register = must_wrap_return_value;
-    let cannot_tailcall = must_wrap_return_value;
-
-    if cannot_tailcall {
-      compiler.allocate_stack(sym);
-      compiler.save_frame_record();
-      if compiler.must_save_preserved_register_to_stack(sym) {
-        compiler.save_preserved_register_to_stack();
-      }
-    }
-
     for param in sym.parameter_types.iter().cloned() {
       compiler.move_left(param)
     }
     if !compiler.is_recv_arg_overridden() {
       // the receiver object should never be expected. Avoid its unexpected or deliberate leak
       compiler.zero_first_arg();
     }
-    if compiler.must_wrap_return_value_in_typed_array(&sym.result_type) {
-      compiler.save_out_array_to_preserved_register();
-    }
 
-    if cannot_tailcall {
-      compiler.call(sym.ptr.as_ptr());
-      if must_wrap_return_value {
-        compiler.wrap_return_value_in_out_array();
-      }
-      if must_save_preserved_register {
-        compiler.recover_preserved_register();
-      }
-      compiler.recover_frame_record();
-      compiler.deallocate_stack();
-      compiler.ret();
-    } else {
-      compiler.tailcall(sym.ptr.as_ptr());
-    }
+    compiler.tailcall(sym.ptr.as_ptr());
 
     Trampoline(compiler.finalize())
   }
@@ -980,10 +926,6 @@ impl Aarch64Apple {
     // > Each frame shall link to the frame of its caller by means of a frame record of two 64-bit values on the stack
     stack_size += 16;
 
-    if self.must_save_preserved_register_to_stack(symbol) {
-      stack_size += 8;
-    }
-
     // Section 6.2.2 of Aarch64 PCS:
     // > At any point at which memory is accessed via SP, the hardware requires that
     // > - SP mod 16 = 0. The stack must be quad-word aligned.
@@ -1064,16 +1006,6 @@ impl Aarch64Apple {
     self.integral_params > 0
   }
 
-  fn must_save_preserved_register_to_stack(&mut self, symbol: &Symbol) -> bool {
-    self.must_wrap_return_value_in_typed_array(&symbol.result_type)
-  }
-
-  fn must_wrap_return_value_in_typed_array(&self, rv: &NativeType) -> bool {
-    // V8 only supports i32 and u32 return types for integers
-    // We support 64 bit integers by wrapping them in a TypedArray out parameter
-    crate::dlfcn::needs_unwrap(rv)
-  }
-
   fn finalize(self) -> ExecutableBuffer {
     self.assmblr.finalize().unwrap()
   }
@@ -1117,15 +1049,9 @@ impl Win64 {
 
     let must_cast_return_value =
       compiler.must_cast_return_value(&sym.result_type);
-    let must_wrap_return_value =
-      compiler.must_wrap_return_value_in_typed_array(&sym.result_type);
-    let must_save_preserved_register = must_wrap_return_value;
-    let cannot_tailcall = must_cast_return_value || must_wrap_return_value;
+    let cannot_tailcall = must_cast_return_value;
 
     if cannot_tailcall {
-      if must_save_preserved_register {
-        compiler.save_preserved_register_to_stack();
-      }
       compiler.allocate_stack(&sym.parameter_types);
     }
 
@@ -1136,22 +1062,13 @@ impl Win64 {
       // the receiver object should never be expected. Avoid its unexpected or deliberate leak
       compiler.zero_first_arg();
     }
-    if must_wrap_return_value {
-      compiler.save_out_array_to_preserved_register();
-    }
 
     if cannot_tailcall {
       compiler.call(sym.ptr.as_ptr());
       if must_cast_return_value {
         compiler.cast_return_value(&sym.result_type);
       }
-      if must_wrap_return_value {
-        compiler.wrap_return_value_in_out_array();
-      }
       compiler.deallocate_stack();
-      if must_save_preserved_register {
-        compiler.recover_preserved_register();
-      }
       compiler.ret();
     } else {
       compiler.tailcall(sym.ptr.as_ptr());
@@ -1424,12 +1341,6 @@ impl Win64 {
     )
   }
 
-  fn must_wrap_return_value_in_typed_array(&self, rv: &NativeType) -> bool {
-    // V8 only supports i32 and u32 return types for integers
-    // We support 64 bit integers by wrapping them in a TypedArray out parameter
-    crate::dlfcn::needs_unwrap(rv)
-  }
-
   fn finalize(self) -> ExecutableBuffer {
     self.assmblr.finalize().unwrap()
   }

diff --git a/tests/ffi/tests/test.js b/tests/ffi/tests/test.js
@@ -420,7 +420,7 @@ function addU32Fast(a, b) {
 testOptimized(addU32Fast, () => addU32Fast(123, 456));
 
 function addU64Fast(a, b) { return add_usize_fast(a, b); };
-testOptimized(addU64Fast, () => addU64Fast(2, 3));
+testOptimized(addU64Fast, () => addU64Fast(2n, 3n));
 
 console.log(dylib.symbols.add_i32(123, 456));
 console.log(dylib.symbols.add_u64(0xffffffffn, 0xffffffffn));
@@ -578,7 +578,7 @@ function logManyParametersFast(a, b, c, d, e, f, g, h, i, j, k, l, m, n, o, p, q
 testOptimized(
   logManyParametersFast,
   () => logManyParametersFast(
-    255, 65535, 4294967295, 4294967296, 123.456, 789.876, -1, -2, -3, -4, -1000, 1000,
+    255, 65535, 4294967295, 4294967296n, 123.456, 789.876, -1n, -2, -3, -4, -1000n, 1000n,
     12345.678910, 12345.678910, 12345.678910, 12345.678910, 12345.678910, 12345.678910, 12345.678910
   )
 );