Skip to content

Commit

Permalink
Address review feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
@fitzgen
fitzgen committed Feb 25, 2020
1 parent c761553 commit 1468fb5
Show file tree
Hide file tree
Showing 15 changed files with 220 additions and 347 deletions.
96 changes: 55 additions & 41 deletions build.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,40 +26,38 @@ fn main() -> anyhow::Result<()> {
writeln!(out, "#[allow(non_snake_case)]")?;
writeln!(out, "mod {} {{", strategy)?;

test_directory(&mut out, "tests/misc_testsuite", strategy)?;
let spec_tests = test_directory(&mut out, "tests/spec_testsuite", strategy)?;
// Skip running spec_testsuite tests if the submodule isn't checked
// out.
if spec_tests > 0 {
test_directory(&mut out, "tests/spec_testsuite/proposals/simd", strategy)
.expect("generating tests");

test_directory(
&mut out,
"tests/spec_testsuite/proposals/multi-value",
strategy,
)
.expect("generating tests");

test_directory(
&mut out,
"tests/spec_testsuite/proposals/reference-types",
strategy,
)
.expect("generating tests");

test_directory(
&mut out,
"tests/spec_testsuite/proposals/bulk-memory-operations",
strategy,
)
.expect("generating tests");
} else {
println!(
"cargo:warning=The spec testsuite is disabled. To enable, run `git submodule \
with_test_module(&mut out, "misc", |out| {
test_directory(out, "tests/misc_testsuite", strategy)?;
test_directory_module(out, "tests/misc_testsuite/bulk-memory-operations", strategy)?;
test_directory_module(out, "tests/misc_testsuite/reference-types", strategy)?;
Ok(())
})?;

with_test_module(&mut out, "spec", |out| {
let spec_tests = test_directory(out, "tests/spec_testsuite", strategy)?;
// Skip running spec_testsuite tests if the submodule isn't checked
// out.
if spec_tests > 0 {
test_directory_module(out, "tests/spec_testsuite/proposals/simd", strategy)?;
test_directory_module(out, "tests/spec_testsuite/proposals/multi-value", strategy)?;
test_directory_module(
out,
"tests/spec_testsuite/proposals/reference-types",
strategy,
)?;
test_directory_module(
out,
"tests/spec_testsuite/proposals/bulk-memory-operations",
strategy,
)?;
} else {
println!(
"cargo:warning=The spec testsuite is disabled. To enable, run `git submodule \
update --remote`."
);
}
);
}
Ok(())
})?;

writeln!(out, "}}")?;
}
Expand All @@ -72,6 +70,16 @@ fn main() -> anyhow::Result<()> {
Ok(())
}

fn test_directory_module(
out: &mut String,
path: impl AsRef<Path>,
strategy: &str,
) -> anyhow::Result<usize> {
let path = path.as_ref();
let testsuite = &extract_name(path);
with_test_module(out, testsuite, |out| test_directory(out, path, strategy))
}

