Skip to content

Commit

Permalink
winch: Add support for WebAssembly loads/stores (#7894)
Browse files Browse the repository at this point in the history 
* winch: Add support for WebAssembly loads/stores

Closes #6529

This patch adds support for all the instructions involving WebAssembly
loads and stores for 32-bit memories. Given that the `memory64` proposal
is not enabled by default, this patch doesn't include an
implementation/tests for it; in theory minimal tweaks to the
currrent implementation will be needed in order to support 64-bit
memories.

Implemenation-wise, this change, follows a similar pattern as Cranelift
in order to calculate addresses for dynamic/static heaps, the main
difference being that in some cases, doing less work at compile time is
preferred; the current implemenation only checks for the general case of
out-of-bounds access for dynamic heaps for example.

Another important detail regarding the implementation, is the
introduction of `MacroAssembler::wasm_load` and
`MacroAssembler::wasm_store`, which internally use a common
implemenation for loads and stores, with the only difference that the
`wasm_*` variants set the right flags in order to signal that these
operations are not trusted and might trap.

Finally, given that this change introduces support for the last set of
instructions missing for a Wasm MVP, it removes most of Winch's copy of
the spectest suite, and switches over to using the official test suite
where possible (for tests that don't use SIMD or Reference Types).

Follow-up items:

* Before doing any deep benchmarking I'm planning on landing a couple of
  improvements regarding compile times that I've identified in parallel
  to this change.
* The `imports.wast` tests are disabled because I've identified a bug
  with `call_indirect`, which is not related to this change and exists
  in main.
* Find a way to run the `tests/all/memory.rs` (or perhaps most of
  integration tests) with Winch.

--
prtest:full

* Review comments
  • Loading branch information
@saulecabrera
saulecabrera authored Feb 9, 2024
1 parent cee0eeb commit 83cf743
Show file tree
Hide file tree
Showing 52 changed files with 1,429 additions and 16,420 deletions.
25 changes: 22 additions & 3 deletions build.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ fn write_testsuite_tests(
fn ignore(testsuite: &str, testname: &str, strategy: &str) -> bool {
assert!(strategy == "Cranelift" || strategy == "Winch");

// Ignore everything except the winch misc test suite.
// Ignore some tests for when testing Winch.
if strategy == "Winch" {
if testsuite == "misc_testsuite" {
// The misc/call_indirect is fully supported by Winch.
Expand All @@ -212,8 +212,27 @@ fn ignore(testsuite: &str, testname: &str, strategy: &str) -> bool {
}
}
if testsuite == "spec_testsuite" {
// The official following tests are supported.
return !["table_init", "table_copy"].contains(&testname);
let denylist = [
"br_table",
"conversions",
"global",
"table_fill",
"table_get",
"table_set",
"table_grow",
"table_size",
"elem",
"select",
"unreached_invalid",
"imports",
"linking",
]
.contains(&testname);

let ref_types = testname.starts_with("ref_");
let simd = testname.starts_with("simd_");

return denylist || ref_types || simd;
}

if testsuite != "winch" {
Expand Down
3 changes: 2 additions & 1 deletion cranelift/codegen/src/isa/x64/inst/args.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,8 @@ impl Amode {
Self::RipRelative { target }
}

pub(crate) fn with_flags(&self, flags: MemFlags) -> Self {
/// Set the specified [MemFlags] to the [Amode].
pub fn with_flags(&self, flags: MemFlags) -> Self {
match self {
&Self::ImmReg { simm32, base, .. } => Self::ImmReg {
simm32,
Expand Down
177 changes: 19 additions & 158 deletions fuzz/fuzz_targets/differential.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,17 @@ fn execute_one(data: &[u8]) -> Result<()> {
// `Cranelift`.
if fuzz_winch {
config.wasmtime.compiler_strategy = CompilerStrategy::Winch;
// Disable the Wasm proposals not supported by Winch.
// Reference Types and (Function References) are not disabled entirely
// because certain instructions involving `funcref` are supported (all
// the table instructions).
config.module_config.config.simd_enabled = false;
config.module_config.config.relaxed_simd_enabled = false;
config.module_config.config.memory64_enabled = false;
config.module_config.config.gc_enabled = false;
config.module_config.config.threads_enabled = false;
config.module_config.config.tail_call_enabled = false;
config.module_config.config.exceptions_enabled = false;
}

// Choose an engine that Wasmtime will be differentially executed against.
Expand Down Expand Up @@ -284,7 +295,6 @@ fn winch_supports_module(module: &[u8]) -> bool {

fn is_type_supported(ty: &ValType) -> bool {
match ty {
ValType::V128 => false,
ValType::Ref(r) => r.is_func_ref(),
_ => true,
}
Expand All @@ -307,166 +317,17 @@ fn winch_supports_module(module: &[u8]) -> bool {
let op_reader = body.get_operators_reader().unwrap();
for op in op_reader {
match op.unwrap() {
I32Const { .. }
| I64Const { .. }
| I32Add { .. }
| I64Add { .. }
| I32Sub { .. }
| I32Mul { .. }
| I32DivS { .. }
| I32DivU { .. }
| I64DivS { .. }
| I64DivU { .. }
| I64RemU { .. }
| I64RemS { .. }
| I32RemU { .. }
| I32RemS { .. }
| I64Mul { .. }
| I64Sub { .. }
| I32Eq { .. }
| I64Eq { .. }
| I32Ne { .. }
| I64Ne { .. }
| I32LtS { .. }
| I64LtS { .. }
| I32LtU { .. }
| I64LtU { .. }
| I32LeS { .. }
| I64LeS { .. }
| I32LeU { .. }
| I64LeU { .. }
| I32GtS { .. }
| I64GtS { .. }
| I32GtU { .. }
| I64GtU { .. }
| I32GeS { .. }
| I64GeS { .. }
| I32GeU { .. }
| I64GeU { .. }
| I32Eqz { .. }
| I64Eqz { .. }
| I32And { .. }
| I64And { .. }
| I32Or { .. }
| I64Or { .. }
| I32Xor { .. }
| I64Xor { .. }
| I32Shl { .. }
| I64Shl { .. }
| I32ShrS { .. }
| I64ShrS { .. }
| I32ShrU { .. }
| I64ShrU { .. }
| I32Rotl { .. }
| I64Rotl { .. }
| I32Rotr { .. }
| I64Rotr { .. }
| I32Clz { .. }
| I64Clz { .. }
| I32Ctz { .. }
| I64Ctz { .. }
| I32Popcnt { .. }
| I64Popcnt { .. }
| I32WrapI64 { .. }
| I64ExtendI32S { .. }
| I64ExtendI32U { .. }
| I32Extend8S { .. }
| I32Extend16S { .. }
| I64Extend8S { .. }
| I64Extend16S { .. }
| I64Extend32S { .. }
| I32TruncF32S { .. }
| I32TruncF32U { .. }
| I32TruncF64S { .. }
| I32TruncF64U { .. }
| I64TruncF32S { .. }
| I64TruncF32U { .. }
| I64TruncF64S { .. }
| I64TruncF64U { .. }
| I32ReinterpretF32 { .. }
| I64ReinterpretF64 { .. }
| LocalGet { .. }
| LocalSet { .. }
| LocalTee { .. }
| GlobalGet { .. }
| GlobalSet { .. }
| Call { .. }
| Nop { .. }
| End { .. }
| If { .. }
| Else { .. }
| Block { .. }
| Loop { .. }
| Br { .. }
| BrIf { .. }
| BrTable { .. }
| Unreachable { .. }
| Return { .. }
| F32Const { .. }
| F64Const { .. }
| F32Add { .. }
| F64Add { .. }
| F32Sub { .. }
| F64Sub { .. }
| F32Mul { .. }
| F64Mul { .. }
| F32Div { .. }
| F64Div { .. }
| F32Min { .. }
| F64Min { .. }
| F32Max { .. }
| F64Max { .. }
| F32Copysign { .. }
| F64Copysign { .. }
| F32Abs { .. }
| F64Abs { .. }
| F32Neg { .. }
| F64Neg { .. }
| F32Sqrt { .. }
| F64Sqrt { .. }
| F32Eq { .. }
| F64Eq { .. }
| F32Ne { .. }
| F64Ne { .. }
| F32Lt { .. }
| F64Lt { .. }
| F32Gt { .. }
| F64Gt { .. }
| F32Le { .. }
| F64Le { .. }
| F32Ge { .. }
| F64Ge { .. }
| F32ConvertI32S { .. }
| F32ConvertI32U { .. }
| F32ConvertI64S { .. }
| F32ConvertI64U { .. }
| F64ConvertI32S { .. }
| F64ConvertI32U { .. }
| F64ConvertI64S { .. }
| F64ConvertI64U { .. }
| F32ReinterpretI32 { .. }
| F64ReinterpretI64 { .. }
| F32DemoteF64 { .. }
| F64PromoteF32 { .. }
| CallIndirect { .. }
| ElemDrop { .. }
| TableCopy { .. }
| TableSet { .. }
| TableGet { .. }
| TableFill { .. }
| TableGrow { .. }
| TableSize { .. }
| TableInit { .. }
| MemoryCopy { .. }
| MemoryGrow { .. }
| MemoryFill { .. }
| MemoryInit { .. }
| DataDrop { .. }
| MemorySize { .. } => {}
_ => {
RefIsNull { .. }
| RefNull { .. }
| RefFunc { .. }
| RefAsNonNull { .. }
| BrOnNonNull { .. }
| CallRef { .. }
| BrOnNull { .. } => {
supported = false;
break 'main;
}
_ => {}
}
}
}
Expand Down
Loading

0 comments on commit 83cf743

Please sign in to comment.