Skip to content

Commit

Permalink
remove faulty br_table optimization & adjust tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@Robbepop
Robbepop committed Sep 18, 2024
1 parent 03323b9 commit a646d27
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 44 deletions.
24 changes: 12 additions & 12 deletions crates/wasmi/src/engine/translator/tests/fuzz/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,13 +65,9 @@ fn fuzz_regression_3() {
EngineFunc::from_u32(0),
),
Instruction::branch_table(Register::from_i16(5), 2),
Instruction::copy_span_non_overlapping(
RegisterSpan::new(Register::from_i16(0)),
RegisterSpan::new(Register::from_i16(2)),
3,
),
Instruction::return_span(RegisterSpan::new(Register::from_i16(0)).iter_u16(3)),
Instruction::return_span(RegisterSpan::new(Register::from_i16(0)).iter_u16(3)),
Instruction::branch(BranchOffset::from(2)),
Instruction::branch(BranchOffset::from(1)),
Instruction::return_reg3(2, 3, 4),
])
.run()
}
Expand Down Expand Up @@ -357,10 +353,12 @@ fn fuzz_regression_15_01_codegen() {
ExpectedFunc::new([
Instruction::i32_wrap_i64(Register::from_i16(1), Register::from_i16(0)),
Instruction::branch_table(Register::from_i16(1), 3),
Instruction::copy_imm32(Register::from_i16(1), 10.0_f32),
Instruction::branch(BranchOffset::from(3)),
Instruction::return_reg(1),
Instruction::branch(BranchOffset::from(4)),
Instruction::branch(BranchOffset::from(1)),
Instruction::copy_imm32(Register::from_i16(1), 10.0_f32),
Instruction::branch(BranchOffset::from(2)),
Instruction::return_imm32(10.0_f32),
Instruction::Trap(TrapCode::UnreachableCodeReached),
]),
)
Expand Down Expand Up @@ -403,14 +401,16 @@ fn fuzz_regression_15_02() {
ExpectedFunc::new([
Instruction::i32_wrap_i64(Register::from_i16(1), Register::from_i16(0)),
Instruction::branch_table(Register::from_i16(1), 3),
Instruction::branch(BranchOffset::from(3)),
Instruction::branch(BranchOffset::from(4)),
Instruction::branch(BranchOffset::from(1)),
Instruction::copy2(
RegisterSpan::new(Register::from_i16(1)),
Register::from_i16(-1),
Register::from_i16(-2),
),
Instruction::branch(BranchOffset::from(3)),
Instruction::return_span(RegisterSpan::new(Register::from_i16(1)).iter_u16(2)),
Instruction::branch(BranchOffset::from(1)),
Instruction::branch(BranchOffset::from(2)),
Instruction::return_reg2(-1, -2),
Instruction::Trap(TrapCode::UnreachableCodeReached),
])
.consts([10.0_f32, 20.0_f32]),
Expand Down
49 changes: 36 additions & 13 deletions crates/wasmi/src/engine/translator/tests/op/br_table.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,12 +127,20 @@ fn reg_params_1_return() {
TranslationTest::from_wat(wasm)
.expect_func_instrs([
Instruction::branch_table(index, 5),
Instruction::copy(result, value),
Instruction::return_reg(result),
Instruction::branch(BranchOffset::from(13)),
Instruction::branch(BranchOffset::from(10)),
Instruction::branch(BranchOffset::from(7)),
Instruction::branch(BranchOffset::from(4)),
Instruction::branch(BranchOffset::from(1)),
Instruction::copy(result, value),
Instruction::branch(BranchOffset::from(8)),
Instruction::copy(result, value),
Instruction::branch(BranchOffset::from(8)),
Instruction::copy(result, value),
Instruction::branch(BranchOffset::from(8)),
Instruction::copy(result, value),
Instruction::branch(BranchOffset::from(8)),
Instruction::return_reg(1),
Instruction::i32_add_imm16(result, result, 10),
Instruction::return_reg(result),
Instruction::i32_add_imm16(result, result, 20),
Expand Down Expand Up @@ -171,23 +179,30 @@ fn reg_params_1_pass() {
)
)";
let index = Register::from_i16(0);
let value = Register::from_i16(1);
let result = Register::from_i16(2);
TranslationTest::from_wat(wasm)
.expect_func_instrs([
Instruction::branch_table(index, 3),
Instruction::copy(result, value),
Instruction::branch(BranchOffset::from(7)),
Instruction::branch(BranchOffset::from(4)),
Instruction::branch(BranchOffset::from(1)),
Instruction::copy(2, 1),
Instruction::branch(BranchOffset::from(5)),
Instruction::copy(2, 1),
Instruction::branch(BranchOffset::from(5)),
Instruction::copy(2, 1),
Instruction::branch(BranchOffset::from(5)),
Instruction::copy_imm32(Register::from_i16(3), 10_i32),
Instruction::branch(BranchOffset::from(5)),
Instruction::copy_imm32(Register::from_i16(3), 20_i32),
Instruction::branch(BranchOffset::from(3)),
Instruction::copy_imm32(Register::from_i16(3), 30_i32),
Instruction::branch(BranchOffset::from(1)),
Instruction::i32_add(result, result, Register::from_i16(3)),
Instruction::return_reg(result),
Instruction::i32_add(
Register::from(2),
Register::from_i16(2),
Register::from_i16(3),
),
Instruction::return_reg(2),
])
.run()
}
Expand All @@ -213,15 +228,19 @@ fn reg_params_2_ops() {
)
)";
let index = Register::from_i16(0);
let lhs = Register::from_i16(1);
let result = Register::from_i16(3);
TranslationTest::from_wat(wasm)
.expect_func_instrs([
Instruction::branch_table(index, 3),
Instruction::copy2(RegisterSpan::new(result), lhs, lhs.next()),
Instruction::branch(BranchOffset::from(7)),
Instruction::branch(BranchOffset::from(4)),
Instruction::branch(BranchOffset::from(1)),
Instruction::copy2(RegisterSpan::new(result), 1, 2),
Instruction::branch(BranchOffset::from(5)),
Instruction::copy2(RegisterSpan::new(result), 1, 2),
Instruction::branch(BranchOffset::from(5)),
Instruction::copy2(RegisterSpan::new(result), 1, 2),
Instruction::branch(BranchOffset::from(5)),
Instruction::i32_add(result, result, Register::from_i16(4)),
Instruction::return_reg(result),
Instruction::i32_sub(result, result, Register::from_i16(4)),
Expand Down Expand Up @@ -253,19 +272,23 @@ fn reg_params_2_return() {
)
)";
let index = Register::from_i16(0);
let lhs = Register::from_i16(1);
let result = Register::from_i16(3);
let result2 = result.next();
let results = RegisterSpan::new(result).iter(2);
TranslationTest::from_wat(wasm)
.expect_func(
ExpectedFunc::new([
Instruction::branch_table(index, 4),
Instruction::copy2(RegisterSpan::new(result), lhs, lhs.next()),
Instruction::return_span(results),
Instruction::branch(BranchOffset::from(10)),
Instruction::branch(BranchOffset::from(7)),
Instruction::branch(BranchOffset::from(4)),
Instruction::branch(BranchOffset::from(1)),
Instruction::copy2(RegisterSpan::new(result), 1, 2),
Instruction::branch(BranchOffset::from(6)),
Instruction::copy2(RegisterSpan::new(result), 1, 2),
Instruction::branch(BranchOffset::from(6)),
Instruction::copy2(RegisterSpan::new(result), 1, 2),
Instruction::branch(BranchOffset::from(6)),
Instruction::return_reg2(1, 2),
Instruction::i32_add(result, result, result2),
Instruction::return_reg2(3, -1),
Instruction::i32_sub(result, result, result2),
Expand Down
20 changes: 1 addition & 19 deletions crates/wasmi/src/engine/translator/visit.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -534,25 +534,7 @@ impl<'a> VisitOperator<'a> for FuncTranslator {
.acquire_target(default_target)
.control_frame()
.branch_params(&engine);
let same_branch_params = self
.alloc
.buffer
.br_table_targets
.iter()
.copied()
.all(|target| {
match self.alloc.control_stack.acquire_target(target) {
AcquiredTarget::Return(_) => {
// Return instructions never need to copy anything and
// thus we can simply ignore them for this conditional.
true
}
AcquiredTarget::Branch(frame) => {
default_branch_params == frame.branch_params(&engine)
}
}
});
if default_branch_params.is_empty() || same_branch_params {
if default_branch_params.is_empty() {
// Case 1: No `br_target` target requires values to be copied around.
// Case 2: All `br_target` targets use the same branch parameter registers.
//
Expand Down

0 comments on commit a646d27

Please sign in to comment.