fn test_directory(
out: &mut String,
path: impl AsRef<Path>,
Expand Down Expand Up @@ -100,11 +108,10 @@ fn test_directory(
dir_entries.sort();

let testsuite = &extract_name(path);
start_test_module(out, testsuite)?;
for entry in dir_entries.iter() {
write_testsuite_tests(out, entry, testsuite, strategy)?;
}
finish_test_module(out)?;

Ok(dir_entries.len())
}

Expand All @@ -119,14 +126,19 @@ fn extract_name(path: impl AsRef<Path>) -> String {
.replace("/", "_")
}

fn start_test_module(out: &mut String, testsuite: &str) -> anyhow::Result<()> {
writeln!(out, "mod {} {{", testsuite)?;
Ok(())
}
fn with_test_module<T>(
out: &mut String,
testsuite: &str,
f: impl FnOnce(&mut String) -> anyhow::Result<T>,
) -> anyhow::Result<T> {
out.push_str("mod ");
out.push_str(testsuite);
out.push_str(" {\n");

let result = f(out)?;

fn finish_test_module(out: &mut String) -> anyhow::Result<()> {
out.push_str("}\n");
Ok(())
Ok(result)
}

fn write_testsuite_tests(
Expand Down Expand Up @@ -180,6 +192,8 @@ fn ignore(testsuite: &str, testname: &str, strategy: &str) -> bool {
("simd", "simd_load_splat") => return true, // FIXME Unsupported feature: proposed SIMD operator V8x16LoadSplat { memarg: MemoryImmediate { flags: 0, offset: 0 } }
("simd", "simd_splat") => return true, // FIXME Unsupported feature: proposed SIMD operator I8x16ShrS

// Still working on implementing these. See #929.
("reference_types", "table_copy_on_imported_tables") => return false,
("reference_types", _) => return true,

// Still working on implementing these. See #928
Expand Down
13 changes: 3 additions & 10 deletions crates/api/src/instance.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use crate::runtime::{Config, Store};
use crate::trap::Trap;
use anyhow::{Error, Result};
use wasmtime_jit::{CompiledModule, Resolver};
use wasmtime_runtime::{Export, InstanceHandle, InstantiationError, LinkError};
use wasmtime_runtime::{Export, InstanceHandle, InstantiationError};

struct SimpleResolver<'a> {
imports: &'a [Extern],
Expand Down Expand Up @@ -32,15 +32,8 @@ fn instantiate(
)
.map_err(|e| -> Error {
match e {
InstantiationError::StartTrap(trap) => Trap::from_jit(trap).into(),
e @ InstantiationError::TableOutOfBounds(_)
| e @ InstantiationError::MemoryOutOfBounds(_) => {
let msg = e.to_string();
if config.validating_config.operator_config.enable_bulk_memory {
Trap::new(msg).into()
} else {
InstantiationError::Link(LinkError(msg)).into()
}
InstantiationError::StartTrap(trap) | InstantiationError::Trap(trap) => {
Trap::from_jit(trap).into()
}
other => other.into(),
}
Expand Down
96 changes: 16 additions & 80 deletions crates/environ/src/func_environ.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,50 +45,36 @@ impl BuiltinFunctionIndex {
}
/// Returns an index for wasm's `table.copy` when both tables are locally
/// defined.
pub const fn get_table_copy_defined_defined_index() -> Self {
pub const fn get_table_copy_index() -> Self {
Self(4)
}
/// Returns an index for wasm's `table.copy` when the destination table is
/// locally defined and the source table is imported.
pub const fn get_table_copy_defined_imported_index() -> Self {
Self(5)
}
/// Returns an index for wasm's `table.copy` when the destination table is
/// imported and the source table is locally defined.
pub const fn get_table_copy_imported_defined_index() -> Self {
Self(6)
}
/// Returns an index for wasm's `table.copy` when both tables are imported.
pub const fn get_table_copy_imported_imported_index() -> Self {
Self(7)
}
/// Returns an index for wasm's `table.init`.
pub const fn get_table_init_index() -> Self {
Self(8)
Self(5)
}
/// Returns an index for wasm's `elem.drop`.
pub const fn get_elem_drop_index() -> Self {
Self(9)
Self(6)
}
/// Returns an index for wasm's `memory.copy` for locally defined memories.
pub const fn get_memory_copy_index() -> Self {
Self(10)
pub const fn get_defined_memory_copy_index() -> Self {
Self(7)
}
/// Returns an index for wasm's `memory.copy` for imported memories.
pub const fn get_imported_memory_copy_index() -> Self {
Self(11)
Self(8)
}
/// Returns an index for wasm's `memory.fill` for locally defined memories.
pub const fn get_memory_fill_index() -> Self {
Self(12)
Self(9)
}
/// Returns an index for wasm's `memory.fill` for imported memories.
pub const fn get_imported_memory_fill_index() -> Self {
Self(13)
Self(10)
}
/// Returns the total number of builtin functions.
pub const fn builtin_functions_total_number() -> u32 {
14
11
}

/// Return the index as an u32 number.
Expand Down Expand Up @@ -242,7 +228,6 @@ impl<'module_environment> FuncEnvironment<'module_environment> {
}
}

// NB: All `table_copy` libcall variants have the same signature.
fn get_table_copy_sig(&mut self, func: &mut Function) -> ir::SigRef {
let sig = self.table_copy_sig.unwrap_or_else(|| {
func.import_signature(Signature {
Expand Down Expand Up @@ -276,61 +261,12 @@ impl<'module_environment> FuncEnvironment<'module_environment> {
src_table_index: TableIndex,
) -> (ir::SigRef, usize, usize, BuiltinFunctionIndex) {
let sig = self.get_table_copy_sig(func);
match (
self.module.is_imported_table(dst_table_index),
self.module.is_imported_table(src_table_index),
) {
(false, false) => {
let dst_table_index = self
.module
.defined_table_index(dst_table_index)
.unwrap()
.index();
let src_table_index = self
.module
.defined_table_index(src_table_index)
.unwrap()
.index();
(
sig,
dst_table_index,
src_table_index,
BuiltinFunctionIndex::get_table_copy_defined_defined_index(),
)
}
(false, true) => {
let dst_table_index = self
.module
.defined_table_index(dst_table_index)
.unwrap()
.index();
(
sig,
dst_table_index,
src_table_index.as_u32() as usize,
BuiltinFunctionIndex::get_table_copy_defined_imported_index(),
)
}
(true, false) => {
let src_table_index = self
.module
.defined_table_index(src_table_index)
.unwrap()
.index();
(
sig,
dst_table_index.as_u32() as usize,
src_table_index,
BuiltinFunctionIndex::get_table_copy_imported_defined_index(),
)
}
(true, true) => (
sig,
dst_table_index.as_u32() as usize,
src_table_index.as_u32() as usize,
BuiltinFunctionIndex::get_table_copy_imported_imported_index(),
),
}
(
sig,
dst_table_index.as_u32() as usize,
src_table_index.as_u32() as usize,
BuiltinFunctionIndex::get_table_copy_index(),
)
}

fn get_table_init_sig(&mut self, func: &mut Function) -> ir::SigRef {
Expand Down Expand Up @@ -428,7 +364,7 @@ impl<'module_environment> FuncEnvironment<'module_environment> {
(
sig,
defined_memory_index.index(),
BuiltinFunctionIndex::get_memory_copy_index(),
BuiltinFunctionIndex::get_defined_memory_copy_index(),
)
} else {
(
Expand Down
Loading

0 comments on commit 1468fb5

Please sign in to comment.