From 0c2da2ae4699b26070be46721a29e00ce11fe044 Mon Sep 17 00:00:00 2001 From: Michel Heily Date: Fri, 14 Feb 2020 13:05:14 +0200 Subject: [PATCH] refactor: Refactor instruction decoding to panic instead of returning a Result flamegraph shows too much time is being spent 'unwraping' decoded instructions Former-commit-id: e89072e6cf648e83f87d8c01891f50474246a36b --- src/core/arm7tdmi/arm/mod.rs | 55 ++++++++++++++-------------------- src/core/arm7tdmi/cpu.rs | 33 +++----------------- src/core/arm7tdmi/mod.rs | 4 +-- src/core/arm7tdmi/thumb/mod.rs | 54 ++++++++++++++++----------------- src/disass.rs | 29 +++++------------- 5 files changed, 63 insertions(+), 112 deletions(-) diff --git a/src/core/arm7tdmi/arm/mod.rs b/src/core/arm7tdmi/arm/mod.rs index 9df752f..09aa2d6 100644 --- a/src/core/arm7tdmi/arm/mod.rs +++ b/src/core/arm7tdmi/arm/mod.rs @@ -110,65 +110,56 @@ pub struct ArmInstruction { impl InstructionDecoder for ArmInstruction { type IntType = u32; - fn decode(raw: u32, addr: Addr) -> Result { + fn decode(raw: u32, addr: Addr) -> Self { use ArmFormat::*; let cond_code = raw.bit_range(28..32) as u8; - let cond = match ArmCond::from_u8(cond_code) { - Some(cond) => Ok(cond), - None => Err(ArmDecodeError::new( - UndefinedConditionCode(cond_code as u32), - raw, - addr, - )), - }?; + let cond = ArmCond::from_u8(cond_code).expect("invalid arm condition"); let fmt = if (0x0fff_fff0 & raw) == 0x012f_ff10 { - Ok(BX) + BX } else if (0x0e00_0000 & raw) == 0x0a00_0000 { - Ok(B_BL) + B_BL } else if (0xe000_0010 & raw) == 0x0600_0000 { - Err(ArmDecodeError::new(UnknownInstructionFormat, raw, addr)) + panic!("unknown instruction {:#x} at @{:#x}", raw, addr); } else if (0x0fb0_0ff0 & raw) == 0x0100_0090 { - Ok(SWP) + SWP } else if (0x0fc0_00f0 & raw) == 0x0000_0090 { - Ok(MUL_MLA) + MUL_MLA } else if (0x0f80_00f0 & raw) == 0x0080_0090 { - Ok(MULL_MLAL) + MULL_MLAL } else if (0x0fbf_0fff & raw) == 0x010f_0000 { - Ok(MRS) + MRS } else if (0x0fbf_fff0 & raw) == 0x0129_f000 { - Ok(MSR_REG) + MSR_REG } else if (0x0dbf_f000 & raw) == 0x0128_f000 { - Ok(MSR_FLAGS) + MSR_FLAGS } else if (0x0c00_0000 & raw) == 0x0400_0000 { - Ok(LDR_STR) + LDR_STR } else if (0x0e40_0F90 & raw) == 0x0000_0090 { - Ok(LDR_STR_HS_REG) + LDR_STR_HS_REG } else if (0x0e40_0090 & raw) == 0x0040_0090 { - Ok(LDR_STR_HS_IMM) + LDR_STR_HS_IMM } else if (0x0e00_0000 & raw) == 0x0800_0000 { - Ok(LDM_STM) + LDM_STM } else if (0x0f00_0000 & raw) == 0x0f00_0000 { - Ok(SWI) + SWI } else if (0x0c00_0000 & raw) == 0x0000_0000 { - Ok(DP) + DP } else { - Err(ArmDecodeError::new(UnknownInstructionFormat, raw, addr)) - }?; + panic!("unknown arm instruction {:#x} at @{:#x}", raw, addr); + }; - Ok(ArmInstruction { + ArmInstruction { cond: cond, fmt: fmt, raw: raw, pc: addr, - }) + } } - fn decode_from_bytes(bytes: &[u8], addr: Addr) -> Result { + fn decode_from_bytes(bytes: &[u8], addr: Addr) -> Self { let mut rdr = std::io::Cursor::new(bytes); - let raw = rdr - .read_u32::() - .map_err(|e| InstructionDecoderError::IoError(e.kind()))?; + let raw = rdr.read_u32::().unwrap(); Self::decode(raw, addr) } diff --git a/src/core/arm7tdmi/cpu.rs b/src/core/arm7tdmi/cpu.rs index 8f8447b..035901a 100644 --- a/src/core/arm7tdmi/cpu.rs +++ b/src/core/arm7tdmi/cpu.rs @@ -293,25 +293,8 @@ impl Core { } } - // fn handle_exec_result(&mut self, sb: &mut SysBus, exec_result: CpuAction) { - // match self.cpsr.state() { - // CpuState::ARM => { - // match exec_result { - // CpuAction::AdvancePC => self.advance_arm(), - // CpuAction::FlushPipeline => self.reload_pipeline32(sb), - // } - // } - // CpuState::THUMB => { - // match exec_result { - // CpuAction::AdvancePC => self.advance_thumb(), - // CpuAction::FlushPipeline => self.reload_pipeline16(sb), - // } - // } - // } - // } - fn step_arm_exec(&mut self, insn: u32, sb: &mut SysBus) { - let decoded_arm = ArmInstruction::decode(insn, self.pc.wrapping_sub(8)).unwrap(); + let decoded_arm = ArmInstruction::decode(insn, self.pc.wrapping_sub(8)); #[cfg(feature = "debugger")] { self.gpr_previous = self.get_registers(); @@ -320,12 +303,12 @@ impl Core { let result = self.exec_arm(sb, decoded_arm); match result { CpuAction::AdvancePC => self.advance_arm(), - CpuAction::FlushPipeline => {}, + CpuAction::FlushPipeline => {} } } fn step_thumb_exec(&mut self, insn: u16, sb: &mut SysBus) { - let decoded_thumb = ThumbInstruction::decode(insn, self.pc.wrapping_sub(4)).unwrap(); + let decoded_thumb = ThumbInstruction::decode(insn, self.pc.wrapping_sub(4)); #[cfg(feature = "debugger")] { self.gpr_previous = self.get_registers(); @@ -334,7 +317,7 @@ impl Core { let result = self.exec_thumb(sb, decoded_thumb); match result { CpuAction::AdvancePC => self.advance_thumb(), - CpuAction::FlushPipeline => {}, + CpuAction::FlushPipeline => {} } } @@ -358,14 +341,6 @@ impl Core { self.advance_arm(); } - #[inline] - pub(super) fn reload_pipeline(&mut self, sb: &mut SysBus) { - match self.cpsr.state() { - CpuState::THUMB => self.reload_pipeline16(sb), - CpuState::ARM => self.reload_pipeline32(sb), - } - } - #[inline] pub(super) fn advance_thumb(&mut self) { self.pc = self.pc.wrapping_add(2) diff --git a/src/core/arm7tdmi/mod.rs b/src/core/arm7tdmi/mod.rs index b97098c..39d8a3b 100644 --- a/src/core/arm7tdmi/mod.rs +++ b/src/core/arm7tdmi/mod.rs @@ -75,9 +75,9 @@ impl From for InstructionDecoderError { pub trait InstructionDecoder: Sized { type IntType: Num; - fn decode(n: Self::IntType, addr: Addr) -> Result; + fn decode(n: Self::IntType, addr: Addr) -> Self; /// Helper functions for the Disassembler - fn decode_from_bytes(bytes: &[u8], addr: Addr) -> Result; + fn decode_from_bytes(bytes: &[u8], addr: Addr) -> Self; fn get_raw(&self) -> Self::IntType; } diff --git a/src/core/arm7tdmi/thumb/mod.rs b/src/core/arm7tdmi/thumb/mod.rs index b823363..e2303a4 100644 --- a/src/core/arm7tdmi/thumb/mod.rs +++ b/src/core/arm7tdmi/thumb/mod.rs @@ -87,63 +87,61 @@ pub struct ThumbInstruction { impl InstructionDecoder for ThumbInstruction { type IntType = u16; - fn decode(raw: u16, addr: Addr) -> Result { + fn decode(raw: u16, addr: Addr) -> Self { use self::ThumbFormat::*; let fmt = if raw & 0xf800 == 0x1800 { - Ok(AddSub) + AddSub } else if raw & 0xe000 == 0x0000 { - Ok(MoveShiftedReg) + MoveShiftedReg } else if raw & 0xe000 == 0x2000 { - Ok(DataProcessImm) + DataProcessImm } else if raw & 0xfc00 == 0x4000 { - Ok(AluOps) + AluOps } else if raw & 0xfc00 == 0x4400 { - Ok(HiRegOpOrBranchExchange) + HiRegOpOrBranchExchange } else if raw & 0xf800 == 0x4800 { - Ok(LdrPc) + LdrPc } else if raw & 0xf200 == 0x5000 { - Ok(LdrStrRegOffset) + LdrStrRegOffset } else if raw & 0xf200 == 0x5200 { - Ok(LdrStrSHB) + LdrStrSHB } else if raw & 0xe000 == 0x6000 { - Ok(LdrStrImmOffset) + LdrStrImmOffset } else if raw & 0xf000 == 0x8000 { - Ok(LdrStrHalfWord) + LdrStrHalfWord } else if raw & 0xf000 == 0x9000 { - Ok(LdrStrSp) + LdrStrSp } else if raw & 0xf000 == 0xa000 { - Ok(LoadAddress) + LoadAddress } else if raw & 0xff00 == 0xb000 { - Ok(AddSp) + AddSp } else if raw & 0xf600 == 0xb400 { - Ok(PushPop) + PushPop } else if raw & 0xf000 == 0xc000 { - Ok(LdmStm) + LdmStm } else if raw & 0xff00 == 0xdf00 { - Ok(Swi) + Swi } else if raw & 0xf000 == 0xd000 { - Ok(BranchConditional) + BranchConditional } else if raw & 0xf800 == 0xe000 { - Ok(Branch) + Branch } else if raw & 0xf000 == 0xf000 { - Ok(BranchLongWithLink) + BranchLongWithLink } else { - Err(ThumbDecodeError::new(UnknownInstructionFormat, raw, addr)) - }?; + panic!("unknown thumb instruction {:#x} at @{:#x}", raw, addr); + }; - Ok(ThumbInstruction { + ThumbInstruction { fmt: fmt, raw: raw, pc: addr, - }) + } } - fn decode_from_bytes(bytes: &[u8], addr: Addr) -> Result { + fn decode_from_bytes(bytes: &[u8], addr: Addr) -> Self { let mut rdr = std::io::Cursor::new(bytes); - let raw = rdr - .read_u16::() - .map_err(|e| InstructionDecoderError::IoError(e.kind()))?; + let raw = rdr.read_u16::().unwrap(); Self::decode(raw, addr) } diff --git a/src/disass.rs b/src/disass.rs index d41c8bb..abd50ab 100644 --- a/src/disass.rs +++ b/src/disass.rs @@ -42,27 +42,14 @@ where let mut line = String::new(); let addr = self.base + self.pos as Addr; - let decoded: Option = - match D::decode_from_bytes(&self.bytes[(self.pos as usize)..], addr) { - Ok(decoded) => { - self.pos += self.word_size; - Some(decoded) - } - Err(InstructionDecoderError::IoError(ErrorKind::UnexpectedEof)) => { - return None; - } - _ => { - self.pos += self.word_size; - None - } - }; - - match decoded { - Some(insn) => { - line.push_str(&format!("{:8x}:\t{:08x} \t{}", addr, insn.get_raw(), insn)) - } - _ => line.push_str(&format!("{:8x}:\t \t", addr)), - }; + let decoded: D = D::decode_from_bytes(&self.bytes[(self.pos as usize)..], addr); + self.pos += self.word_size; + line.push_str(&format!( + "{:8x}:\t{:08x} \t{}", + addr, + decoded.get_raw(), + decoded + )); Some((self.pos as Addr, line)) }