From 6cf72cc701c9cbafa96468835ee534dda15e6baf Mon Sep 17 00:00:00 2001 From: Aapo Alasuutari Date: Sat, 25 May 2024 13:21:14 +0300 Subject: [PATCH 01/10] Use 64-bit integer representation in FFI turbocall --- ext/ffi/00_ffi.js | 29 +----------- ext/ffi/dlfcn.rs | 48 ++----------------- ext/ffi/turbocall.rs | 101 +++------------------------------------- tests/ffi/tests/test.js | 4 +- 4 files changed, 13 insertions(+), 169 deletions(-) diff --git a/ext/ffi/00_ffi.js b/ext/ffi/00_ffi.js index 7f39db13e96892..b0a151ed5239cd 100644 --- 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 index bd46f14b23e59f..02ab4bb6abe16e 100644 --- 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 = 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 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 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 index 0417da6332cbc1..e0cf4e01763d5b 100644 --- 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::>(); - 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,19 +577,6 @@ 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) } @@ -622,24 +584,8 @@ impl Aarch64Apple { // 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 index fccf6b35e8f27f..3a6af270d89b2c 100644 --- 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 ) ); From 79579c3aa7805f4202e553cb376a285b47b61d5a Mon Sep 17 00:00:00 2001 From: Aapo Alasuutari Date: Sat, 25 May 2024 14:14:00 +0300 Subject: [PATCH 02/10] Use BigInt repr in op_ffi_read_u64, op_ffi_read_i64 and op_ffi_ptr_value --- ext/ffi/00_ffi.js | 36 +++++++++----------------------- ext/ffi/repr.rs | 52 +++++++++++------------------------------------ 2 files changed, 22 insertions(+), 66 deletions(-) diff --git a/ext/ffi/00_ffi.js b/ext/ffi/00_ffi.js index b0a151ed5239cd..06caf7c6cc161b 100644 --- a/ext/ffi/00_ffi.js +++ b/ext/ffi/00_ffi.js @@ -42,18 +42,14 @@ const { ArrayBufferPrototypeGetByteLength, ArrayPrototypeMap, ArrayPrototypeJoin, + BigInt, DataViewPrototypeGetByteLength, ObjectDefineProperty, ObjectHasOwn, ObjectPrototypeIsPrototypeOf, - NumberIsSafeInteger, - TypedArrayPrototypeGetBuffer, TypedArrayPrototypeGetByteLength, TypeError, Uint8Array, - Uint32Array, - BigInt64Array, - BigUint64Array, Function, ReflectHas, PromisePrototypeThen, @@ -78,9 +74,6 @@ function getBufferSourceByteLength(source) { } return ArrayBufferPrototypeGetByteLength(source); } -const U32_BUFFER = new Uint32Array(2); -const U64_BUFFER = new BigUint64Array(TypedArrayPrototypeGetBuffer(U32_BUFFER)); -const I64_BUFFER = new BigInt64Array(TypedArrayPrototypeGetBuffer(U32_BUFFER)); class UnsafePointerView { pointer; @@ -138,21 +131,21 @@ class UnsafePointerView { } getBigUint64(offset = 0) { - op_ffi_read_u64( + return op_ffi_read_u64( this.pointer, - offset, - U32_BUFFER, + // We return a BigInt, so the turbocall + // is forced to use BigInts everywhere. + BigInt(offset), ); - return U64_BUFFER[0]; } getBigInt64(offset = 0) { - op_ffi_read_i64( + return op_ffi_read_i64( this.pointer, - offset, - U32_BUFFER, + // We return a BigInt, so the turbocall + // is forced to use BigInts everywhere. + BigInt(offset), ); - return I64_BUFFER[0]; } getFloat32(offset = 0) { @@ -225,10 +218,6 @@ class UnsafePointerView { } } -const OUT_BUFFER = new Uint32Array(2); -const OUT_BUFFER_64 = new BigInt64Array( - TypedArrayPrototypeGetBuffer(OUT_BUFFER), -); const POINTER_TO_BUFFER_WEAK_MAP = new SafeWeakMap(); class UnsafePointer { static create(value) { @@ -278,12 +267,7 @@ class UnsafePointer { if (ObjectPrototypeIsPrototypeOf(UnsafeCallbackPrototype, value)) { value = value.pointer; } - op_ffi_ptr_value(value, OUT_BUFFER); - const result = OUT_BUFFER[0] + 2 ** 32 * OUT_BUFFER[1]; - if (NumberIsSafeInteger(result)) { - return result; - } - return OUT_BUFFER_64[0]; + return op_ffi_ptr_value(value); } } diff --git a/ext/ffi/repr.rs b/ext/ffi/repr.rs index d6acc8ad21934f..6a5bc067f5e42c 100644 --- a/ext/ffi/repr.rs +++ b/ext/ffi/repr.rs @@ -112,11 +112,11 @@ unsafe extern "C" fn noop_deleter_callback( } #[op2(fast)] +#[bigint] pub fn op_ffi_ptr_value( state: &mut OpState, ptr: *mut c_void, - #[buffer] out: &mut [u32], -) -> Result<(), AnyError> +) -> Result where FP: FfiPermissions + 'static, { @@ -124,18 +124,7 @@ where let permissions = state.borrow_mut::(); permissions.check_partial(None)?; - let outptr = out.as_ptr() as *mut usize; - let length = out.len(); - assert!( - length >= (std::mem::size_of::() / std::mem::size_of::()) - ); - assert_eq!(outptr as usize % std::mem::size_of::(), 0); - - // SAFETY: Out buffer was asserted to be at least large enough to hold a usize, and properly aligned. - let out = unsafe { &mut *outptr }; - *out = ptr as usize; - - Ok(()) + Ok(ptr as usize) } #[op2] @@ -398,12 +387,12 @@ where } #[op2(fast)] +#[bigint] pub fn op_ffi_read_u64( state: &mut OpState, ptr: *mut c_void, - #[number] offset: isize, - #[buffer] out: &mut [u32], -) -> Result<(), AnyError> + #[bigint] offset: isize, +) -> Result where FP: FfiPermissions + 'static, { @@ -412,13 +401,6 @@ where let permissions = state.borrow_mut::(); permissions.check_partial(None)?; - let outptr = out.as_mut_ptr() as *mut u64; - - assert!( - out.len() >= (std::mem::size_of::() / std::mem::size_of::()) - ); - assert_eq!((outptr as usize % std::mem::size_of::()), 0); - if ptr.is_null() { return Err(type_error("Invalid u64 pointer, pointer is null")); } @@ -427,33 +409,24 @@ where // SAFETY: ptr and offset are user provided. unsafe { ptr::read_unaligned::(ptr.offset(offset) as *const u64) }; - // SAFETY: Length and alignment of out slice were asserted to be correct. - unsafe { *outptr = value }; - Ok(()) + Ok(value) } #[op2(fast)] +#[bigint] pub fn op_ffi_read_i64( state: &mut OpState, ptr: *mut c_void, - #[number] offset: isize, - #[buffer] out: &mut [u32], -) -> Result<(), AnyError> + #[bigint] offset: isize, +) -> Result where FP: FfiPermissions + 'static, { - check_unstable(state, "Deno.UnsafePointerView#getBigUint64"); + check_unstable(state, "Deno.UnsafePointerView#getBigInt64"); let permissions = state.borrow_mut::(); permissions.check_partial(None)?; - let outptr = out.as_mut_ptr() as *mut i64; - - assert!( - out.len() >= (std::mem::size_of::() / std::mem::size_of::()) - ); - assert_eq!((outptr as usize % std::mem::size_of::()), 0); - if ptr.is_null() { return Err(type_error("Invalid i64 pointer, pointer is null")); } @@ -462,8 +435,7 @@ where // SAFETY: ptr and offset are user provided. unsafe { ptr::read_unaligned::(ptr.offset(offset) as *const i64) }; // SAFETY: Length and alignment of out slice were asserted to be correct. - unsafe { *outptr = value }; - Ok(()) + Ok(value) } #[op2(fast)] From e5908de133c4b6b1cadde995629061519eb4da0a Mon Sep 17 00:00:00 2001 From: Aapo Alasuutari Date: Sat, 25 May 2024 14:14:47 +0300 Subject: [PATCH 03/10] Fix and improve FFI bench file --- tests/ffi/tests/bench.js | 110 ++++++++++++++++++++++++++++++++++----- 1 file changed, 96 insertions(+), 14 deletions(-) diff --git a/tests/ffi/tests/bench.js b/tests/ffi/tests/bench.js index 49884d32edb357..2b4fbd55ba8384 100644 --- a/tests/ffi/tests/bench.js +++ b/tests/ffi/tests/bench.js @@ -44,130 +44,179 @@ const dylib = Deno.dlopen(libPath, { "return_f64": { parameters: [], result: "f64" }, "return_buffer": { parameters: [], result: "buffer" }, // Nonblocking calls - "nop_nonblocking": { name: "nop", parameters: [], result: "void" }, + "nop_nonblocking": { + name: "nop", + parameters: [], + result: "void", + nonblocking: true, + }, "nop_bool_nonblocking": { name: "nop_bool", parameters: ["bool"], result: "void", + nonblocking: true, + }, + "nop_u8_nonblocking": { + name: "nop_u8", + parameters: ["u8"], + result: "void", + nonblocking: true, + }, + "nop_i8_nonblocking": { + name: "nop_i8", + parameters: ["i8"], + result: "void", + nonblocking: true, }, - "nop_u8_nonblocking": { name: "nop_u8", parameters: ["u8"], result: "void" }, - "nop_i8_nonblocking": { name: "nop_i8", parameters: ["i8"], result: "void" }, "nop_u16_nonblocking": { name: "nop_u16", parameters: ["u16"], result: "void", + nonblocking: true, }, "nop_i16_nonblocking": { name: "nop_i16", parameters: ["i16"], result: "void", + nonblocking: true, }, "nop_u32_nonblocking": { name: "nop_u32", parameters: ["u32"], result: "void", + nonblocking: true, }, "nop_i32_nonblocking": { name: "nop_i32", parameters: ["i32"], result: "void", + nonblocking: true, }, "nop_u64_nonblocking": { name: "nop_u64", parameters: ["u64"], result: "void", + nonblocking: true, }, "nop_i64_nonblocking": { name: "nop_i64", parameters: ["i64"], result: "void", + nonblocking: true, }, "nop_usize_nonblocking": { name: "nop_usize", parameters: ["usize"], result: "void", + nonblocking: true, }, "nop_isize_nonblocking": { name: "nop_isize", parameters: ["isize"], result: "void", + nonblocking: true, }, "nop_f32_nonblocking": { name: "nop_f32", parameters: ["f32"], result: "void", + nonblocking: true, }, "nop_f64_nonblocking": { name: "nop_f64", parameters: ["f64"], result: "void", + nonblocking: true, }, "nop_buffer_nonblocking": { name: "nop_buffer", parameters: ["buffer"], result: "void", + nonblocking: true, }, "return_bool_nonblocking": { name: "return_bool", parameters: [], result: "bool", + nonblocking: true, + }, + "return_u8_nonblocking": { + name: "return_u8", + parameters: [], + result: "u8", + nonblocking: true, + }, + "return_i8_nonblocking": { + name: "return_i8", + parameters: [], + result: "i8", + nonblocking: true, }, - "return_u8_nonblocking": { name: "return_u8", parameters: [], result: "u8" }, - "return_i8_nonblocking": { name: "return_i8", parameters: [], result: "i8" }, "return_u16_nonblocking": { name: "return_u16", parameters: [], result: "u16", + nonblocking: true, }, "return_i16_nonblocking": { name: "return_i16", parameters: [], result: "i16", + nonblocking: true, }, "return_u32_nonblocking": { name: "return_u32", parameters: [], result: "u32", + nonblocking: true, }, "return_i32_nonblocking": { name: "return_i32", parameters: [], result: "i32", + nonblocking: true, }, "return_u64_nonblocking": { name: "return_u64", parameters: [], result: "u64", + nonblocking: true, }, "return_i64_nonblocking": { name: "return_i64", parameters: [], result: "i64", + nonblocking: true, }, "return_usize_nonblocking": { name: "return_usize", parameters: [], result: "usize", + nonblocking: true, }, "return_isize_nonblocking": { name: "return_isize", parameters: [], result: "isize", + nonblocking: true, }, "return_f32_nonblocking": { name: "return_f32", parameters: [], result: "f32", + nonblocking: true, }, "return_f64_nonblocking": { name: "return_f64", parameters: [], result: "f64", + nonblocking: true, }, "return_buffer_nonblocking": { name: "return_buffer", parameters: [], result: "buffer", + nonblocking: true, }, // Parameter checking "nop_many_parameters": { @@ -216,7 +265,7 @@ const dylib = Deno.dlopen(libPath, { "isize", "f32", "f64", - "pointer", + "buffer", "u8", "i8", "u16", @@ -229,7 +278,7 @@ const dylib = Deno.dlopen(libPath, { "isize", "f32", "f64", - "pointer", + "buffer", ], result: "void", nonblocking: true, @@ -265,10 +314,14 @@ Deno.bench("return_buffer()", () => { }); const { add_u64 } = dylib.symbols; -Deno.bench("add_u64()", () => { +Deno.bench("add_u64() number", () => { add_u64(1, 2); }); +Deno.bench("add_u64() bigint", () => { + add_u64(1n, 2n); +}); + const { return_u64 } = dylib.symbols; Deno.bench("return_u64()", () => { return_u64(); @@ -315,15 +368,23 @@ Deno.bench("nop_i32()", () => { }); const { nop_u64 } = dylib.symbols; -Deno.bench("nop_u64()", () => { +Deno.bench("nop_u64() number", () => { nop_u64(100); }); +Deno.bench("nop_u64() bigint", () => { + nop_u64(100n); +}); + const { nop_i64 } = dylib.symbols; -Deno.bench("nop_i64()", () => { +Deno.bench("nop_i64() number", () => { nop_i64(100); }); +Deno.bench("nop_i64() bigint", () => { + nop_i64(100n); +}); + const { nop_usize } = dylib.symbols; Deno.bench("nop_usize() number", () => { nop_usize(100); @@ -457,22 +518,38 @@ Deno.bench("nop_i32_nonblocking()", async () => { }); const { nop_u64_nonblocking } = dylib.symbols; -Deno.bench("nop_u64_nonblocking()", async () => { +Deno.bench("nop_u64_nonblocking() number", async () => { + await nop_u64_nonblocking(100); +}); + +Deno.bench("nop_u64_nonblocking() bigint", async () => { await nop_u64_nonblocking(100); }); const { nop_i64_nonblocking } = dylib.symbols; -Deno.bench("nop_i64_nonblocking()", async () => { +Deno.bench("nop_i64_nonblocking() number", async () => { + await nop_i64_nonblocking(100); +}); + +Deno.bench("nop_i64_nonblocking() bigint", async () => { await nop_i64_nonblocking(100); }); const { nop_usize_nonblocking } = dylib.symbols; -Deno.bench("nop_usize_nonblocking()", async () => { +Deno.bench("nop_usize_nonblocking() number", async () => { + await nop_usize_nonblocking(100); +}); + +Deno.bench("nop_usize_nonblocking() bigint", async () => { await nop_usize_nonblocking(100); }); const { nop_isize_nonblocking } = dylib.symbols; -Deno.bench("nop_isize_nonblocking()", async () => { +Deno.bench("nop_isize_nonblocking() number", async () => { + await nop_isize_nonblocking(100); +}); + +Deno.bench("nop_isize_nonblocking() bigint", async () => { await nop_isize_nonblocking(100); }); @@ -630,6 +707,11 @@ Deno.bench("Deno.UnsafePointer.of", () => { Deno.UnsafePointer.of(buffer); }); +const bufferPointer = Deno.UnsafePointer.of(buffer); +Deno.bench("Deno.UnsafePointer.value", () => { + Deno.UnsafePointer.value(bufferPointer); +}); + const cstringBuffer = new TextEncoder().encode("Best believe it!\0"); const cstringPointerView = new Deno.UnsafePointerView( Deno.UnsafePointer.of(cstringBuffer), From 79b14df42478f1a818f5d4afea738d4ffcfd1170 Mon Sep 17 00:00:00 2001 From: Aapo Alasuutari Date: Sat, 25 May 2024 14:16:02 +0300 Subject: [PATCH 04/10] Remove now-unnecessary typed-array-return-value tests --- ext/ffi/turbocall.rs | 175 ------------------------------------------- 1 file changed, 175 deletions(-) diff --git a/ext/ffi/turbocall.rs b/ext/ffi/turbocall.rs index e0cf4e01763d5b..2043889462f3df 100644 --- a/ext/ffi/turbocall.rs +++ b/ext/ffi/turbocall.rs @@ -1567,61 +1567,6 @@ mod tests { let expected = assembler.finalize().unwrap(); assert_eq!(trampoline.0.deref(), expected.deref()); } - - #[test] - fn return_u64_in_register_typed_array() { - let trampoline = SysVAmd64::compile(&symbol(vec![], U64)); - - let mut assembler = dynasmrt::x64::Assembler::new().unwrap(); - // See https://godbolt.org/z/8G7a488o7 - dynasm!(assembler - ; .arch x64 - ; push rbx - ; xor edi, edi // recv - ; mov rbx, [rsi + 8] // save data array pointer to non-volatile register - ; mov rax, QWORD 0 - ; call rax - ; mov [rbx], rax // copy return value to data pointer address - ; pop rbx - ; ret - ); - let expected = assembler.finalize().unwrap(); - assert_eq!(trampoline.0.deref(), expected.deref()); - } - - #[test] - fn return_u64_in_stack_typed_array() { - let trampoline = SysVAmd64::compile(&symbol( - vec![U64, U64, U64, U64, U64, U64, U64], - U64, - )); - - let mut assembler = dynasmrt::x64::Assembler::new().unwrap(); - // See https://godbolt.org/z/cPnPYWdWq - dynasm!(assembler - ; .arch x64 - ; push rbx - ; sub rsp, DWORD 16 - ; mov rdi, rsi // u64 - ; mov rsi, rdx // u64 - ; mov rdx, rcx // u64 - ; mov rcx, r8 // u64 - ; mov r8, r9 // u64 - ; mov r9, [DWORD rsp + 32] // u64 - ; mov rax, [DWORD rsp + 40] // u64 - ; mov [DWORD rsp + 0], rax // .. - ; mov rax, [DWORD rsp + 48] // save data array pointer to non-volatile register - ; mov rbx, [rax + 8] // .. - ; mov rax, QWORD 0 - ; call rax - ; mov [rbx], rax // copy return value to data pointer address - ; add rsp, DWORD 16 - ; pop rbx - ; ret - ); - let expected = assembler.finalize().unwrap(); - assert_eq!(trampoline.0.deref(), expected.deref()); - } } mod aarch64_apple { @@ -1743,73 +1688,6 @@ mod tests { let expected = assembler.finalize().unwrap(); assert_eq!(trampoline.0.deref(), expected.deref()); } - - #[test] - fn return_u64_in_register_typed_array() { - let trampoline = Aarch64Apple::compile(&symbol(vec![], U64)); - - let mut assembler = dynasmrt::aarch64::Assembler::new().unwrap(); - // See https://godbolt.org/z/47EvvYb83 - dynasm!(assembler - ; .arch aarch64 - ; sub sp, sp, 32 - ; stp x29, x30, [sp, 16] - ; add x29, sp, 16 - ; str x19, [sp, 8] - ; mov x0, xzr // recv - ; ldr x19, [x1, 8] // save data array pointer to non-volatile register - ; movz x8, 0 - ; blr x8 - ; str x0, [x19] // copy return value to data pointer address - ; ldr x19, [sp, 8] - ; ldp x29, x30, [sp, 16] - ; add sp, sp, 32 - ; ret - ); - let expected = assembler.finalize().unwrap(); - assert_eq!(trampoline.0.deref(), expected.deref()); - } - - #[test] - fn return_u64_in_stack_typed_array() { - let trampoline = Aarch64Apple::compile(&symbol( - vec![U64, U64, U64, U64, U64, U64, U64, U64, U8, U8], - U64, - )); - - let mut assembler = dynasmrt::aarch64::Assembler::new().unwrap(); - // See https://godbolt.org/z/PvYPbsE1b - dynasm!(assembler - ; .arch aarch64 - ; sub sp, sp, 32 - ; stp x29, x30, [sp, 16] - ; add x29, sp, 16 - ; str x19, [sp, 8] - ; mov x0, x1 // u64 - ; mov x1, x2 // u64 - ; mov x2, x3 // u64 - ; mov x3, x4 // u64 - ; mov x4, x5 // u64 - ; mov x5, x6 // u64 - ; mov x6, x7 // u64 - ; ldr x7, [sp, 32] // u64 - ; ldr w8, [sp, 40] // u8 - ; strb w8, [sp] // .. - ; ldr w8, [sp, 48] // u8 - ; strb w8, [sp, 1] // .. - ; ldr x19, [sp, 56] // save data array pointer to non-volatile register - ; ldr x19, [x19, 8] // .. - ; movz x8, 0 - ; blr x8 - ; str x0, [x19] // copy return value to data pointer address - ; ldr x19, [sp, 8] - ; ldp x29, x30, [sp, 16] - ; add sp, sp, 32 - ; ret - ); - let expected = assembler.finalize().unwrap(); - assert_eq!(trampoline.0.deref(), expected.deref()); - } } mod x64_windows { @@ -1919,58 +1797,5 @@ mod tests { let expected = assembler.finalize().unwrap(); assert_eq!(trampoline.0.deref(), expected.deref()); } - - #[test] - fn return_u64_in_register_typed_array() { - let trampoline = Win64::compile(&symbol(vec![], U64)); - - let mut assembler = dynasmrt::x64::Assembler::new().unwrap(); - // See https://godbolt.org/z/7EnPE7o3T - dynasm!(assembler - ; .arch x64 - ; push rbx - ; sub rsp, DWORD 32 - ; xor ecx, ecx // recv - ; mov rbx, [rdx + 8] // save data array pointer to non-volatile register - ; mov rax, QWORD 0 - ; call rax - ; mov [rbx], rax // copy return value to data pointer address - ; add rsp, DWORD 32 - ; pop rbx - ; ret - ); - let expected = assembler.finalize().unwrap(); - assert_eq!(trampoline.0.deref(), expected.deref()); - } - - #[test] - fn return_u64_in_stack_typed_array() { - let trampoline = - Win64::compile(&symbol(vec![U64, U64, U64, U64, U64], U64)); - - let mut assembler = dynasmrt::x64::Assembler::new().unwrap(); - // See https://godbolt.org/z/3966sfEex - dynasm!(assembler - ; .arch x64 - ; push rbx - ; sub rsp, DWORD 48 - ; mov rcx, rdx // u64 - ; mov rdx, r8 // u64 - ; mov r8, r9 // u64 - ; mov r9, [DWORD rsp + 96] // u64 - ; mov rax, [DWORD rsp + 104] // u64 - ; mov [DWORD rsp + 32], rax // .. - ; mov rax, [DWORD rsp + 112] // save data array pointer to non-volatile register - ; mov rbx, [rax + 8] // .. - ; mov rax, QWORD 0 - ; call rax - ; mov [rbx], rax // copy return value to data pointer address - ; add rsp, DWORD 48 - ; pop rbx - ; ret - ); - let expected = assembler.finalize().unwrap(); - assert_eq!(trampoline.0.deref(), expected.deref()); - } } } From 6ffac158b4f496622d65b788cc7ac70094c3ee79 Mon Sep 17 00:00:00 2001 From: Aapo Alasuutari Date: Sat, 25 May 2024 14:20:08 +0300 Subject: [PATCH 05/10] Add comment about offset representation --- ext/ffi/repr.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ext/ffi/repr.rs b/ext/ffi/repr.rs index 6a5bc067f5e42c..c3656f0fec9386 100644 --- a/ext/ffi/repr.rs +++ b/ext/ffi/repr.rs @@ -391,6 +391,8 @@ where pub fn op_ffi_read_u64( state: &mut OpState, ptr: *mut c_void, + // Note: The representation of 64-bit integers is function-wide. We cannot + // choose to take this parameter as a number while returning a bigint. #[bigint] offset: isize, ) -> Result where @@ -417,6 +419,8 @@ where pub fn op_ffi_read_i64( state: &mut OpState, ptr: *mut c_void, + // Note: The representation of 64-bit integers is function-wide. We cannot + // choose to take this parameter as a number while returning a bigint. #[bigint] offset: isize, ) -> Result where From 5f6f9a1cd973c55000a31ed798d8afdffaeb69d7 Mon Sep 17 00:00:00 2001 From: Aapo Alasuutari Date: Sat, 25 May 2024 14:54:59 +0300 Subject: [PATCH 06/10] Change types to bigint only --- cli/tsc/dts/lib.deno.unstable.d.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/cli/tsc/dts/lib.deno.unstable.d.ts b/cli/tsc/dts/lib.deno.unstable.d.ts index a3e0d2eb06b7d6..4595f607eea642 100644 --- a/cli/tsc/dts/lib.deno.unstable.d.ts +++ b/cli/tsc/dts/lib.deno.unstable.d.ts @@ -220,7 +220,7 @@ declare namespace Deno { : T extends NativeU32Enum ? U : T extends NativeI32Enum ? U : number - : T extends NativeBigIntType ? number | bigint + : T extends NativeBigIntType ? bigint : T extends NativeBooleanType ? boolean : T extends NativePointerType ? T extends NativeTypedPointer ? U | null : PointerValue @@ -501,7 +501,7 @@ declare namespace Deno { */ export class UnsafePointer { /** Create a pointer from a numeric value. This one is really dangerous! */ - static create(value: number | bigint): PointerValue; + static create(value: bigint): PointerValue; /** Returns `true` if the two pointers point to the same address. */ static equals(a: PointerValue, b: PointerValue): boolean; /** Return the direct memory pointer to the typed array in memory. */ @@ -514,7 +514,7 @@ declare namespace Deno { offset: number, ): PointerValue; /** Get the numeric value of a pointer */ - static value(value: PointerValue): number | bigint; + static value(value: PointerValue): bigint; } /** **UNSTABLE**: New API, yet to be vetted. @@ -554,10 +554,10 @@ declare namespace Deno { getInt32(offset?: number): number; /** Gets an unsigned 64-bit integer at the specified byte offset from the * pointer. */ - getBigUint64(offset?: number): number | bigint; + getBigUint64(offset?: number): bigint; /** Gets a signed 64-bit integer at the specified byte offset from the * pointer. */ - getBigInt64(offset?: number): number | bigint; + getBigInt64(offset?: number): bigint; /** Gets a signed 32-bit float at the specified byte offset from the * pointer. */ getFloat32(offset?: number): number; From aa205004971ac685fe17351a12067d344344f2de Mon Sep 17 00:00:00 2001 From: Aapo Alasuutari Date: Sat, 25 May 2024 14:56:53 +0300 Subject: [PATCH 07/10] Fix tests --- tests/ffi/tests/ffi_types.ts | 24 ++++++++++++------------ tests/unit/ffi_test.ts | 4 ++-- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/tests/ffi/tests/ffi_types.ts b/tests/ffi/tests/ffi_types.ts index 93ac98d926d5c4..f093e33eb104a6 100644 --- a/tests/ffi/tests/ffi_types.ts +++ b/tests/ffi/tests/ffi_types.ts @@ -154,8 +154,8 @@ remote.symbols.method15({} as Deno.PointerValue); const result = remote.symbols.method16(); // @ts-expect-error: Invalid argument -let r_0: string = result; -let r_1: number | bigint = result; +let r_0: number = result; +let r_1: bigint = result; const result2 = remote.symbols.method17(); // @ts-expect-error: Invalid argument @@ -290,17 +290,17 @@ let r42_1: number = remote.symbols.method24(true); remote.symbols.method25(); // @ts-expect-error: Invalid member type -const static1_wrong: null = remote.symbols.static1; -const static1_right: number | bigint = remote.symbols.static1; +const static1_wrong: number = remote.symbols.static1; +const static1_right: bigint = remote.symbols.static1; // @ts-expect-error: Invalid member type const static2_wrong: null = remote.symbols.static2; const static2_right: null | Deno.UnsafePointer = remote.symbols.static2; // @ts-expect-error: Invalid member type -const static3_wrong: null = remote.symbols.static3; -const static3_right: number | bigint = remote.symbols.static3; +const static3_wrong: number = remote.symbols.static3; +const static3_right: bigint = remote.symbols.static3; // @ts-expect-error: Invalid member type -const static4_wrong: null = remote.symbols.static4; -const static4_right: number | bigint = remote.symbols.static4; +const static4_wrong: number = remote.symbols.static4; +const static4_right: bigint = remote.symbols.static4; // @ts-expect-error: Invalid member type const static5_wrong: null = remote.symbols.static5; const static5_right: number = remote.symbols.static5; @@ -311,8 +311,8 @@ const static6_right: number = remote.symbols.static6; const static7_wrong: null = remote.symbols.static7; const static7_right: number = remote.symbols.static7; // @ts-expect-error: Invalid member type -const static8_wrong: null = remote.symbols.static8; -const static8_right: number | bigint = remote.symbols.static8; +const static8_wrong: number = remote.symbols.static8; +const static8_right: bigint = remote.symbols.static8; // @ts-expect-error: Invalid member type const static9_wrong: null = remote.symbols.static9; const static9_right: number = remote.symbols.static9; @@ -323,8 +323,8 @@ const static10_right: number = remote.symbols.static10; const static11_wrong: null = remote.symbols.static11; const static11_right: number = remote.symbols.static11; // @ts-expect-error: Invalid member type -const static12_wrong: null = remote.symbols.static12; -const static12_right: number | bigint = remote.symbols.static12; +const static12_wrong: number = remote.symbols.static12; +const static12_right: bigint = remote.symbols.static12; // @ts-expect-error: Invalid member type const static13_wrong: null = remote.symbols.static13; const static13_right: number = remote.symbols.static13; diff --git a/tests/unit/ffi_test.ts b/tests/unit/ffi_test.ts index 2b56a8db1404a6..70a914c0af4a18 100644 --- a/tests/unit/ffi_test.ts +++ b/tests/unit/ffi_test.ts @@ -92,11 +92,11 @@ Deno.test({ permissions: { ffi: true } }, function pointerOf() { const uint8AddressOffset = Deno.UnsafePointer.value( Deno.UnsafePointer.of(new Uint8Array(buffer, 100)), ); - assertEquals(Number(baseAddress) + 100, uint8AddressOffset); + assertEquals(baseAddress + 100n, uint8AddressOffset); const float64AddressOffset = Deno.UnsafePointer.value( Deno.UnsafePointer.of(new Float64Array(buffer, 80)), ); - assertEquals(Number(baseAddress) + 80, float64AddressOffset); + assertEquals(baseAddress + 80n, float64AddressOffset); }); Deno.test({ permissions: { ffi: true } }, function callWithError() { From 70c65078bb2d797a82ee8fcdbfccbc3275e3fb60 Mon Sep 17 00:00:00 2001 From: Aapo Alasuutari Date: Sat, 25 May 2024 15:33:51 +0300 Subject: [PATCH 08/10] Fix doc comment test --- cli/tsc/dts/lib.deno.unstable.d.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/tsc/dts/lib.deno.unstable.d.ts b/cli/tsc/dts/lib.deno.unstable.d.ts index 4595f607eea642..4a92f6872ff09f 100644 --- a/cli/tsc/dts/lib.deno.unstable.d.ts +++ b/cli/tsc/dts/lib.deno.unstable.d.ts @@ -816,7 +816,7 @@ declare namespace Deno { * ); * * // Call the symbol `add` - * const result = dylib.symbols.add(35, 34); // 69 + * const result = dylib.symbols.add(35n, 34n); // 69n * * console.log(`Result from external addition of 35 and 34: ${result}`); * ``` From 4bb0240d62ae2fd2ad277d168b386fb101fdee6c Mon Sep 17 00:00:00 2001 From: Aapo Alasuutari Date: Sat, 25 May 2024 16:13:18 +0300 Subject: [PATCH 09/10] Fix tests again --- tests/ffi/tests/test.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/ffi/tests/test.js b/tests/ffi/tests/test.js index 3a6af270d89b2c..4b02b2961d1977 100644 --- a/tests/ffi/tests/test.js +++ b/tests/ffi/tests/test.js @@ -703,17 +703,17 @@ assertEquals(view.getUint32(), 55); const createdPointer = Deno.UnsafePointer.create(1); assertNotEquals(createdPointer, null); assertEquals(typeof createdPointer, "object"); - assertEquals(Deno.UnsafePointer.value(null), 0); - assertEquals(Deno.UnsafePointer.value(createdPointer), 1); + assertEquals(Deno.UnsafePointer.value(null), 0n); + assertEquals(Deno.UnsafePointer.value(createdPointer), 1n); assert(Deno.UnsafePointer.equals(null, null)); assertFalse(Deno.UnsafePointer.equals(null, createdPointer)); assertFalse(Deno.UnsafePointer.equals(Deno.UnsafePointer.create(2), createdPointer)); // Do not allow offsetting from null, `create` function should be used instead. assertThrows(() => Deno.UnsafePointer.offset(null, 5)); const offsetPointer = Deno.UnsafePointer.offset(createdPointer, 5); - assertEquals(Deno.UnsafePointer.value(offsetPointer), 6); + assertEquals(Deno.UnsafePointer.value(offsetPointer), 6n); const zeroPointer = Deno.UnsafePointer.offset(offsetPointer, -6); - assertEquals(Deno.UnsafePointer.value(zeroPointer), 0); + assertEquals(Deno.UnsafePointer.value(zeroPointer), 0n); assertEquals(zeroPointer, null); } From 2bf0fbb6329be8e45f34c28f22cb9ef4c3447f2d Mon Sep 17 00:00:00 2001 From: Aapo Alasuutari Date: Sat, 25 May 2024 17:06:19 +0300 Subject: [PATCH 10/10] ci