patternrustMinor
A simple register VM written in Rust
Viewed 0 times
rustregistersimplewritten
Problem
I'm teaching myself Rust, and to do this I've written a toy register based virtual machine. I hope the code is easy to follow - I just want to know if there are any mistakes I am making with the language philosophy or if there are syntax tricks I could be using. For example, the program counter
```
const NUMBER_OF_REGISTERS: usize = 3;
enum Instruction {
Halt,
Load { reg: usize, value: u16 },
Swap { reg1: usize, reg2: usize, reg3: usize },
Add { reg1: usize, reg2: usize, reg3: usize },
Branch { offset: usize }
}
fn main() {
let encoded_instructions = &[0x1110, 0x2100, 0x3010, 0x0];
cpu(encoded_instructions);
}
fn cpu(encoded_instructions: &[u16]) {
let mut pc = 0;
let mut reg_field = [0; NUMBER_OF_REGISTERS];
loop {
let encoded_instruction = fetch(pc, encoded_instructions);
let decoded_instruction = decode(encoded_instruction);
match decoded_instruction {
Some(Instruction::Load { reg, value })
=> load(reg, value, &mut reg_field, &mut pc),
Some(Instruction::Swap { reg1, reg2, reg3 })
=> swap(reg1, reg2, reg3, &mut reg_field, &mut pc),
Some(Instruction::Add { reg1, reg2, reg3 })
=> add(reg1, reg2, reg3, &mut reg_field, &mut pc),
Some(Instruction::Branch { offset })
=> branch(offset, &mut pc),
Some(Instruction::Halt)
=> { halt(®_field); break }
None => break,
}
}
}
fn fetch(pc: usize, instructions: &[u16]) -> u16 {
instructions[pc]
}
fn halt(register_field: &[u16]) {
println!("{:?}", register_field[0]);
}
fn load(register: usize, value: u16, register_field: &mut [u16], pc: &mut usize) {
register_field[register] = value;
*pc += 1;
}
pc and the register fields reg_field are mutable. I feel like this is the right thing to do in this case as it better reflects processor architecture. But of course, I might be wrong...```
const NUMBER_OF_REGISTERS: usize = 3;
enum Instruction {
Halt,
Load { reg: usize, value: u16 },
Swap { reg1: usize, reg2: usize, reg3: usize },
Add { reg1: usize, reg2: usize, reg3: usize },
Branch { offset: usize }
}
fn main() {
let encoded_instructions = &[0x1110, 0x2100, 0x3010, 0x0];
cpu(encoded_instructions);
}
fn cpu(encoded_instructions: &[u16]) {
let mut pc = 0;
let mut reg_field = [0; NUMBER_OF_REGISTERS];
loop {
let encoded_instruction = fetch(pc, encoded_instructions);
let decoded_instruction = decode(encoded_instruction);
match decoded_instruction {
Some(Instruction::Load { reg, value })
=> load(reg, value, &mut reg_field, &mut pc),
Some(Instruction::Swap { reg1, reg2, reg3 })
=> swap(reg1, reg2, reg3, &mut reg_field, &mut pc),
Some(Instruction::Add { reg1, reg2, reg3 })
=> add(reg1, reg2, reg3, &mut reg_field, &mut pc),
Some(Instruction::Branch { offset })
=> branch(offset, &mut pc),
Some(Instruction::Halt)
=> { halt(®_field); break }
None => break,
}
}
}
fn fetch(pc: usize, instructions: &[u16]) -> u16 {
instructions[pc]
}
fn halt(register_field: &[u16]) {
println!("{:?}", register_field[0]);
}
fn load(register: usize, value: u16, register_field: &mut [u16], pc: &mut usize) {
register_field[register] = value;
*pc += 1;
}
Solution
Your code was very readable and easy to follow. Most of my suggestions center around trying to showcase some more Rust idioms and features.
-
A collection of registers in normally called a register file.
-
Since the constant for the number of registers is only used to declare the register file, create a type alias and use that. This sets you up nicely to make it a standalone type in the future.
-
I'm glad to see names for registers in
-
You almost always want to derive
-
There's a distinct lack of types and methods, overall the code feels pretty C-like. I'd associate functions with types, making them methods. For example,
-
Returning an
-
When splitting match arms onto a different line from the pattern, use braces.
-
Consider inlining some of the instruction implementations into
-
In a processor, it's very typical to automatically increment the program counter every instruction. Branch offsets take that automatic increment into account. This is nice here because it removes
-
Create a type for sequences of instructions, I used
-
A collection of registers in normally called a register file.
-
Since the constant for the number of registers is only used to declare the register file, create a type alias and use that. This sets you up nicely to make it a standalone type in the future.
-
I'm glad to see names for registers in
Instruction, but maybe give them more meaningful names? Add is a great example - which registers are the inputs and which is the output?-
You almost always want to derive
Debug for a type. Copy and Clone are also very common. Implementing PartialEq will allow easier test writing.-
There's a distinct lack of types and methods, overall the code feels pretty C-like. I'd associate functions with types, making them methods. For example,
decode is the most obvious change to me.-
Returning an
Option from decode is iffy. Failing to decode an instruction doesn't seem like the absence of a value, it feels like a failure, which is normally reserved for Result.-
When splitting match arms onto a different line from the pattern, use braces.
-
Consider inlining some of the instruction implementations into
execute as many are only one line long anyway.-
In a processor, it's very typical to automatically increment the program counter every instruction. Branch offsets take that automatic increment into account. This is nice here because it removes
pc from all the non-branching methods. I just subtracted 1 when applying the jump, but it's probably better to fixup the offsets.-
Create a type for sequences of instructions, I used
Program. This will give you somewhere to hang fetch.type RegisterFile = [u16; 3];
#[derive(Debug, Copy, Clone)]
enum Instruction {
Halt,
Load { reg: usize, value: u16 },
Swap { reg1: usize, reg2: usize, reg3: usize },
Add { reg1: usize, reg2: usize, reg3: usize },
Branch { offset: usize }
}
impl Instruction {
fn decode(encoded_instruction: u16) -> Option {
let operator = encoded_instruction >> 12;
let reg1 = ((encoded_instruction >> 8) & 0xF) as usize;
let reg2 = ((encoded_instruction >> 4) & 0xF) as usize;
let reg3 = (encoded_instruction & 0xF) as usize;
let offset = (encoded_instruction & 0xFFF) as usize;
let value = encoded_instruction & 0xFF;
match operator {
0 => Some(Instruction::Halt),
1 => Some(Instruction::Load { reg: reg1, value: value }),
2 => Some(Instruction::Swap { reg1: reg1, reg2: reg2, reg3: reg3 }),
3 => Some(Instruction::Add { reg1: reg1, reg2: reg2, reg3: reg3 }),
4 => Some(Instruction::Branch { offset: offset }),
_ => None,
}
}
fn execute(&self, registers: &mut [u16], pc: &mut usize) -> bool {
match *self {
Instruction::Load { reg, value } => {
load(reg, value, registers);
},
Instruction::Swap { reg1, reg2, reg3 } => {
swap(reg1, reg2, reg3, registers);
},
Instruction::Add { reg1, reg2, reg3 } => {
add(reg1, reg2, reg3, registers);
},
Instruction::Branch { offset } => {
branch(offset, pc);
},
Instruction::Halt => {
halt(registers);
return false;
},
}
true
}
}
fn halt(register_file: &[u16]) {
println!("{:?}", register_file[0]);
}
fn load(register: usize, value: u16, register_file: &mut [u16]) {
register_file[register] = value;
}
fn swap(reg1: usize, reg2: usize, reg3: usize, register_file: &mut [u16]) {
register_file[reg3] = register_file[reg1];
register_file[reg1] = register_file[reg2];
register_file[reg2] = register_file[reg3];
}
fn add(reg1: usize, reg2: usize, reg3: usize, register_file: &mut [u16]) {
register_file[reg3] = register_file[reg1] + register_file[reg2];
}
fn branch(offset: usize, pc: &mut usize) {
*pc -= offset - 1;
}
struct Program {
instructions: &'a [u16],
}
impl Program {
fn fetch(&self, pc: usize) -> u16 {
self.instructions[pc]
}
}
fn cpu(program: Program) {
let mut pc = 0;
let mut registers = RegisterFile::default();
loop {
let encoded_instruction = program.fetch(pc);
let decoded_instruction = Instruction::decode(encoded_instruction);
match decoded_instruction {
Some(instr) => {
if !instr.execute(&mut registers, &mut pc) { break }
}
None => break,
}
pc += 1;
}
}
fn main() {
let encoded_instructions = Program { instructions: &[0x1110, 0x2100, 0x3010, 0x0] };
cpu(encoded_instructions);
}Code Snippets
type RegisterFile = [u16; 3];
#[derive(Debug, Copy, Clone)]
enum Instruction {
Halt,
Load { reg: usize, value: u16 },
Swap { reg1: usize, reg2: usize, reg3: usize },
Add { reg1: usize, reg2: usize, reg3: usize },
Branch { offset: usize }
}
impl Instruction {
fn decode(encoded_instruction: u16) -> Option<Self> {
let operator = encoded_instruction >> 12;
let reg1 = ((encoded_instruction >> 8) & 0xF) as usize;
let reg2 = ((encoded_instruction >> 4) & 0xF) as usize;
let reg3 = (encoded_instruction & 0xF) as usize;
let offset = (encoded_instruction & 0xFFF) as usize;
let value = encoded_instruction & 0xFF;
match operator {
0 => Some(Instruction::Halt),
1 => Some(Instruction::Load { reg: reg1, value: value }),
2 => Some(Instruction::Swap { reg1: reg1, reg2: reg2, reg3: reg3 }),
3 => Some(Instruction::Add { reg1: reg1, reg2: reg2, reg3: reg3 }),
4 => Some(Instruction::Branch { offset: offset }),
_ => None,
}
}
fn execute(&self, registers: &mut [u16], pc: &mut usize) -> bool {
match *self {
Instruction::Load { reg, value } => {
load(reg, value, registers);
},
Instruction::Swap { reg1, reg2, reg3 } => {
swap(reg1, reg2, reg3, registers);
},
Instruction::Add { reg1, reg2, reg3 } => {
add(reg1, reg2, reg3, registers);
},
Instruction::Branch { offset } => {
branch(offset, pc);
},
Instruction::Halt => {
halt(registers);
return false;
},
}
true
}
}
fn halt(register_file: &[u16]) {
println!("{:?}", register_file[0]);
}
fn load(register: usize, value: u16, register_file: &mut [u16]) {
register_file[register] = value;
}
fn swap(reg1: usize, reg2: usize, reg3: usize, register_file: &mut [u16]) {
register_file[reg3] = register_file[reg1];
register_file[reg1] = register_file[reg2];
register_file[reg2] = register_file[reg3];
}
fn add(reg1: usize, reg2: usize, reg3: usize, register_file: &mut [u16]) {
register_file[reg3] = register_file[reg1] + register_file[reg2];
}
fn branch(offset: usize, pc: &mut usize) {
*pc -= offset - 1;
}
struct Program<'a> {
instructions: &'a [u16],
}
impl<'a> Program<'a> {
fn fetch(&self, pc: usize) -> u16 {
self.instructions[pc]
}
}
fn cpu(program: Program) {
let mut pc = 0;
let mut registers = RegisterFile::default();
loop {
let encoded_instruction = program.fetch(pc);
let decoded_instruction = Instruction::decode(encoded_instruction);
match decoded_instruction {
Some(instr) => {
if !instr.execute(&mut registers, &mut pc) { break }
}
None => break,
}
pc += 1;
}
}
fnContext
StackExchange Code Review Q#129505, answer score: 6
Revisions (0)
No revisions yet.