Skip to content

Commit

Permalink
Move native signatures out of Module
Browse files Browse the repository at this point in the history
After compilation there's actually no need to hold onto the native
signature for a wasm function type, so this commit moves out the
`ir::Signature` value from a `Module` into a separate field that's
deallocated when compilation is finished. This simplifies the
`SignatureRegistry` because it only needs to track wasm functino types
and it also means less work is done for `Func::wrap`.
  • Loading branch information
alexcrichton committed Nov 4, 2020
1 parent dd9bfce commit 0e02c62
Show file tree
Hide file tree
Showing 13 changed files with 65 additions and 66 deletions.
14 changes: 10 additions & 4 deletions crates/cranelift/src/func_environ.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use cranelift_codegen::ir::immediates::{Offset32, Uimm64};
use cranelift_codegen::ir::types::*;
use cranelift_codegen::ir::{AbiParam, ArgumentPurpose, Function, InstBuilder, Signature};
use cranelift_codegen::isa::{self, TargetFrontendConfig};
use cranelift_entity::EntityRef;
use cranelift_entity::{EntityRef, PrimaryMap};
use cranelift_frontend::FunctionBuilder;
use cranelift_wasm::{
self, FuncIndex, GlobalIndex, GlobalVariable, MemoryIndex, SignatureIndex, TableIndex,
Expand Down Expand Up @@ -99,6 +99,9 @@ pub struct FuncEnvironment<'module_environment> {
/// The module-level environment which this function-level environment belongs to.
module: &'module_environment Module,

/// The native signatures for each type signature in this module
native_signatures: &'module_environment PrimaryMap<SignatureIndex, ir::Signature>,

/// The Cranelift global holding the vmctx address.
vmctx: Option<ir::GlobalValue>,

Expand All @@ -115,6 +118,7 @@ impl<'module_environment> FuncEnvironment<'module_environment> {
pub fn new(
target_config: TargetFrontendConfig,
module: &'module_environment Module,
native_signatures: &'module_environment PrimaryMap<SignatureIndex, ir::Signature>,
tunables: &'module_environment Tunables,
) -> Self {
let builtin_function_signatures = BuiltinFunctionSignatures::new(
Expand All @@ -129,6 +133,7 @@ impl<'module_environment> FuncEnvironment<'module_environment> {
Self {
target_config,
module,
native_signatures,
vmctx: None,
builtin_function_signatures,
offsets: VMOffsets::new(target_config.pointer_bytes(), module),
Expand Down Expand Up @@ -993,16 +998,17 @@ impl<'module_environment> cranelift_wasm::FuncEnvironment for FuncEnvironment<'m
func: &mut ir::Function,
index: SignatureIndex,
) -> WasmResult<ir::SigRef> {
Ok(func.import_signature(self.module.signatures[index].1.clone()))
Ok(func.import_signature(self.native_signatures[index].clone()))
}

fn make_direct_func(
&mut self,
func: &mut ir::Function,
index: FuncIndex,
) -> WasmResult<ir::FuncRef> {
let sig = self.module.native_func_signature(index);
let signature = func.import_signature(sig.clone());
let sig_index = self.module.functions[index];
let sig = self.native_signatures[sig_index].clone();
let signature = func.import_signature(sig);
let name = get_func_name(index);
Ok(func.import_function(ir::ExtFuncData {
name,
Expand Down
10 changes: 8 additions & 2 deletions crates/cranelift/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -362,12 +362,18 @@ impl Compiler for Cranelift {
let func_index = module.func_index(func_index);
let mut context = Context::new();
context.func.name = get_func_name(func_index);
context.func.signature = module.native_func_signature(func_index).clone();
let sig_index = module.functions[func_index];
context.func.signature = translation.native_signatures[sig_index].clone();
if tunables.debug_info {
context.func.collect_debug_info();
}

let mut func_env = FuncEnvironment::new(isa.frontend_config(), module, tunables);
let mut func_env = FuncEnvironment::new(
isa.frontend_config(),
module,
&translation.native_signatures,
tunables,
);

// We use these as constant offsets below in
// `stack_limit_from_arguments`, so assert their values here. This
Expand Down
11 changes: 2 additions & 9 deletions crates/environ/src/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
use crate::tunables::Tunables;
use crate::WASM_MAX_PAGES;
use cranelift_codegen::ir;
use cranelift_entity::{EntityRef, PrimaryMap};
use cranelift_wasm::{
DataIndex, DefinedFuncIndex, DefinedGlobalIndex, DefinedMemoryIndex, DefinedTableIndex,
Expand Down Expand Up @@ -169,7 +168,7 @@ pub struct Module {
pub func_names: HashMap<FuncIndex, String>,

/// Unprocessed signatures exactly as provided by `declare_signature()`.
pub signatures: PrimaryMap<SignatureIndex, (WasmFuncType, ir::Signature)>,
pub signatures: PrimaryMap<SignatureIndex, WasmFuncType>,

/// Number of imported functions in the module.
pub num_imported_funcs: usize,
Expand Down Expand Up @@ -319,16 +318,10 @@ impl Module {
index.index() < self.num_imported_globals
}

/// Convenience method for looking up the native signature of a compiled
/// Wasm function.
pub fn native_func_signature(&self, func_index: FuncIndex) -> &ir::Signature {
&self.signatures[self.functions[func_index]].1
}

/// Convenience method for looking up the original Wasm signature of a
/// function.
pub fn wasm_func_type(&self, func_index: FuncIndex) -> &WasmFuncType {
&self.signatures[self.functions[func_index]].0
&self.signatures[self.functions[func_index]]
}
}

Expand Down
16 changes: 10 additions & 6 deletions crates/environ/src/module_environ.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ pub struct ModuleTranslation<'data> {
/// Module information.
pub module: Module,

/// Map of native signatures
pub native_signatures: PrimaryMap<SignatureIndex, ir::Signature>,

/// References to the function bodies.
pub function_body_inputs: PrimaryMap<DefinedFuncIndex, FunctionBodyData<'data>>,

Expand Down Expand Up @@ -111,6 +114,7 @@ impl<'data> ModuleEnvironment<'data> {
result: ModuleTranslation {
target_config,
module: Module::new(),
native_signatures: PrimaryMap::new(),
function_body_inputs: PrimaryMap::new(),
data_initializers: Vec::new(),
tunables: tunables.clone(),
Expand Down Expand Up @@ -198,17 +202,17 @@ impl<'data> TargetEnvironment for ModuleEnvironment<'data> {
/// environment-dependent wasm instructions. These functions should not be called by the user.
impl<'data> cranelift_wasm::ModuleEnvironment<'data> for ModuleEnvironment<'data> {
fn reserve_signatures(&mut self, num: u32) -> WasmResult<()> {
self.result
.module
.signatures
.reserve_exact(usize::try_from(num).unwrap());
let num = usize::try_from(num).unwrap();
self.result.module.signatures.reserve_exact(num);
self.result.native_signatures.reserve_exact(num);
Ok(())
}

fn declare_signature(&mut self, wasm: WasmFuncType, sig: ir::Signature) -> WasmResult<()> {
let sig = translate_signature(sig, self.pointer_type());
// TODO: Deduplicate signatures.
self.result.module.signatures.push((wasm, sig));
self.result.module.signatures.push(wasm);
self.result.native_signatures.push(sig);
Ok(())
}

Expand Down Expand Up @@ -461,7 +465,7 @@ impl<'data> cranelift_wasm::ModuleEnvironment<'data> for ModuleEnvironment<'data
}
info.wasm_file.funcs.push(FunctionMetadata {
locals: locals.into_boxed_slice(),
params: sig.0.params.iter().cloned().map(|i| i.into()).collect(),
params: sig.params.iter().cloned().map(|i| i.into()).collect(),
});
}
self.result
Expand Down
3 changes: 1 addition & 2 deletions crates/jit/src/compiler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,8 +158,7 @@ impl Compiler {
vec![]
};

let (obj, unwind_info) =
build_object(&*self.isa, &translation.module, &funcs, dwarf_sections)?;
let (obj, unwind_info) = build_object(&*self.isa, &translation, &funcs, dwarf_sections)?;

Ok(Compilation {
obj,
Expand Down
12 changes: 6 additions & 6 deletions crates/jit/src/object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use wasmtime_debug::DwarfSection;
use wasmtime_environ::entity::PrimaryMap;
use wasmtime_environ::isa::{unwind::UnwindInfo, TargetIsa};
use wasmtime_environ::wasm::{FuncIndex, SignatureIndex};
use wasmtime_environ::{CompiledFunctions, Module};
use wasmtime_environ::{CompiledFunctions, ModuleTranslation};
use wasmtime_obj::{ObjectBuilder, ObjectBuilderTarget};

pub use wasmtime_obj::utils;
Expand All @@ -23,7 +23,7 @@ pub enum ObjectUnwindInfo {
// Builds ELF image from the module `Compilation`.
pub(crate) fn build_object(
isa: &dyn TargetIsa,
module: &Module,
translation: &ModuleTranslation,
funcs: &CompiledFunctions,
dwarf_sections: Vec<DwarfSection>,
) -> Result<(Object, Vec<ObjectUnwindInfo>), anyhow::Error> {
Expand All @@ -39,13 +39,13 @@ pub(crate) fn build_object(
unwind_info.extend(funcs.iter().filter_map(|(index, func)| {
func.unwind_info
.as_ref()
.map(|info| ObjectUnwindInfo::Func(module.func_index(index), info.clone()))
.map(|info| ObjectUnwindInfo::Func(translation.module.func_index(index), info.clone()))
}));

let mut trampolines = PrimaryMap::with_capacity(module.signatures.len());
let mut trampolines = PrimaryMap::with_capacity(translation.module.signatures.len());
let mut cx = FunctionBuilderContext::new();
// Build trampolines for every signature.
for (i, (_, native_sig)) in module.signatures.iter() {
for (i, native_sig) in translation.native_signatures.iter() {
let func = build_trampoline(isa, &mut cx, native_sig, std::mem::size_of::<u128>())?;
// Preserve trampoline function unwind info.
if let Some(info) = &func.unwind_info {
Expand All @@ -55,7 +55,7 @@ pub(crate) fn build_object(
}

let target = ObjectBuilderTarget::new(isa.triple().architecture)?;
let mut builder = ObjectBuilder::new(target, module, funcs);
let mut builder = ObjectBuilder::new(target, &translation.module, funcs);
builder
.set_code_alignment(CODE_SECTION_ALIGNMENT)
.set_trampolines(trampolines)
Expand Down
18 changes: 11 additions & 7 deletions crates/lightbeam/wasmtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,9 @@ use wasmtime_environ::wasm::{
GlobalIndex, MemoryIndex, SignatureIndex, TableIndex,
};
use wasmtime_environ::{
BuiltinFunctionIndex, CompileError, CompiledFunction, Compiler, FunctionBodyData, Module,
ModuleTranslation, Relocation, RelocationTarget, TrapInformation, VMOffsets,
entity::PrimaryMap, BuiltinFunctionIndex, CompileError, CompiledFunction, Compiler,
FunctionBodyData, Module, ModuleTranslation, Relocation, RelocationTarget, TrapInformation,
VMOffsets,
};

/// A compiler that compiles a WebAssembly module with Lightbeam, directly translating the Wasm file.
Expand All @@ -32,7 +33,7 @@ impl Compiler for Lightbeam {
}
let func_index = translation.module.func_index(i);

let env = FuncEnvironment::new(isa.frontend_config().pointer_bytes(), &translation.module);
let env = FuncEnvironment::new(isa.frontend_config().pointer_bytes(), translation);
let mut codegen_session: CodeGenSession<_> = CodeGenSession::new(
translation.function_body_inputs.len() as u32,
&env,
Expand Down Expand Up @@ -180,15 +181,18 @@ struct FuncEnvironment<'module_environment> {
/// The module-level environment which this function-level environment belongs to.
module: &'module_environment Module,

native_signatures: &'module_environment PrimaryMap<SignatureIndex, ir::Signature>,

/// Offsets to struct fields accessed by JIT code.
offsets: VMOffsets,
}

impl<'module_environment> FuncEnvironment<'module_environment> {
fn new(pointer_bytes: u8, module: &'module_environment Module) -> Self {
fn new(pointer_bytes: u8, translation: &'module_environment ModuleTranslation<'_>) -> Self {
Self {
module,
offsets: VMOffsets::new(pointer_bytes, module),
module: &translation.module,
offsets: VMOffsets::new(pointer_bytes, &translation.module),
native_signatures: &translation.native_signatures,
}
}
}
Expand Down Expand Up @@ -227,7 +231,7 @@ impl lightbeam::ModuleContext for FuncEnvironment<'_> {
}

fn signature(&self, index: u32) -> &Self::Signature {
&self.module.signatures[SignatureIndex::from_u32(index)].1
&self.native_signatures[SignatureIndex::from_u32(index)]
}

fn defined_table_index(&self, table_index: u32) -> Option<u32> {
Expand Down
8 changes: 4 additions & 4 deletions crates/wasmtime/src/func.rs
Original file line number Diff line number Diff line change
Expand Up @@ -541,7 +541,7 @@ impl Func {
// Signatures should always be registered in the store's registry of
// shared signatures, so we should be able to unwrap safely here.
let signatures = self.instance.store.signatures().borrow();
let (wft, _, _) = signatures
let (wft, _) = signatures
.lookup_shared(self.sig_index())
.expect("signature should be registered");

Expand All @@ -554,7 +554,7 @@ impl Func {
/// Returns the number of parameters that this function takes.
pub fn param_arity(&self) -> usize {
let signatures = self.instance.store.signatures().borrow();
let (sig, _, _) = signatures
let (sig, _) = signatures
.lookup_shared(self.sig_index())
.expect("signature should be registered");
sig.params.len()
Expand All @@ -563,7 +563,7 @@ impl Func {
/// Returns the number of results this function produces.
pub fn result_arity(&self) -> usize {
let signatures = self.instance.store.signatures().borrow();
let (sig, _, _) = signatures
let (sig, _) = signatures
.lookup_shared(self.sig_index())
.expect("signature should be registered");
sig.returns.len()
Expand Down Expand Up @@ -657,7 +657,7 @@ impl Func {
.borrow()
.lookup_shared(unsafe { export.anyfunc.as_ref().type_index })
.expect("failed to retrieve trampoline from module")
.2;
.1;

Func {
instance,
Expand Down
2 changes: 1 addition & 1 deletion crates/wasmtime/src/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ fn with_imports<R>(
let ty = store
.signatures()
.borrow()
.lookup(&m.signatures[m.functions[i]].0)
.lookup(&m.signatures[m.functions[i]])
.ok_or_else(|| anyhow!("function types incompatible"))?;
if !func.matches_expected(ty) {
bail!("function types incompatible");
Expand Down
7 changes: 3 additions & 4 deletions crates/wasmtime/src/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -915,10 +915,9 @@ impl Store {
module: &'a wasmtime_environ::Module,
) -> impl Fn(wasm::SignatureIndex) -> VMSharedSignatureIndex + 'a {
move |index| {
let (wasm, _native) = &module.signatures[index];
self.signatures()
.borrow()
.lookup(wasm)
.lookup(&module.signatures[index])
.expect("signature not previously registered")
}
}
Expand Down Expand Up @@ -993,8 +992,8 @@ impl Store {
let trampolines = module.compiled_module().trampolines();
let module = module.compiled_module().module();
let mut signatures = self.signatures().borrow_mut();
for (index, (wasm, native)) in module.signatures.iter() {
signatures.register(wasm, native, trampolines[index]);
for (index, wasm) in module.signatures.iter() {
signatures.register(wasm, trampolines[index]);
}
}

Expand Down
10 changes: 3 additions & 7 deletions crates/wasmtime/src/sig_registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
use std::collections::{hash_map, HashMap};
use std::convert::TryFrom;
use wasmtime_environ::{ir, wasm::WasmFuncType};
use wasmtime_environ::wasm::WasmFuncType;
use wasmtime_runtime::{VMSharedSignatureIndex, VMTrampoline};

/// WebAssembly requires that the caller and callee signatures in an indirect
Expand All @@ -25,8 +25,6 @@ pub struct SignatureRegistry {
struct Entry {
// The WebAssembly type signature, using wasm types.
wasm: WasmFuncType,
// The native signature we're using for this wasm type signature.
native: ir::Signature,
// The native trampoline used to invoke this type signature from `Func`.
// Note that the code memory for this trampoline is not owned by this
// type, but instead it's expected to be owned by the store that this
Expand All @@ -39,7 +37,6 @@ impl SignatureRegistry {
pub fn register(
&mut self,
wasm: &WasmFuncType,
native: &ir::Signature,
trampoline: VMTrampoline,
) -> VMSharedSignatureIndex {
let len = self.wasm2index.len();
Expand All @@ -57,7 +54,6 @@ impl SignatureRegistry {
let index = VMSharedSignatureIndex::new(u32::try_from(len).unwrap());
self.index_map.push(Entry {
wasm: wasm.clone(),
native: native.clone(),
trampoline,
});
entry.insert(index);
Expand All @@ -78,9 +74,9 @@ impl SignatureRegistry {
pub fn lookup_shared(
&self,
idx: VMSharedSignatureIndex,
) -> Option<(&WasmFuncType, &ir::Signature, VMTrampoline)> {
) -> Option<(&WasmFuncType, VMTrampoline)> {
self.index_map
.get(idx.bits() as usize)
.map(|e| (&e.wasm, &e.native, e.trampoline))
.map(|e| (&e.wasm, e.trampoline))
}
}
Loading

0 comments on commit 0e02c62

Please sign in to comment.