Skip to content

Commit

Permalink
Move closure shims into the descriptor
Browse files Browse the repository at this point in the history
Currently closure shims are communicated to JS at runtime, although at
runtime the same constant value is always passed to JS! More pressing,
however, work in rustwasm#1002 requires knowledge of closure descriptor indices
at `wasm-bindgen` time which is not currently known.

Since the closure descriptor shims and such are already constant values,
this commit moves the descriptor function indices into the *descriptor*
for a closure/function pointer. This way we can learn about these values
at `wasm-bindgen` time instead of only knowing them at runtime.

This should have no semantic change on users of `wasm-bindgen`, although
some closure invocations may be slightly speedier because there's less
arguments being transferred over the boundary. Overall though this will
help rustwasm#1002 as the closure shims that the Rust compiler generates may not
be the exact ones we hand out to JS, but rather wrappers around them
which do `anyref` business things.
  • Loading branch information
alexcrichton committed Nov 29, 2018
1 parent bbde39f commit 6acecd1
Show file tree
Hide file tree
Showing 10 changed files with 128 additions and 190 deletions.
2 changes: 2 additions & 0 deletions crates/backend/src/codegen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -501,6 +501,7 @@ impl TryToTokens for ast::Export {
&export,
quote! {
inform(FUNCTION);
inform(0);
inform(#nargs);
#(<#argtys as WasmDescribe>::describe();)*
#describe_ret
Expand Down Expand Up @@ -999,6 +1000,7 @@ impl<'a> ToTokens for DescribeImport<'a> {
&f.shim,
quote! {
inform(FUNCTION);
inform(0);
inform(#nargs);
#(<#argtys as WasmDescribe>::describe();)*
#inform_ret
Expand Down
9 changes: 9 additions & 0 deletions crates/cli-support/src/descriptor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,11 +70,14 @@ pub enum Descriptor {
#[derive(Debug)]
pub struct Function {
pub arguments: Vec<Descriptor>,
pub shim_idx: u32,
pub ret: Descriptor,
}

#[derive(Debug)]
pub struct Closure {
pub shim_idx: u32,
pub dtor_idx: u32,
pub function: Function,
pub mutable: bool,
}
Expand Down Expand Up @@ -293,9 +296,13 @@ fn get(a: &mut &[u32]) -> u32 {

impl Closure {
fn decode(data: &mut &[u32]) -> Closure {
let shim_idx = get(data);
let dtor_idx = get(data);
let mutable = get(data) == REFMUT;
assert_eq!(get(data), FUNCTION);
Closure {
shim_idx,
dtor_idx,
mutable,
function: Function::decode(data),
}
Expand All @@ -304,11 +311,13 @@ impl Closure {

impl Function {
fn decode(data: &mut &[u32]) -> Function {
let shim_idx = get(data);
let arguments = (0..get(data))
.map(|_| Descriptor::_decode(data))
.collect::<Vec<_>>();
Function {
arguments,
shim_idx,
ret: Descriptor::_decode(data),
}
}
Expand Down
17 changes: 10 additions & 7 deletions crates/cli-support/src/js/closures.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,12 @@ pub fn rewrite(input: &mut Context) -> Result<(), Error> {
// If this was an imported function we didn't reorder those, so nothing
// to do.
if idx < old_num_imports {
return idx
idx
} else {
// ... otherwise we're injecting a number of new imports, so offset
// everything.
idx + info.code_idx_to_descriptor.len() as u32
}
// ... otherwise we're injecting a number of new imports, so offset
// everything.
idx + info.code_idx_to_descriptor.len() as u32
}).remap_module(input.module);

info.delete_function_table_entries(input);
Expand Down Expand Up @@ -252,16 +253,18 @@ impl ClosureDescriptors {
input.expose_add_heap_object();
input.function_table_needed = true;
let body = format!(
"function(a, b, fi, di, _ignored) {{
const f = wasm.__wbg_function_table.get(fi);
const d = wasm.__wbg_function_table.get(di);
"function(a, b, _ignored) {{
const f = wasm.__wbg_function_table.get({});
const d = wasm.__wbg_function_table.get({});
const cb = {};
cb.a = a;
cb.cnt = 1;
let real = cb.bind(cb);
real.original = cb;
return addHeapObject(real);
}}",
closure.shim_idx,
closure.dtor_idx,
js,
);
input.export(&import_name, &body, None);
Expand Down
17 changes: 0 additions & 17 deletions crates/cli-support/src/js/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1682,23 +1682,6 @@ impl<'a> Context<'a> {
}
}

fn expose_get_global_argument(&mut self) -> Result<(), Error> {
if !self.exposed_globals.insert("get_global_argument") {
return Ok(());
}
self.expose_uint32_memory();
self.expose_global_argument_ptr()?;
self.global(
"
function getGlobalArgument(arg) {
const idx = globalArgumentPtr() / 4 + arg;
return getUint32Memory()[idx];
}
",
);
Ok(())
}

fn expose_global_argument_ptr(&mut self) -> Result<(), Error> {
if !self.exposed_globals.insert("global_argument_ptr") {
return Ok(());
Expand Down
12 changes: 6 additions & 6 deletions crates/cli-support/src/js/rust2js.rs
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,7 @@ impl<'a, 'b> Rust2Js<'a, 'b> {
}

if let Some((f, mutable)) = arg.stack_closure() {
let arg2 = self.shim_argument();
let (js, _ts, _js_doc) = {
let mut builder = Js2Rust::new("", self.cx);
if mutable {
Expand All @@ -268,20 +269,19 @@ impl<'a, 'b> Rust2Js<'a, 'b> {
.process(f)?
.finish("function", "this.f")
};
self.cx.expose_get_global_argument()?;
self.cx.function_table_needed = true;
let next_global = self.global_idx();
self.global_idx();
self.prelude(&format!(
"\
let cb{0} = {js};\n\
cb{0}.f = wasm.__wbg_function_table.get({0});\n\
cb{0}.a = getGlobalArgument({next_global});\n\
cb{0}.b = getGlobalArgument({next_global} + 1);\n\
cb{0}.f = wasm.__wbg_function_table.get({idx});\n\
cb{0}.a = {0};\n\
cb{0}.b = {1};\n\
",
abi,
arg2,
js = js,
next_global = next_global
idx = f.shim_idx,
));
self.finally(&format!("cb{0}.a = cb{0}.b = 0;", abi));
self.js_arguments.push(format!("cb{0}.bind(cb{0})", abi));
Expand Down
2 changes: 0 additions & 2 deletions crates/wasm-interpreter/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -353,8 +353,6 @@ impl Interpreter {
self.descriptor_table_idx = Some(self.stack.pop().unwrap() as u32);
self.stack.pop();
self.stack.pop();
self.stack.pop();
self.stack.pop();
self.stack.push(0);
} else {
self.call(*idx, sections);
Expand Down
43 changes: 13 additions & 30 deletions src/closure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,20 +197,16 @@ impl<T> Closure<T>
unsafe fn breaks_if_inlined<T: WasmClosure + ?Sized>(
a: usize,
b: usize,
invoke: u32,
destroy: u32,
) -> u32 {
super::__wbindgen_describe_closure(
a as u32,
b as u32,
invoke,
destroy,
describe::<T> as u32,
)
}

let idx = unsafe {
breaks_if_inlined::<T>(a, b, T::invoke_fn(), T::destroy_fn())
breaks_if_inlined::<T>(a, b)
};

Closure {
Expand Down Expand Up @@ -294,9 +290,6 @@ impl<T> Drop for Closure<T>
#[doc(hidden)]
pub unsafe trait WasmClosure: 'static {
fn describe();

fn invoke_fn() -> u32;
fn destroy_fn() -> u32;
}

// The memory safety here in these implementations below is a bit tricky. We
Expand All @@ -322,11 +315,6 @@ macro_rules! doit {
R: ReturnWasmAbi + 'static,
{
fn describe() {
<&Self>::describe();
}

#[inline]
fn invoke_fn() -> u32 {
#[allow(non_snake_case)]
unsafe extern "C" fn invoke<$($var: FromWasmAbi,)* R: ReturnWasmAbi>(
a: usize,
Expand All @@ -350,12 +338,10 @@ macro_rules! doit {
};
ret.return_abi(&mut GlobalStack::new())
}
invoke::<$($var,)* R> as u32
}

#[inline]
fn destroy_fn() -> u32 {
unsafe extern "C" fn destroy<$($var: FromWasmAbi,)* R: ReturnWasmAbi>(
inform(invoke::<$($var,)* R> as u32);

unsafe extern fn destroy<$($var: FromWasmAbi,)* R: ReturnWasmAbi>(
a: usize,
b: usize,
) {
Expand All @@ -364,7 +350,9 @@ macro_rules! doit {
fields: (a, b)
}.ptr));
}
destroy::<$($var,)* R> as u32
inform(destroy::<$($var,)* R> as u32);

<&Self>::describe();
}
}

Expand All @@ -373,11 +361,6 @@ macro_rules! doit {
R: ReturnWasmAbi + 'static,
{
fn describe() {
<&mut Self>::describe();
}

#[inline]
fn invoke_fn() -> u32 {
#[allow(non_snake_case)]
unsafe extern "C" fn invoke<$($var: FromWasmAbi,)* R: ReturnWasmAbi>(
a: usize,
Expand All @@ -402,12 +385,10 @@ macro_rules! doit {
};
ret.return_abi(&mut GlobalStack::new())
}
invoke::<$($var,)* R> as u32
}

#[inline]
fn destroy_fn() -> u32 {
unsafe extern "C" fn destroy<$($var: FromWasmAbi,)* R: ReturnWasmAbi>(
inform(invoke::<$($var,)* R> as u32);

unsafe extern fn destroy<$($var: FromWasmAbi,)* R: ReturnWasmAbi>(
a: usize,
b: usize,
) {
Expand All @@ -416,7 +397,9 @@ macro_rules! doit {
fields: (a, b)
}.ptr));
}
destroy::<$($var,)* R> as u32
inform(destroy::<$($var,)* R> as u32);

<&mut Self>::describe();
}
}
)*)
Expand Down
Loading

0 comments on commit 6acecd1

Please sign in to comment.