Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make the memory.{grow,size} library calls libcalls instead of pointer-to-function fields on the Instance. #2086

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions lib/compiler-llvm/src/object_file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,16 @@ where
libcalls.insert("nearbyintf".to_string(), LibCall::NearestF32);
libcalls.insert("nearbyint".to_string(), LibCall::NearestF64);
libcalls.insert("wasmer_probestack".to_string(), LibCall::Probestack);
libcalls.insert("wasmer_memory32_grow".to_string(), LibCall::Memory32Grow);
libcalls.insert(
"wasmer_imported_memory32_grow".to_string(),
LibCall::ImportedMemory32Grow,
);
libcalls.insert("wasmer_memory32_size".to_string(), LibCall::Memory32Size);
libcalls.insert(
"wasmer_imported_memory32_size".to_string(),
LibCall::ImportedMemory32Size,
);

let elf = goblin::elf::Elf::parse(&contents).map_err(map_goblin_err)?;
let get_section_name = |section: &goblin::elf::section_header::SectionHeader| {
Expand Down
8 changes: 4 additions & 4 deletions lib/compiler-llvm/src/translator/code.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9288,9 +9288,9 @@ impl<'ctx, 'a> LLVMFunctionCodeGenerator<'ctx, 'a> {
Operator::MemoryGrow { mem, mem_byte: _ } => {
let memory_index = MemoryIndex::from_u32(mem);
let delta = self.state.pop1()?;
let grow_fn_ptr = self.ctx.memory_grow(memory_index, self.intrinsics);
let grow_fn = self.ctx.memory_grow(memory_index, self.intrinsics);
let grow = self.builder.build_call(
grow_fn_ptr,
grow_fn,
&[
vmctx.as_basic_value_enum(),
delta,
Expand All @@ -9305,9 +9305,9 @@ impl<'ctx, 'a> LLVMFunctionCodeGenerator<'ctx, 'a> {
}
Operator::MemorySize { mem, mem_byte: _ } => {
let memory_index = MemoryIndex::from_u32(mem);
let size_fn_ptr = self.ctx.memory_size(memory_index, self.intrinsics);
let size_fn = self.ctx.memory_size(memory_index, self.intrinsics);
let size = self.builder.build_call(
size_fn_ptr,
size_fn,
&[
vmctx.as_basic_value_enum(),
self.intrinsics
Expand Down
134 changes: 39 additions & 95 deletions lib/compiler-llvm/src/translator/intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ use wasmer_types::{
Mutability, SignatureIndex, TableIndex, Type,
};
use wasmer_vm::ModuleInfo as WasmerCompilerModule;
use wasmer_vm::{MemoryStyle, TrapCode, VMBuiltinFunctionIndex, VMOffsets};
use wasmer_vm::{MemoryStyle, TrapCode, VMOffsets};

pub fn type_to_llvm_ptr<'ctx>(
intrinsics: &Intrinsics<'ctx>,
Expand Down Expand Up @@ -180,10 +180,10 @@ pub struct Intrinsics<'ctx> {
pub vmmemory_definition_base_element: u32,
pub vmmemory_definition_current_length_element: u32,

pub memory32_grow_ptr_ty: PointerType<'ctx>,
pub imported_memory32_grow_ptr_ty: PointerType<'ctx>,
pub memory32_size_ptr_ty: PointerType<'ctx>,
pub imported_memory32_size_ptr_ty: PointerType<'ctx>,
pub memory32_grow: FunctionValue<'ctx>,
pub imported_memory32_grow: FunctionValue<'ctx>,
pub memory32_size: FunctionValue<'ctx>,
pub imported_memory32_size: FunctionValue<'ctx>,

pub ctx_ptr_ty: PointerType<'ctx>,
}
Expand Down Expand Up @@ -468,25 +468,32 @@ impl<'ctx> Intrinsics<'ctx> {
vmmemory_definition_base_element: 0,
vmmemory_definition_current_length_element: 1,

memory32_grow_ptr_ty: i32_ty
.fn_type(
memory32_grow: module.add_function(
"wasmer_memory32_grow",
i32_ty.fn_type(
&[ctx_ptr_ty.as_basic_type_enum(), i32_ty_basic, i32_ty_basic],
false,
)
.ptr_type(AddressSpace::Generic),
imported_memory32_grow_ptr_ty: i32_ty
.fn_type(
),
None,
),
imported_memory32_grow: module.add_function(
"wasmer_imported_memory32_grow",
i32_ty.fn_type(
&[ctx_ptr_ty.as_basic_type_enum(), i32_ty_basic, i32_ty_basic],
false,
)
.ptr_type(AddressSpace::Generic),
memory32_size_ptr_ty: i32_ty
.fn_type(&[ctx_ptr_ty.as_basic_type_enum(), i32_ty_basic], false)
.ptr_type(AddressSpace::Generic),
imported_memory32_size_ptr_ty: i32_ty
.fn_type(&[ctx_ptr_ty.as_basic_type_enum(), i32_ty_basic], false)
.ptr_type(AddressSpace::Generic),

),
None,
),
memory32_size: module.add_function(
"wasmer_memory32_size",
i32_ty.fn_type(&[ctx_ptr_ty.as_basic_type_enum(), i32_ty_basic], false),
None,
),
imported_memory32_size: module.add_function(
"wasmer_imported_memory32_size",
i32_ty.fn_type(&[ctx_ptr_ty.as_basic_type_enum(), i32_ty_basic], false),
None,
),
ctx_ptr_ty,
};

Expand Down Expand Up @@ -543,8 +550,6 @@ pub struct CtxType<'ctx, 'a> {
cached_sigindices: HashMap<SignatureIndex, IntValue<'ctx>>,
cached_globals: HashMap<GlobalIndex, GlobalCache<'ctx>>,
cached_functions: HashMap<FunctionIndex, FunctionCache<'ctx>>,
cached_memory_grow: HashMap<MemoryIndex, PointerValue<'ctx>>,
cached_memory_size: HashMap<MemoryIndex, PointerValue<'ctx>>,

offsets: VMOffsets,
}
Expand All @@ -568,8 +573,6 @@ impl<'ctx, 'a> CtxType<'ctx, 'a> {
cached_sigindices: HashMap::new(),
cached_globals: HashMap::new(),
cached_functions: HashMap::new(),
cached_memory_grow: HashMap::new(),
cached_memory_size: HashMap::new(),

// TODO: pointer width
offsets: VMOffsets::new(8, &wasm_module),
Expand Down Expand Up @@ -1025,83 +1028,24 @@ impl<'ctx, 'a> CtxType<'ctx, 'a> {
&mut self,
memory_index: MemoryIndex,
intrinsics: &Intrinsics<'ctx>,
) -> PointerValue<'ctx> {
let (cached_memory_grow, wasm_module, offsets, cache_builder, ctx_ptr_value) = (
&mut self.cached_memory_grow,
&self.wasm_module,
&self.offsets,
&self.cache_builder,
&self.ctx_ptr_value,
);
*cached_memory_grow.entry(memory_index).or_insert_with(|| {
let (grow_fn, grow_fn_ty) = if wasm_module.local_memory_index(memory_index).is_some() {
(
VMBuiltinFunctionIndex::get_memory32_grow_index(),
intrinsics.memory32_grow_ptr_ty,
)
} else {
(
VMBuiltinFunctionIndex::get_imported_memory32_grow_index(),
intrinsics.imported_memory32_grow_ptr_ty,
)
};
let offset = offsets.vmctx_builtin_function(grow_fn);
let offset = intrinsics.i32_ty.const_int(offset.into(), false);
let grow_fn_ptr_ptr = unsafe { cache_builder.build_gep(*ctx_ptr_value, &[offset], "") };

let grow_fn_ptr_ptr = cache_builder
.build_bitcast(
grow_fn_ptr_ptr,
grow_fn_ty.ptr_type(AddressSpace::Generic),
"",
)
.into_pointer_value();
cache_builder
.build_load(grow_fn_ptr_ptr, "")
.into_pointer_value()
})
) -> FunctionValue<'ctx> {
if self.wasm_module.local_memory_index(memory_index).is_some() {
intrinsics.memory32_grow
} else {
intrinsics.imported_memory32_grow
}
}

pub fn memory_size(
&mut self,
memory_index: MemoryIndex,
intrinsics: &Intrinsics<'ctx>,
) -> PointerValue<'ctx> {
let (cached_memory_size, wasm_module, offsets, cache_builder, ctx_ptr_value) = (
&mut self.cached_memory_size,
&self.wasm_module,
&self.offsets,
&self.cache_builder,
&self.ctx_ptr_value,
);
*cached_memory_size.entry(memory_index).or_insert_with(|| {
let (size_fn, size_fn_ty) = if wasm_module.local_memory_index(memory_index).is_some() {
(
VMBuiltinFunctionIndex::get_memory32_size_index(),
intrinsics.memory32_size_ptr_ty,
)
} else {
(
VMBuiltinFunctionIndex::get_imported_memory32_size_index(),
intrinsics.imported_memory32_size_ptr_ty,
)
};
let offset = offsets.vmctx_builtin_function(size_fn);
let offset = intrinsics.i32_ty.const_int(offset.into(), false);
let size_fn_ptr_ptr = unsafe { cache_builder.build_gep(*ctx_ptr_value, &[offset], "") };

let size_fn_ptr_ptr = cache_builder
.build_bitcast(
size_fn_ptr_ptr,
size_fn_ty.ptr_type(AddressSpace::Generic),
"",
)
.into_pointer_value();

cache_builder
.build_load(size_fn_ptr_ptr, "")
.into_pointer_value()
})
) -> FunctionValue<'ctx> {
if self.wasm_module.local_memory_index(memory_index).is_some() {
intrinsics.memory32_size
} else {
intrinsics.imported_memory32_size
}
}

pub fn get_offsets(&self) -> &VMOffsets {
Expand Down
26 changes: 25 additions & 1 deletion lib/vm/src/libcalls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ pub extern "C" fn wasmer_f64_nearest(x: f64) -> f64 {
/// # Safety
///
/// `vmctx` must be valid and not null.
#[no_mangle]
pub unsafe extern "C" fn wasmer_memory32_grow(
vmctx: *mut VMContext,
delta: u32,
Expand All @@ -156,6 +157,7 @@ pub unsafe extern "C" fn wasmer_memory32_grow(
/// # Safety
///
/// `vmctx` must be valid and not null.
#[no_mangle]
pub unsafe extern "C" fn wasmer_imported_memory32_grow(
vmctx: *mut VMContext,
delta: u32,
Expand All @@ -175,6 +177,7 @@ pub unsafe extern "C" fn wasmer_imported_memory32_grow(
/// # Safety
///
/// `vmctx` must be valid and not null.
#[no_mangle]
pub unsafe extern "C" fn wasmer_memory32_size(vmctx: *mut VMContext, memory_index: u32) -> u32 {
let instance = (&*vmctx).instance();
let memory_index = LocalMemoryIndex::from_u32(memory_index);
Expand All @@ -187,6 +190,7 @@ pub unsafe extern "C" fn wasmer_memory32_size(vmctx: *mut VMContext, memory_inde
/// # Safety
///
/// `vmctx` must be valid and not null.
#[no_mangle]
pub unsafe extern "C" fn wasmer_imported_memory32_size(
vmctx: *mut VMContext,
memory_index: u32,
Expand Down Expand Up @@ -435,8 +439,20 @@ pub enum LibCall {
/// trunc.f32
TruncF32,

/// frunc.f64
/// trunc.f64
TruncF64,

/// memory.grow for a locally defined memory
Memory32Grow,

/// memory.grow for an imported memory
ImportedMemory32Grow,

/// memory.size for a locally defined memory
Memory32Size,

/// memory.size for an imported memory
ImportedMemory32Size,
}

impl LibCall {
Expand All @@ -453,6 +469,10 @@ impl LibCall {
Self::RaiseTrap => wasmer_raise_trap as usize,
Self::TruncF32 => wasmer_f32_trunc as usize,
Self::TruncF64 => wasmer_f64_trunc as usize,
Self::Memory32Grow => wasmer_memory32_grow as usize,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bit confusing.
Were this libcalls not available before? How cranelift was using them?

Copy link
Contributor Author

@nlewycky nlewycky Feb 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They were not libcalls before! This PR doesn't change how singlepass or cranelift use them, which is to use the "vm builtins" mechanism where every instance has pointers-to these functions as fields in the instance. See translate_memory_grow in compiler-cranelift's func_environ.rs:

        let (vmctx, func_addr) = self.translate_load_builtin_function_address(&mut pos, func_idx);
        let call_inst = pos
            .ins()
            .call_indirect(func_sig, func_addr, &[vmctx, val, memory_index]);

translate_load_builtin_function_address will load the function pointer out of the instance itself (using VMBuiltinFunctionIndex on VMOffsets and then return that function pointer.

IMO the direction should be that we remove the VMBuiltinFunction system entirely, unless you know of a good reason why we want to have function for different instances at runtime and don't know this at compile time.

Self::ImportedMemory32Grow => wasmer_imported_memory32_grow as usize,
Self::Memory32Size => wasmer_memory32_size as usize,
Self::ImportedMemory32Size => wasmer_imported_memory32_size as usize,
}
}

Expand All @@ -474,6 +494,10 @@ impl LibCall {
Self::RaiseTrap => "wasmer_raise_trap",
Self::TruncF32 => "wasmer_f32_trunc",
Self::TruncF64 => "wasmer_f64_trunc",
Self::Memory32Grow => "wasmer_memory32_grow",
Self::ImportedMemory32Grow => "wasmer_imported_memory32_grow",
Self::Memory32Size => "wasmer_memory32_size",
Self::ImportedMemory32Size => "wasmer_imported_memory32_size",
}
}
}
Expand Down