armwrestler-fix: Refactor barrel shifter and fix ALU carry flag, hopefully for good.

Passing most of armwrestler ALU tests (still have bugs in UMULL and
SMULL)


Former-commit-id: 3c57ca9b5360b5c9bba74b00a5bede5a8cc496af

src/core/arm7tdmi/alu.rs

@@ -57,33 +57,24 @@ pub enum BarrelShiftOpCode {
 }
 
 #[derive(Debug, PartialEq)]
-pub enum ShiftedRegister {
+pub enum ShiftRegisterBy {
-    ByAmount(u32, BarrelShiftOpCode),
+    ByAmount(u32),
-    ByRegister(usize, BarrelShiftOpCode),
+    ByRegister(usize),
 }
 
-impl From<u32> for ShiftedRegister {
+#[derive(Debug, PartialEq)]
-    fn from(v: u32) -> ShiftedRegister {
+pub struct ShiftedRegister {
-        let typ = BarrelShiftOpCode::from_u8(v.bit_range(5..7) as u8).unwrap();
+    pub reg: usize,
-        if v.bit(4) {
+    pub shift_by: ShiftRegisterBy,
-            let rs = v.bit_range(8..12) as usize;
+    pub bs_op: BarrelShiftOpCode,
-            ShiftedRegister::ByRegister(rs, typ)
+    pub added: Option<bool>,
-        } else {
-            let amount = v.bit_range(7..12) as u32;
-            ShiftedRegister::ByAmount(amount, typ)
-        }
-    }
 }
 
 #[derive(Debug, PartialEq)]
 pub enum BarrelShifterValue {
     ImmediateValue(i32),
     RotatedImmediate(u32, u32),
-    ShiftedRegister {
+    ShiftedRegister(ShiftedRegister),
-        reg: usize,
-        shift: ShiftedRegister,
-        added: Option<bool>,
-    },
 }
 
 impl BarrelShifterValue {
@@ -97,15 +88,116 @@ impl BarrelShifterValue {
 }
 
 impl Core {
+    pub fn lsl(&mut self, val: u32, amount: u32, carry_in: bool) -> u32 {
+        match amount {
+            0 => {
+                self.bs_carry_out = carry_in;
+                val
+            }
+            x if x < 32 => {
+                self.bs_carry_out = val.wrapping_shr(32 - x) & 1 == 1;
+                val << x
+            }
+            32 => {
+                self.bs_carry_out = val & 1 == 1;
+                0
+            }
+            _ => {
+                self.bs_carry_out = false;
+                0
+            }
+        }
+    }
+
+    pub fn lsr(&mut self, val: u32, amount: u32, carry_in: bool, immediate: bool) -> u32 {
+        match amount {
+            0 | 32 => {
+                if immediate {
+                    self.bs_carry_out = val.bit(31);
+                    0
+                } else {
+                    val
+                }
+            }
+            x if x < 32 => {
+                self.bs_carry_out = val >> (amount - 1) & 1 == 1;
+                (val as u32) >> amount
+            }
+            _ => {
+                self.bs_carry_out = false;
+                0
+            }
+        }
+    }
+
+    pub fn asr(&mut self, val: u32, amount: u32, carry_in: bool, immediate: bool) -> u32 {
+        match amount {
+            0 => {
+                if immediate {
+                    let bit31 = (val as i32 as u32).bit(31);
+                    self.bs_carry_out = bit31;
+                    if bit31 {
+                        0xffffffff
+                    } else {
+                        0
+                    }
+                } else {
+                    val
+                }
+            }
+            x if x < 32 => {
+                self.bs_carry_out = val.wrapping_shr(amount - 1) & 1 == 1;
+                (val as i32).wrapping_shr(amount) as u32
+            }
+            _ => {
+                let bit31 = val.bit(31);
+                self.bs_carry_out = bit31;
+                if bit31 {
+                    0xffffffff
+                } else {
+                    0
+                }
+            }
+        }
+    }
+
+    pub fn rrx(&mut self, val: u32, carry_in: bool) -> u32 {
+        let old_c = carry_in as i32;
+        self.bs_carry_out = val & 0b1 != 0;
+        (((val as u32) >> 1) as i32 | (old_c << 31)) as u32
+    }
+
+    pub fn ror(&mut self, val: u32, amount: u32, carry_in: bool, immediate: bool, rrx: bool) -> u32 {
+        match amount {
+            0 => {
+                if immediate & rrx{
+                    self.rrx(val, carry_in)
+                } else {
+                    val
+                }
+            }
+            _ => {
+                let amount = amount % 32;
+                let val = if amount != 0 {
+                    val.rotate_right(amount)
+                } else {
+                    val
+                };
+                self.bs_carry_out = (val as u32).bit(31);
+                val
+            }
+        }
+    }
+
     /// Performs a generic barrel shifter operation
-    pub fn barrel_shift(
+    pub fn barrel_shift_op(
         &mut self,
-        val: i32,
-        amount: u32,
         shift: BarrelShiftOpCode,
-        carry: &mut bool,
+        val: u32,
+        amount: u32,
+        carry_in: bool,
         immediate: bool,
-    ) -> i32 {
+    ) -> u32 {
         //
         // From GBATEK:
         // Zero Shift Amount (Shift Register by Immediate, with Immediate=0)
@@ -126,108 +218,28 @@ impl Core {
         //   in the range 1 to 32 and see above.
         //
         match shift {
-            BarrelShiftOpCode::LSL => match amount {
+            BarrelShiftOpCode::LSL => self.lsl(val, amount, carry_in),
-                0 => val,
+            BarrelShiftOpCode::LSR => self.lsr(val, amount, carry_in, immediate),
-                x if x < 32 => {
+            BarrelShiftOpCode::ASR => self.asr(val, amount, carry_in, immediate),
-                    *carry = val.wrapping_shr(32 - x) & 1 == 1;
+            BarrelShiftOpCode::ROR => self.ror(val, amount, carry_in, immediate, true),
-                    val << x
-                }
-                32 => {
-                    *carry = val & 1 == 1;
-                    0
-                }
-                _ => {
-                    *carry = false;
-                    0
-                }
-            },
-            BarrelShiftOpCode::LSR => match amount {
-                0 | 32 => {
-                    if immediate {
-                        *carry = (val as u32).bit(31);
-                        0
-                    } else {
-                        val
-                    }
-                }
-                x if x < 32 => {
-                    *carry = val >> (amount - 1) & 1 == 1;
-                    ((val as u32) >> amount) as i32
-                }
-                _ => {
-                    *carry = false;
-                    0
-                }
-            },
-            BarrelShiftOpCode::ASR => match amount {
-                0 => {
-                    if immediate {
-                        let bit31 = (val as u32).bit(31);
-                        *carry = bit31;
-                        if bit31 {
-                            -1
-                        } else {
-                            0
-                        }
-                    } else {
-                        val
-                    }
-                }
-                x if x < 32 => {
-                    *carry = val.wrapping_shr(amount - 1) & 1 == 1;
-                    val.wrapping_shr(amount)
-                }
-                _ => {
-                    let bit31 = (val as u32).bit(31);
-                    *carry = bit31;
-                    if bit31 {
-                        -1
-                    } else {
-                        0
-                    }
-                }
-            },
-            BarrelShiftOpCode::ROR => {
-                match amount {
-                    0 => {
-                        if immediate {
-                            /* RRX */
-                            let old_c = self.cpsr.C() as i32;
-                            *carry = val & 0b1 != 0;
-                            ((val as u32) >> 1) as i32 | (old_c << 31)
-                        } else {
-                            val
-                        }
-                    }
-                    _ => {
-                        let amount = amount % 32;
-                        let val = if amount != 0 {
-                            val.rotate_right(amount)
-                        } else {
-                            val
-                        };
-                        *carry = (val as u32).bit(31);
-                        val
-                    }
-                }
-            }
         }
     }
 
-    pub fn register_shift(&mut self, reg: usize, shift: ShiftedRegister) -> CpuResult<i32> {
+    pub fn register_shift(&mut self, shift: ShiftedRegister) -> CpuResult<u32> {
-        let val = self.get_reg(reg) as i32;
+        let mut val = self.get_reg(shift.reg);
-        let mut carry = self.cpsr.C();
+        let carry = self.cpsr.C();
-        match shift {
+        match shift.shift_by {
-            ShiftedRegister::ByAmount(amount, shift) => {
+            ShiftRegisterBy::ByAmount(amount) => {
-                let result = self.barrel_shift(val, amount, shift, &mut carry, true);
+                let result = self.barrel_shift_op(shift.bs_op, val, amount, carry, true);
-                self.cpsr.set_C(carry);
                 Ok(result)
             }
-            ShiftedRegister::ByRegister(reg, shift) => {
+            ShiftRegisterBy::ByRegister(rs) => {
-                if reg != REG_PC {
+                if shift.reg == REG_PC {
-                    let result =
+                    val = val + 4; // PC prefetching
-                        self.barrel_shift(val, self.get_reg(reg) & 0xff, shift, &mut carry, false);
+                }
-                    self.cpsr.set_C(carry);
+                if rs != REG_PC {
+                    let amount = self.get_reg(rs) & 0xff;
+                    let result = self.barrel_shift_op(shift.bs_op, val, amount, carry, false);
                     Ok(result)
                 } else {
                     Err(CpuError::IllegalInstruction)
@@ -240,12 +252,9 @@ impl Core {
         // TODO decide if error handling or panic here
         match sval {
             BarrelShifterValue::ImmediateValue(offset) => offset,
-            BarrelShifterValue::ShiftedRegister {
+            BarrelShifterValue::ShiftedRegister(shifted_reg) => {
-                reg,
+                let added = shifted_reg.added.unwrap_or(true);
-                shift,
+                let abs = self.register_shift(shifted_reg).unwrap() as i32;
-                added: Some(added),
-            } => {
-                let abs = self.register_shift(reg, shift).unwrap();
                 if added {
                     abs
                 } else {
@@ -273,18 +282,32 @@ impl Core {
     }
 
     #[allow(non_snake_case)]
-    pub fn alu(
+    pub fn alu(&mut self, opcode: AluOpCode, op1: i32, op2: i32) -> i32 {
-        &mut self,
-        opcode: AluOpCode,
-        op1: i32,
-        op2: i32,
-        set_cond_flags: bool,
-    ) -> Option<i32> {
         use AluOpCode::*;
-
         let C = self.cpsr.C() as i32;
 
-        let mut carry = self.cpsr.C();
+        match opcode {
+            AND => op1 & op2,
+            EOR => op1 ^ op2,
+            SUB => op1.wrapping_sub(op2),
+            RSB => op2.wrapping_sub(op1),
+            ADD => op1.wrapping_add(op2),
+            ADC => op1.wrapping_add(op2).wrapping_add(C),
+            SBC => op1.wrapping_sub(op2).wrapping_sub(1 - C),
+            RSC => op2.wrapping_sub(op1).wrapping_sub(1 - C),
+            ORR => op1 | op2,
+            MOV => op2,
+            BIC => op1 & (!op2),
+            MVN => !op2,
+            _ => panic!("{} should be a PSR transfer", opcode),
+        }
+    }
+
+    #[allow(non_snake_case)]
+    pub fn alu_flags(&mut self, opcode: AluOpCode, op1: i32, op2: i32) -> Option<i32> {
+        use AluOpCode::*;
+        let mut carry = self.bs_carry_out;
+        let C = self.cpsr.C() as i32;
         let mut overflow = self.cpsr.V();
 
         let result = match opcode {
@@ -302,18 +325,17 @@ impl Core {
             MVN => !op2,
         };
 
-        if set_cond_flags {
+        self.cpsr.set_N(result < 0);
-            self.cpsr.set_N(result < 0);
+        self.cpsr.set_Z(result == 0);
-            self.cpsr.set_Z(result == 0);
+        self.cpsr.set_C(carry);
-            self.cpsr.set_C(carry);
+        if opcode.is_arithmetic() {
-            if opcode.is_arithmetic() {
+            self.cpsr.set_V(overflow);
-                self.cpsr.set_V(overflow);
-            }
         }
 
-        match opcode {
+        if opcode.is_setting_flags() {
-            TST | TEQ | CMP | CMN => None,
+            None
-            _ => Some(result),
+        } else {
+            Some(result)
         }
     }
 }

src/core/arm7tdmi/arm/display.rs

@@ -3,9 +3,9 @@ use std::fmt;
 use crate::bit::BitIndex;
 
 use super::{AluOpCode, ArmCond, ArmFormat, ArmHalfwordTransferType, ArmInstruction};
-use crate::core::arm7tdmi::{
+use crate::core::arm7tdmi::alu::*;
-    psr::RegPSR, reg_string, Addr, BarrelShiftOpCode, BarrelShifterValue, ShiftedRegister, REG_PC,
+use crate::core::arm7tdmi::psr::RegPSR;
-};
+use crate::core::arm7tdmi::*;
 
 impl fmt::Display for ArmCond {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
@@ -77,26 +77,30 @@ impl fmt::Display for ArmHalfwordTransferType {
     }
 }
 
-fn is_shift(shift: &ShiftedRegister) -> bool {
+fn is_lsl0(shift: &ShiftedRegister) -> bool {
-    if let ShiftedRegister::ByAmount(val, typ) = shift {
+    if let ShiftRegisterBy::ByAmount(val) = shift.shift_by {
-        return !(*val == 0 && *typ == BarrelShiftOpCode::LSL);
+        return !(val == 0 && shift.bs_op == BarrelShiftOpCode::LSL);
     }
     true
 }
 
-impl ArmInstruction {
+impl fmt::Display for ShiftedRegister {
-    fn make_shifted_reg_string(&self, reg: usize, shift: ShiftedRegister) -> String {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        let reg = reg_string(reg).to_string();
+        let reg = reg_string(self.reg).to_string();
-        if !is_shift(&shift) {
+        if !is_lsl0(&self) {
-            return reg;
+            write!(f, "{}", reg)
-        }
+        } else {
-
+            match self.shift_by {
-        match shift {
+                ShiftRegisterBy::ByAmount(imm) => write!(f, "{}, {} #{}", reg, self.bs_op, imm),
-            ShiftedRegister::ByAmount(imm, typ) => format!("{}, {} #{}", reg, typ, imm),
+                ShiftRegisterBy::ByRegister(rs) => {
-            ShiftedRegister::ByRegister(rs, typ) => format!("{}, {} {}", reg, typ, reg_string(rs)),
+                    write!(f, "{}, {} {}", reg, self.bs_op, reg_string(rs))
+                }
+            }
         }
     }
+}
 
+impl ArmInstruction {
     fn fmt_bx(&self, f: &mut fmt::Formatter) -> fmt::Result {
         write!(f, "bx\t{Rn}", Rn = reg_string(self.rn()))
     }
@@ -127,12 +131,8 @@ impl ArmInstruction {
                 write!(f, "#{}\t; {:#x}", value, value)?;
                 Ok(Some(value as u32))
             }
-            BarrelShifterValue::ShiftedRegister {
+            BarrelShifterValue::ShiftedRegister(shift) => {
-                reg,
+                write!(f, "{}", shift)?;
-                shift,
-                added: _,
-            } => {
-                write!(f, "{}", self.make_shifted_reg_string(reg, shift))?;
                 Ok(None)
             }
             _ => panic!("invalid operand2"),
@@ -197,15 +197,11 @@ impl ArmInstruction {
                     Some(format!("\t; {:#x}", value_for_commnet)),
                 )
             }
-            BarrelShifterValue::ShiftedRegister {
+            BarrelShifterValue::ShiftedRegister(shift) => (
-                reg,
-                shift,
-                added: Some(added),
-            } => (
                 format!(
                     "{}{}",
-                    if added { "" } else { "-" },
+                    if shift.added.unwrap_or(true) { "" } else { "-" },
-                    self.make_shifted_reg_string(reg, shift)
+                    shift
                 ),
                 None,
             ),

src/core/arm7tdmi/arm/exec.rs

@@ -1,6 +1,6 @@
 use crate::bit::BitIndex;
 
-use crate::core::arm7tdmi::alu::*;
+use super::super::alu::*;
 use crate::core::arm7tdmi::bus::Bus;
 use crate::core::arm7tdmi::cpu::{Core, CpuExecResult};
 use crate::core::arm7tdmi::psr::RegPSR;
@@ -125,21 +125,17 @@ impl Core {
 
     fn decode_operand2(&mut self, op2: BarrelShifterValue, set_flags: bool) -> CpuResult<u32> {
         match op2 {
-            BarrelShifterValue::RotatedImmediate(imm, r) => {
+            BarrelShifterValue::RotatedImmediate(val, amount) => {
-                let result = imm.rotate_right(r);
+                let result = self.ror(val, amount, self.cpsr.C(), false , true);
-                if set_flags {
-                    self.cpsr.set_C((result as u32).bit(31));
-                }
                 Ok(result)
             }
-            BarrelShifterValue::ShiftedRegister {
+            BarrelShifterValue::ShiftedRegister(x) => {
-                reg,
-                shift,
-                added: _,
-            } => {
                 // +1I
                 self.add_cycle();
-                let result = self.register_shift(reg, shift)?;
+                let result = self.register_shift(x)?;
+                if set_flags {
+                    self.cpsr.set_C(self.bs_carry_out);
+                }
                 Ok(result as u32)
             }
             _ => unreachable!(),
@@ -162,21 +158,19 @@ impl Core {
     /// Cycles: 1S+x+y (from GBATEK)
     ///         Add x=1I cycles if Op2 shifted-by-register. Add y=1S+1N cycles if Rd=R15.
     fn exec_data_processing(&mut self, _bus: &mut Bus, insn: ArmInstruction) -> CpuExecResult {
-        // TODO handle carry flag
-
         let op1 = if insn.rn() == REG_PC {
-            self.pc as i32 // prefething
+            self.pc as i32
         } else {
             self.get_reg(insn.rn()) as i32
         };
 
+        let s_flag = insn.set_cond_flag();
         let opcode = insn.opcode().unwrap();
 
-        let set_flags = opcode.is_setting_flags() || insn.set_cond_flag();
         let op2 = insn.operand2()?;
-        let op2 = self.decode_operand2(op2, set_flags)? as i32;
+        let op2 = self.decode_operand2(op2, s_flag)? as i32;
 
-        if !insn.set_cond_flag() {
+        if !s_flag {
             match opcode {
                 AluOpCode::TEQ | AluOpCode::CMN => {
                     return self.exec_msr(insn, op2 as u32);
@@ -188,7 +182,13 @@ impl Core {
 
         let rd = insn.rd();
 
-        if let Some(result) = self.alu(opcode, op1, op2, set_flags) {
+        let alu_res = if s_flag {
+            self.alu_flags(opcode, op1, op2)
+        } else {
+            Some(self.alu(opcode, op1, op2))
+        };
+
+        if let Some(result) = alu_res {
             if rd == REG_PC {
                 self.transfer_spsr_mode();
                 self.flush_pipeline();
@@ -251,7 +251,7 @@ impl Core {
             if insn.transfer_size() == 1 {
                 self.store_8(addr, value as u8, bus);
             } else {
-                self.store_32(addr, value, bus);
+                self.store_32(addr & !0x3, value, bus);
             };
         }
 

src/core/arm7tdmi/arm/mod.rs

@@ -285,12 +285,12 @@ impl ArmInstruction {
         let ofs = self.raw.bit_range(0..12);
         if self.raw.bit(25) {
             let rm = ofs & 0xf;
-            let shift = ShiftedRegister::from(ofs);
+            BarrelShifterValue::ShiftedRegister(ShiftedRegister {
-            BarrelShifterValue::ShiftedRegister {
                 reg: rm as usize,
-                shift: shift,
+                shift_by: self.get_shift_reg_by(ofs),
+                bs_op: self.get_bs_op(ofs),
                 added: Some(self.add_offset_flag()),
-            }
+            })
         } else {
             let ofs = if self.add_offset_flag() {
                 ofs as i32
@@ -301,6 +301,20 @@ impl ArmInstruction {
         }
     }
 
+    fn get_bs_op(&self, shift_field: u32) -> BarrelShiftOpCode {
+        BarrelShiftOpCode::from_u8(shift_field.bit_range(5..7) as u8).unwrap()
+    }
+
+    fn get_shift_reg_by(&self, shift_field: u32) -> ShiftRegisterBy {
+        if shift_field.bit(4) {
+            let rs = shift_field.bit_range(8..12) as usize;
+            ShiftRegisterBy::ByRegister(rs)
+        } else {
+            let amount = shift_field.bit_range(7..12) as u32;
+            ShiftRegisterBy::ByAmount(amount)
+        }
+    }
+
     pub fn ldr_str_hs_offset(&self) -> Result<BarrelShifterValue, ArmDecodeError> {
         match self.fmt {
             ArmFormat::LDR_STR_HS_IMM => {
@@ -312,11 +326,12 @@ impl ArmInstruction {
                 };
                 Ok(BarrelShifterValue::ImmediateValue(offset8))
             }
-            ArmFormat::LDR_STR_HS_REG => Ok(BarrelShifterValue::ShiftedRegister {
+            ArmFormat::LDR_STR_HS_REG => Ok(BarrelShifterValue::ShiftedRegister(ShiftedRegister {
                 reg: (self.raw & 0xf) as usize,
-                shift: ShiftedRegister::ByAmount(0, BarrelShiftOpCode::LSL),
+                shift_by: ShiftRegisterBy::ByAmount(0),
+                bs_op: BarrelShiftOpCode::LSL,
                 added: Some(self.add_offset_flag()),
-            }),
+            })),
             _ => Err(self.make_decode_error(DecodedPartDoesNotBelongToInstruction)),
         }
     }
@@ -329,12 +344,13 @@ impl ArmInstruction {
             Ok(BarrelShifterValue::RotatedImmediate(immediate, rotate))
         } else {
             let reg = op2 & 0xf;
-            let shift = ShiftedRegister::from(op2); // TODO error handling
+            let shifted_reg = ShiftedRegister {
-            Ok(BarrelShifterValue::ShiftedRegister {
                 reg: reg as usize,
-                shift: shift,
+                bs_op: self.get_bs_op(op2),
+                shift_by: self.get_shift_reg_by(op2),
                 added: None,
-            })
+            }; // TODO error handling
+            Ok(BarrelShifterValue::ShiftedRegister(shifted_reg))
         }
     }
 
@@ -433,11 +449,12 @@ mod tests {
         assert_eq!(decoded.rn(), 5);
         assert_eq!(
             decoded.ldr_str_offset(),
-            BarrelShifterValue::ShiftedRegister {
+            BarrelShifterValue::ShiftedRegister(ShiftedRegister {
                 reg: 6,
-                shift: ShiftedRegister::ByAmount(5, BarrelShiftOpCode::LSL),
+                shift_by: ShiftRegisterBy::ByAmount(5),
+                bs_op: BarrelShiftOpCode::LSL,
                 added: Some(false)
-            }
+            })
         );
 
         assert_eq!(format!("{}", decoded), "ldreq\tr2, [r5, -r6, lsl #5]");
@@ -474,13 +491,13 @@ mod tests {
         assert_eq!(decoded.rn(), 4);
         assert_eq!(
             decoded.ldr_str_offset(),
-            BarrelShifterValue::ShiftedRegister {
+            BarrelShifterValue::ShiftedRegister(ShiftedRegister {
                 reg: 7,
-                shift: ShiftedRegister::ByAmount(8, BarrelShiftOpCode::ASR),
+                shift_by: ShiftRegisterBy::ByAmount(8),
+                bs_op: BarrelShiftOpCode::ASR,
                 added: Some(false)
-            }
+            })
         );
-
         assert_eq!(format!("{}", decoded), "strteq\tr2, [r4], -r7, asr #8");
 
         let mut core = Core::new();

src/core/arm7tdmi/cpu.rs

@@ -41,6 +41,8 @@ pub struct Core {
     pub cpsr: RegPSR,
     pub spsr: [RegPSR; 5],
 
+    pub bs_carry_out: bool,
+
     pipeline_state: PipelineState,
     fetched_arm: u32,
     decoded_arm: u32,
@@ -124,23 +126,12 @@ impl Core {
         }
     }
 
-    pub fn ror(&mut self, value: u32, rotation: u32) -> u32 {
-        let mut carry = false; // we don't need to update the carry flag
-        self.barrel_shift(
-            value as i32,
-            rotation,
-            BarrelShiftOpCode::ROR,
-            &mut carry,
-            false,
-        ) as u32
-    }
-
     /// Helper function for "ldr" instruction that handles misaligned addresses
     pub fn ldr_word(&mut self, addr: Addr, bus: &Bus) -> u32 {
         if addr & 0x3 != 0 {
             let rotation = (addr & 0x3) << 3;
             let value = self.load_32(addr & !0x3, bus);
-            self.ror(value, rotation)
+            self.ror(value, rotation, self.cpsr.C(), false, false)
         } else {
             self.load_32(addr, bus)
         }
@@ -151,7 +142,7 @@ impl Core {
         if addr & 0x1 != 0 {
             let rotation = (addr & 0x1) << 3;
             let value = self.load_16(addr & !0x1, bus);
-            self.ror(value as u32, rotation)
+            self.ror(value as u32, rotation, self.cpsr.C(), false, false)
         } else {
             self.load_16(addr, bus) as u32
         }

src/core/arm7tdmi/thumb/display.rs

@@ -37,14 +37,10 @@ impl ThumbInstruction {
     }
 
     fn fmt_thumb_alu_ops(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        use ShiftedRegister::ByRegister;
+        use ShiftRegisterBy::ByRegister;
         let (op, shft) = self.alu_opcode();
-        if let Some(BarrelShifterValue::ShiftedRegister {
+        if let Some(BarrelShifterValue::ShiftedRegister(x)) = shft {
-            shift: ByRegister(_, op),
+            write!(f, "{}", x.bs_op)?;
-            ..
-        }) = shft
-        {
-            write!(f, "{}", op)?;
         } else if op == AluOpCode::RSB {
             write!(f, "neg")?;
         } else {

src/core/arm7tdmi/thumb/exec.rs

@@ -22,15 +22,18 @@ impl Core {
         insn: ThumbInstruction,
     ) -> CpuExecResult {
         let op2 = self
-            .register_shift(
+            .register_shift(ShiftedRegister {
-                insn.rs(),
+                reg: insn.rs(),
-                ShiftedRegister::ByAmount(insn.offset5() as u8 as u32, insn.format1_op()),
+                shift_by: ShiftRegisterBy::ByAmount(insn.offset5() as u8 as u32),
-            )
+                bs_op: insn.format1_op(),
-            .unwrap();
+                added: None,
+            })
+            .unwrap() as i32;
+        self.cpsr.set_C(self.bs_carry_out);
 
         let rd = insn.rd();
         let op1 = self.get_reg(rd) as i32;
-        let result = self.alu(AluOpCode::MOV, op1, op2, true);
+        let result = self.alu_flags(AluOpCode::MOV, op1, op2);
         if let Some(result) = result {
             self.set_reg(rd, result as u32);
         }
@@ -51,7 +54,7 @@ impl Core {
             AluOpCode::ADD
         };
 
-        let result = self.alu(arm_alu_op, op1, op2, true);
+        let result = self.alu_flags(arm_alu_op, op1, op2);
         if let Some(result) = result {
             self.set_reg(insn.rd(), result as u32);
         }
@@ -67,7 +70,7 @@ impl Core {
         let arm_alu_op: AluOpCode = insn.format3_op().into();
         let op1 = self.get_reg(insn.rd()) as i32;
         let op2 = insn.offset8() as u8 as i32;
-        let result = self.alu(arm_alu_op, op1, op2, true);
+        let result = self.alu_flags(arm_alu_op, op1, op2);
         if let Some(result) = result {
             self.set_reg(insn.rd(), result as u32);
         }
@@ -97,7 +100,7 @@ impl Core {
             self.get_reg(insn.rs()) as i32
         };
 
-        let result = self.alu(arm_alu_op, op1, op2, true);
+        let result = self.alu_flags(arm_alu_op, op1, op2);
         if let Some(result) = result {
             self.set_reg(rd, result as u32);
         }
@@ -137,8 +140,12 @@ impl Core {
             let set_flags = arm_alu_op.is_setting_flags();
             let op1 = self.get_reg(dst_reg) as i32;
             let op2 = self.get_reg(src_reg) as i32;
-            let result = self.alu(arm_alu_op, op1, op2, set_flags);
+            let alu_res = if set_flags {
-            if let Some(result) = result {
+                self.alu_flags(arm_alu_op, op1, op2)
+            } else {
+                Some(self.alu(arm_alu_op, op1, op2))
+            };
+            if let Some(result) = alu_res {
                 self.set_reg(dst_reg, result as u32);
                 if dst_reg == REG_PC {
                     self.flush_pipeline();
@@ -303,10 +310,7 @@ impl Core {
         let op2 = insn.sword7();
         let arm_alu_op = AluOpCode::ADD;
 
-        let result = self.alu(arm_alu_op, op1, op2, false);
+        self.gpr[REG_SP] = self.alu(arm_alu_op, op1, op2) as u32;
-        if let Some(result) = result {
-            self.gpr[REG_SP] = result as u32;
-        }
 
         Ok(())
     }

src/core/arm7tdmi/thumb/mod.rs

@@ -246,39 +246,43 @@ impl ThumbInstruction {
     }
 
     pub fn alu_opcode(&self) -> (AluOpCode, Option<BarrelShifterValue>) {
-        use ShiftedRegister::*;
+        use ShiftRegisterBy::*;
         match self.raw.bit_range(6..10) {
             0b0010 => (
                 AluOpCode::MOV,
-                Some(BarrelShifterValue::ShiftedRegister {
+                Some(BarrelShifterValue::ShiftedRegister(ShiftedRegister {
                     reg: self.rd(),
-                    shift: ByRegister(self.rs(), BarrelShiftOpCode::LSL),
+                    shift_by: ByRegister(self.rs()),
+                    bs_op: BarrelShiftOpCode::LSL,
                     added: Some(true),
-                }),
+                })),
             ),
             0b0011 => (
                 AluOpCode::MOV,
-                Some(BarrelShifterValue::ShiftedRegister {
+                Some(BarrelShifterValue::ShiftedRegister(ShiftedRegister {
                     reg: self.rd(),
-                    shift: ByRegister(self.rs(), BarrelShiftOpCode::LSR),
+                    shift_by: ByRegister(self.rs()),
+                    bs_op: BarrelShiftOpCode::LSR,
                     added: Some(true),
-                }),
+                })),
             ),
             0b0100 => (
                 AluOpCode::MOV,
-                Some(BarrelShifterValue::ShiftedRegister {
+                Some(BarrelShifterValue::ShiftedRegister(ShiftedRegister {
                     reg: self.rd(),
-                    shift: ByRegister(self.rs(), BarrelShiftOpCode::ASR),
+                    shift_by: ByRegister(self.rs()),
+                    bs_op: BarrelShiftOpCode::ASR,
                     added: Some(true),
-                }),
+                })),
             ),
             0b0111 => (
                 AluOpCode::MOV,
-                Some(BarrelShifterValue::ShiftedRegister {
+                Some(BarrelShifterValue::ShiftedRegister(ShiftedRegister {
                     reg: self.rd(),
-                    shift: ByRegister(self.rs(), BarrelShiftOpCode::ROR),
+                    shift_by: ByRegister(self.rs()),
+                    bs_op: BarrelShiftOpCode::ROR,
                     added: Some(true),
-                }),
+                })),
             ),
             0b1001 => (AluOpCode::RSB, Some(BarrelShifterValue::ImmediateValue(0))),
             0b1101 => panic!("tried to decode MUL"),