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

@@ -110,65 +110,56 @@ pub struct ArmInstruction {
 impl InstructionDecoder for ArmInstruction {
     type IntType = u32;
 
-    fn decode(raw: u32, addr: Addr) -> Result<Self, InstructionDecoderError> {
+    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) {
+        let cond = ArmCond::from_u8(cond_code).expect("invalid arm condition");
-            Some(cond) => Ok(cond),
-            None => Err(ArmDecodeError::new(
-                UndefinedConditionCode(cond_code as u32),
-                raw,
-                addr,
-            )),
-        }?;
 
         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<Self, InstructionDecoderError> {
+    fn decode_from_bytes(bytes: &[u8], addr: Addr) -> Self {
         let mut rdr = std::io::Cursor::new(bytes);
-        let raw = rdr
+        let raw = rdr.read_u32::<LittleEndian>().unwrap();
-            .read_u32::<LittleEndian>()
-            .map_err(|e| InstructionDecoderError::IoError(e.kind()))?;
         Self::decode(raw, addr)
     }
 

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)

src/core/arm7tdmi/mod.rs

@@ -75,9 +75,9 @@ impl From<ThumbDecodeError> for InstructionDecoderError {
 pub trait InstructionDecoder: Sized {
     type IntType: Num;
 
-    fn decode(n: Self::IntType, addr: Addr) -> Result<Self, InstructionDecoderError>;
+    fn decode(n: Self::IntType, addr: Addr) -> Self;
     /// Helper functions for the Disassembler
-    fn decode_from_bytes(bytes: &[u8], addr: Addr) -> Result<Self, InstructionDecoderError>;
+    fn decode_from_bytes(bytes: &[u8], addr: Addr) -> Self;
     fn get_raw(&self) -> Self::IntType;
 }
 

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<Self, InstructionDecoderError> {
+    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<Self, InstructionDecoderError> {
+    fn decode_from_bytes(bytes: &[u8], addr: Addr) -> Self {
         let mut rdr = std::io::Cursor::new(bytes);
-        let raw = rdr
+        let raw = rdr.read_u16::<LittleEndian>().unwrap();
-            .read_u16::<LittleEndian>()
-            .map_err(|e| InstructionDecoderError::IoError(e.kind()))?;
         Self::decode(raw, addr)
     }
 

src/disass.rs

@@ -42,27 +42,14 @@ where
         let mut line = String::new();
 
         let addr = self.base + self.pos as Addr;
-        let decoded: Option<D> =
+        let decoded: D = D::decode_from_bytes(&self.bytes[(self.pos as usize)..], addr);
-            match D::decode_from_bytes(&self.bytes[(self.pos as usize)..], addr) {
-                Ok(decoded) => {
         self.pos += self.word_size;
-                    Some(decoded)
+        line.push_str(&format!(
-                }
+            "{:8x}:\t{:08x} \t{}",
-                Err(InstructionDecoderError::IoError(ErrorKind::UnexpectedEof)) => {
+            addr,
-                    return None;
+            decoded.get_raw(),
-                }
+            decoded
-                _ => {
+        ));
-                    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<UNDEFINED>", addr)),
-        };
 
         Some((self.pos as Addr, line))
     }