From 7509477a61c0cc5a6983a22e013b2404eeae7f15 Mon Sep 17 00:00:00 2001 From: Will Glynn Date: Wed, 10 Oct 2018 12:02:27 -0500 Subject: [PATCH] Hide instruction storage details (#129) * Hide Instructions implementation behind an iterator * Hide instruction encoding behind isa::Instructions::push() * Consistently use u32 for program counter storage * Refer to instructions by position rather than index --- src/isa.rs | 80 +++++++++++++++++++++++++++++++++++++++- src/runner.rs | 20 ++++++---- src/validation/func.rs | 55 +++++++-------------------- src/validation/tests.rs | 82 ++++++++++++++++++++++++----------------- 4 files changed, 153 insertions(+), 84 deletions(-) diff --git a/src/isa.rs b/src/isa.rs index 39aaa9e..646db4a 100644 --- a/src/isa.rs +++ b/src/isa.rs @@ -93,6 +93,22 @@ pub struct Target { pub drop_keep: DropKeep, } +/// A relocation entry that specifies. +#[derive(Debug)] +pub enum Reloc { + /// Patch the destination of the branch instruction (br, br_eqz, br_nez) + /// at the specified pc. + Br { + pc: u32, + }, + /// Patch the specified destination index inside of br_table instruction at + /// the specified pc. + BrTable { + pc: u32, + idx: usize, + }, +} + #[derive(Debug, Clone, PartialEq)] pub enum Instruction { /// Push a local variable or an argument from the specified depth. @@ -299,5 +315,67 @@ pub enum Instruction { #[derive(Debug, Clone)] pub struct Instructions { - pub code: Vec, + vec: Vec, +} + +impl Instructions { + pub fn with_capacity(capacity: usize) -> Self { + Instructions { + vec: Vec::with_capacity(capacity), + } + } + + pub fn current_pc(&self) -> u32 { + self.vec.len() as u32 + } + + pub fn push(&mut self, instruction: Instruction) { + self.vec.push(instruction); + } + + pub fn patch_relocation(&mut self, reloc: Reloc, dst_pc: u32) { + match reloc { + Reloc::Br { pc } => match self.vec[pc as usize] { + Instruction::Br(ref mut target) + | Instruction::BrIfEqz(ref mut target) + | Instruction::BrIfNez(ref mut target) => target.dst_pc = dst_pc, + _ => panic!("branch relocation points to a non-branch instruction"), + }, + Reloc::BrTable { pc, idx } => match self.vec[pc as usize] { + Instruction::BrTable(ref mut targets) => targets[idx].dst_pc = dst_pc, + _ => panic!("brtable relocation points to not brtable instruction"), + } + } + } + + pub fn iterate_from(&self, position: u32) -> InstructionIter { + InstructionIter{ + instructions: &self.vec, + position, + } + } +} + +pub struct InstructionIter<'a> { + instructions: &'a [Instruction], + position: u32, +} + +impl<'a> InstructionIter<'a> { + #[inline] + pub fn position(&self) -> u32 { + self.position + } +} + +impl<'a> Iterator for InstructionIter<'a> { + type Item = &'a Instruction; + + #[inline] + fn next(&mut self) -> Option<::Item> { + self.instructions.get(self.position as usize).map(|instruction| { + self.position += 1; + instruction + }) + } } diff --git a/src/runner.rs b/src/runner.rs index 5a9ec84..8fa1a6d 100644 --- a/src/runner.rs +++ b/src/runner.rs @@ -175,7 +175,7 @@ impl Interpreter { let function_return = self.do_run_function( &mut function_context, - &function_body.code.code, + &function_body.code, ).map_err(Trap::new)?; match function_return { @@ -229,18 +229,24 @@ impl Interpreter { } } - fn do_run_function(&mut self, function_context: &mut FunctionContext, instructions: &[isa::Instruction]) -> Result { + fn do_run_function(&mut self, function_context: &mut FunctionContext, instructions: &isa::Instructions) + -> Result + { + let mut iter = instructions.iterate_from(function_context.position); loop { - let instruction = &instructions[function_context.position]; + let instruction = iter.next().expect("instruction"); match self.run_instruction(function_context, instruction)? { - InstructionOutcome::RunNextInstruction => function_context.position += 1, + InstructionOutcome::RunNextInstruction => { + function_context.position = iter.position(); + }, InstructionOutcome::Branch(target) => { - function_context.position = target.dst_pc as usize; + function_context.position = target.dst_pc; + iter = instructions.iterate_from(function_context.position); self.value_stack.drop_keep(target.drop_keep); }, InstructionOutcome::ExecuteCall(func_ref) => { - function_context.position += 1; + function_context.position = iter.position(); return Ok(RunResult::NestedCall(func_ref)); }, InstructionOutcome::Return(drop_keep) => { @@ -1077,7 +1083,7 @@ struct FunctionContext { pub module: ModuleRef, pub memory: Option, /// Current instruction position. - pub position: usize, + pub position: u32, } impl FunctionContext { diff --git a/src/validation/func.rs b/src/validation/func.rs index 7107e05..f7a975e 100644 --- a/src/validation/func.rs +++ b/src/validation/func.rs @@ -1406,9 +1406,7 @@ impl<'a> FunctionValidationContext<'a> { } fn into_code(self) -> isa::Instructions { - isa::Instructions { - code: self.sink.into_inner(), - } + self.sink.into_inner() } } @@ -1628,22 +1626,6 @@ struct Target { drop_keep: isa::DropKeep, } -/// A relocation entry that specifies. -#[derive(Debug)] -enum Reloc { - /// Patch the destination of the branch instruction (br, br_eqz, br_nez) - /// at the specified pc. - Br { - pc: u32, - }, - /// Patch the specified destination index inside of br_table instruction at - /// the specified pc. - BrTable { - pc: u32, - idx: usize, - }, -} - /// Identifier of a label. #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] struct LabelId(usize); @@ -1655,23 +1637,23 @@ enum Label { } struct Sink { - ins: Vec, - labels: Vec<(Label, Vec)>, + ins: isa::Instructions, + labels: Vec<(Label, Vec)>, } impl Sink { fn with_instruction_capacity(capacity: usize) -> Sink { Sink { - ins: Vec::with_capacity(capacity), + ins: isa::Instructions::with_capacity(capacity), labels: Vec::new(), } } fn cur_pc(&self) -> u32 { - self.ins.len() as u32 + self.ins.current_pc() } - fn pc_or_placeholder Reloc>(&mut self, label: LabelId, reloc_creator: F) -> u32 { + fn pc_or_placeholder isa::Reloc>(&mut self, label: LabelId, reloc_creator: F) -> u32 { match self.labels[label.0] { (Label::Resolved(dst_pc), _) => dst_pc, (Label::NotResolved, ref mut unresolved) => { @@ -1692,7 +1674,7 @@ impl Sink { drop_keep, } = target; let pc = self.cur_pc(); - let dst_pc = self.pc_or_placeholder(label, || Reloc::Br { pc }); + let dst_pc = self.pc_or_placeholder(label, || isa::Reloc::Br { pc }); self.ins.push(isa::Instruction::Br(isa::Target { dst_pc, drop_keep: drop_keep.into(), @@ -1705,7 +1687,7 @@ impl Sink { drop_keep, } = target; let pc = self.cur_pc(); - let dst_pc = self.pc_or_placeholder(label, || Reloc::Br { pc }); + let dst_pc = self.pc_or_placeholder(label, || isa::Reloc::Br { pc }); self.ins.push(isa::Instruction::BrIfEqz(isa::Target { dst_pc, drop_keep: drop_keep.into(), @@ -1718,7 +1700,7 @@ impl Sink { drop_keep, } = target; let pc = self.cur_pc(); - let dst_pc = self.pc_or_placeholder(label, || Reloc::Br { pc }); + let dst_pc = self.pc_or_placeholder(label, || isa::Reloc::Br { pc }); self.ins.push(isa::Instruction::BrIfNez(isa::Target { dst_pc, drop_keep: drop_keep.into(), @@ -1731,7 +1713,7 @@ impl Sink { let pc = self.cur_pc(); let mut isa_targets = Vec::new(); for (idx, &Target { label, drop_keep }) in targets.iter().chain(iter::once(&default)).enumerate() { - let dst_pc = self.pc_or_placeholder(label, || Reloc::BrTable { pc, idx }); + let dst_pc = self.pc_or_placeholder(label, || isa::Reloc::BrTable { pc, idx }); isa_targets.push( isa::Target { dst_pc, @@ -1768,26 +1750,15 @@ impl Sink { // particular label. let unresolved_rels = mem::replace(&mut self.labels[label.0].1, Vec::new()); for reloc in unresolved_rels { - match reloc { - Reloc::Br { pc } => match self.ins[pc as usize] { - isa::Instruction::Br(ref mut target) - | isa::Instruction::BrIfEqz(ref mut target) - | isa::Instruction::BrIfNez(ref mut target) => target.dst_pc = dst_pc, - _ => panic!("branch relocation points to a non-branch instruction"), - }, - Reloc::BrTable { pc, idx } => match self.ins[pc as usize] { - isa::Instruction::BrTable(ref mut targets) => targets[idx].dst_pc = dst_pc, - _ => panic!("brtable relocation points to not brtable instruction"), - } - } + self.ins.patch_relocation(reloc, dst_pc); } // Mark this label as resolved. self.labels[label.0] = (Label::Resolved(dst_pc), Vec::new()); } - /// Consume this Sink and returns isa::Instruction. - fn into_inner(self) -> Vec { + /// Consume this Sink and returns isa::Instructions. + fn into_inner(self) -> isa::Instructions { // At this moment all labels should be resolved. assert!({ self.labels.iter().all(|(state, unresolved)| diff --git a/src/validation/tests.rs b/src/validation/tests.rs index 7feb726..9a8829e 100644 --- a/src/validation/tests.rs +++ b/src/validation/tests.rs @@ -310,15 +310,29 @@ fn validate(wat: &str) -> ValidatedModule { validated_module } -fn compile(wat: &str) -> Vec { +fn compile(wat: &str) -> (Vec, Vec) { let validated_module = validate(wat); let code = &validated_module.code_map[0]; - code.code.clone() + + let mut instructions = Vec::new(); + let mut pcs = Vec::new(); + let mut iter = code.iterate_from(0); + loop { + let pc = iter.position(); + if let Some(instruction) = iter.next() { + instructions.push(instruction.clone()); + pcs.push(pc); + } else { + break + } + } + + (instructions, pcs) } #[test] fn implicit_return_no_value() { - let code = compile(r#" + let (code, _) = compile(r#" (module (func (export "call") ) @@ -337,7 +351,7 @@ fn implicit_return_no_value() { #[test] fn implicit_return_with_value() { - let code = compile(r#" + let (code, _) = compile(r#" (module (func (export "call") (result i32) i32.const 0 @@ -358,7 +372,7 @@ fn implicit_return_with_value() { #[test] fn implicit_return_param() { - let code = compile(r#" + let (code, _) = compile(r#" (module (func (export "call") (param i32) ) @@ -377,7 +391,7 @@ fn implicit_return_param() { #[test] fn get_local() { - let code = compile(r#" + let (code, _) = compile(r#" (module (func (export "call") (param i32) (result i32) get_local 0 @@ -398,7 +412,7 @@ fn get_local() { #[test] fn explicit_return() { - let code = compile(r#" + let (code, _) = compile(r#" (module (func (export "call") (param i32) (result i32) get_local 0 @@ -424,7 +438,7 @@ fn explicit_return() { #[test] fn add_params() { - let code = compile(r#" + let (code, _) = compile(r#" (module (func (export "call") (param i32) (param i32) (result i32) get_local 0 @@ -454,7 +468,7 @@ fn add_params() { #[test] fn drop_locals() { - let code = compile(r#" + let (code, _) = compile(r#" (module (func (export "call") (param i32) (local i32) @@ -478,7 +492,7 @@ fn drop_locals() { #[test] fn if_without_else() { - let code = compile(r#" + let (code, pcs) = compile(r#" (module (func (export "call") (param i32) (result i32) i32.const 1 @@ -495,7 +509,7 @@ fn if_without_else() { vec![ isa::Instruction::I32Const(1), isa::Instruction::BrIfEqz(isa::Target { - dst_pc: 4, + dst_pc: pcs[4], drop_keep: isa::DropKeep { drop: 0, keep: isa::Keep::None, @@ -517,7 +531,7 @@ fn if_without_else() { #[test] fn if_else() { - let code = compile(r#" + let (code, pcs) = compile(r#" (module (func (export "call") (local i32) @@ -537,7 +551,7 @@ fn if_else() { vec![ isa::Instruction::I32Const(1), isa::Instruction::BrIfEqz(isa::Target { - dst_pc: 5, + dst_pc: pcs[5], drop_keep: isa::DropKeep { drop: 0, keep: isa::Keep::None, @@ -546,7 +560,7 @@ fn if_else() { isa::Instruction::I32Const(2), isa::Instruction::SetLocal(1), isa::Instruction::Br(isa::Target { - dst_pc: 7, + dst_pc: pcs[7], drop_keep: isa::DropKeep { drop: 0, keep: isa::Keep::None, @@ -564,7 +578,7 @@ fn if_else() { #[test] fn if_else_returns_result() { - let code = compile(r#" + let (code, pcs) = compile(r#" (module (func (export "call") i32.const 1 @@ -582,7 +596,7 @@ fn if_else_returns_result() { vec![ isa::Instruction::I32Const(1), isa::Instruction::BrIfEqz(isa::Target { - dst_pc: 4, + dst_pc: pcs[4], drop_keep: isa::DropKeep { drop: 0, keep: isa::Keep::None, @@ -590,7 +604,7 @@ fn if_else_returns_result() { }), isa::Instruction::I32Const(2), isa::Instruction::Br(isa::Target { - dst_pc: 5, + dst_pc: pcs[5], drop_keep: isa::DropKeep { drop: 0, keep: isa::Keep::None, @@ -608,7 +622,7 @@ fn if_else_returns_result() { #[test] fn if_else_branch_from_true_branch() { - let code = compile(r#" + let (code, pcs) = compile(r#" (module (func (export "call") i32.const 1 @@ -630,7 +644,7 @@ fn if_else_branch_from_true_branch() { vec![ isa::Instruction::I32Const(1), isa::Instruction::BrIfEqz(isa::Target { - dst_pc: 8, + dst_pc: pcs[8], drop_keep: isa::DropKeep { drop: 0, keep: isa::Keep::None, @@ -639,7 +653,7 @@ fn if_else_branch_from_true_branch() { isa::Instruction::I32Const(1), isa::Instruction::I32Const(1), isa::Instruction::BrIfNez(isa::Target { - dst_pc: 9, + dst_pc: pcs[9], drop_keep: isa::DropKeep { drop: 0, keep: isa::Keep::Single, @@ -648,7 +662,7 @@ fn if_else_branch_from_true_branch() { isa::Instruction::Drop, isa::Instruction::I32Const(2), isa::Instruction::Br(isa::Target { - dst_pc: 9, + dst_pc: pcs[9], drop_keep: isa::DropKeep { drop: 0, keep: isa::Keep::None, @@ -666,7 +680,7 @@ fn if_else_branch_from_true_branch() { #[test] fn if_else_branch_from_false_branch() { - let code = compile(r#" + let (code, pcs) = compile(r#" (module (func (export "call") i32.const 1 @@ -688,7 +702,7 @@ fn if_else_branch_from_false_branch() { vec![ isa::Instruction::I32Const(1), isa::Instruction::BrIfEqz(isa::Target { - dst_pc: 4, + dst_pc: pcs[4], drop_keep: isa::DropKeep { drop: 0, keep: isa::Keep::None, @@ -696,7 +710,7 @@ fn if_else_branch_from_false_branch() { }), isa::Instruction::I32Const(1), isa::Instruction::Br(isa::Target { - dst_pc: 9, + dst_pc: pcs[9], drop_keep: isa::DropKeep { drop: 0, keep: isa::Keep::None, @@ -705,7 +719,7 @@ fn if_else_branch_from_false_branch() { isa::Instruction::I32Const(2), isa::Instruction::I32Const(1), isa::Instruction::BrIfNez(isa::Target { - dst_pc: 9, + dst_pc: pcs[9], drop_keep: isa::DropKeep { drop: 0, keep: isa::Keep::Single, @@ -724,7 +738,7 @@ fn if_else_branch_from_false_branch() { #[test] fn loop_() { - let code = compile(r#" + let (code, _) = compile(r#" (module (func (export "call") loop (result i32) @@ -759,7 +773,7 @@ fn loop_() { #[test] fn loop_empty() { - let code = compile(r#" + let (code, _) = compile(r#" (module (func (export "call") loop @@ -780,7 +794,7 @@ fn loop_empty() { #[test] fn brtable() { - let code = compile(r#" + let (code, pcs) = compile(r#" (module (func (export "call") block $1 @@ -806,7 +820,7 @@ fn brtable() { }, }, isa::Target { - dst_pc: 2, + dst_pc: pcs[2], drop_keep: isa::DropKeep { drop: 0, keep: isa::Keep::None, @@ -824,7 +838,7 @@ fn brtable() { #[test] fn brtable_returns_result() { - let code = compile(r#" + let (code, pcs) = compile(r#" (module (func (export "call") block $1 (result i32) @@ -847,14 +861,14 @@ fn brtable_returns_result() { isa::Instruction::BrTable( vec![ isa::Target { - dst_pc: 3, + dst_pc: pcs[3], drop_keep: isa::DropKeep { drop: 0, keep: isa::Keep::Single, }, }, isa::Target { - dst_pc: 4, + dst_pc: pcs[4], drop_keep: isa::DropKeep { keep: isa::Keep::Single, drop: 0, @@ -874,7 +888,7 @@ fn brtable_returns_result() { #[test] fn wabt_example() { - let code = compile(r#" + let (code, pcs) = compile(r#" (module (func (export "call") (param i32) (result i32) block $exit @@ -893,7 +907,7 @@ fn wabt_example() { vec![ isa::Instruction::GetLocal(1), isa::Instruction::BrIfNez(isa::Target { - dst_pc: 4, + dst_pc: pcs[4], drop_keep: isa::DropKeep { drop: 0, keep: isa::Keep::None,