diff options
| -rw-r--r-- | src/armv7.rs | 155 | ||||
| -rw-r--r-- | tests/armv7/mod.rs | 2 |
2 files changed, 75 insertions, 82 deletions
diff --git a/src/armv7.rs b/src/armv7.rs index 0b43ed8..f23d0c9 100644 --- a/src/armv7.rs +++ b/src/armv7.rs @@ -299,6 +299,16 @@ impl RegShift { } } + /// Does this shift actually do anything? + fn is_noop(&self) -> bool { + match self.into_shift() { + RegShiftStyle::RegImm(shift) => shift.is_noop(), + // We can't know statically that this is a no-op, because the final result is based on + // register data. Therefore, consider it a live shift. + RegShiftStyle::RegReg(_) => false, + } + } + /// don't use this. it's for armv7 testing only. #[doc(hidden)] pub fn from_raw(data: u16) -> Self { @@ -445,6 +455,11 @@ impl RegImmShift { pub fn shiftee(&self) -> Reg { Reg::from_u8(self.data as u8 & 0b1111) } + + /// Does this shift actually do anything? + fn is_noop(&self) -> bool { + self.stype() == ShiftStyle::LSL && self.imm() == 0 + } } /// a struct describing an `arm` register. @@ -2540,101 +2555,77 @@ impl Decoder<ARMv7> for InstDecoder { if opcode >= 16 { unreachable!(); } + inst.opcode = DATA_PROCESSING_OPCODES[opcode as usize]; inst.set_s(s); - // at this point we know this is a data processing instruction - // either immediate shift or register shift - if word & 0b00010000 == 0 { - // |c o n d|0 0 0|x x x x|0|x x x x|x x x x|x x x x x|x x|0|x x x x| - // interpret the operands as - // | Rn | Rd | shift amount | shift | 0 | Rm | - let (Rn, Rd, shift_spec, Rm) = { - let Rm = (word & 0x0f) as u8; - let shift_spec = (word & 0xfff) as u16; - let word = word >> 12; - let Rd = (word & 0x0f) as u8; - let Rn = ((word >> 4) & 0x0f) as u8; - (Rn, Rd, shift_spec, Rm) - }; - - let last_operand = if shift_spec & 0xff0 == 0 { - // no shift, so the operand is just a register. - // - // TODO: this shift style is `lsl 0`, and not incorrect to report - // as just that. should it be impossible for `format_reg_shift` to - // get a register shifted by lsl 0? simplifying the register here - // seems valuable for consumers of individual operands. as-is, this - // is inconsistent across the library, which is probably the worst - // it could be... - Operand::Reg(Reg::from_u8(Rm)) - } else { - Operand::RegShift(RegShift::from_raw(shift_spec)) - }; + // |c o n d|0 0 0|x x x x|0|x x x x|x x x x|x x x x x|x x|x|x x x x| + // interpret the operands as + // | Rn | Rd | shift info... | + // + // note that bits 7 down to 4 are either `x . . 0` or `0 x x 1`, though. + // those bits comprise `op2` and are handled when determining where in + // table A5-2 this instruction lies. + + let (Rn, Rd, shift_spec, Rm) = { + let Rm = (word & 0x0f) as u8; + let shift_spec = (word & 0xfff) as u16; + let word = word >> 12; + let Rd = (word & 0x0f) as u8; + let Rn = ((word >> 4) & 0x0f) as u8; + (Rn, Rd, shift_spec, Rm) + }; - match inst.opcode { - Opcode::MOV | Opcode::MVN => { - if self.should_is_must { - if Rn != 0 { - return Err(DecodeError::Nonconforming); - } - } - inst.operands = [ - Operand::Reg(Reg::from_u8(Rd)), - last_operand, - Operand::Nothing, - Operand::Nothing - ]; - } + let reg_shift = RegShift::from_raw(shift_spec); + let last_operand = if reg_shift.is_noop() { + // no shift, so the operand is just a register. + // + // TODO: this shift style is `lsl 0`, and not incorrect to report + // as just that. should it be impossible for `format_reg_shift` to + // get a register shifted by lsl 0? simplifying the register here + // seems valuable for consumers of individual operands. as-is, this + // is inconsistent across the library, which is probably the worst + // it could be... + Operand::Reg(Reg::from_u8(Rm)) + } else { + Operand::RegShift(reg_shift) + }; - Opcode::CMP | Opcode::CMN => { - if self.should_is_must { - if Rd != 0 { - return Err(DecodeError::Nonconforming); - } + match inst.opcode { + Opcode::MOV | Opcode::MVN => { + if self.should_is_must { + if Rn != 0 { + return Err(DecodeError::Nonconforming); } - inst.operands = [ - Operand::Reg(Reg::from_u8(Rn)), - last_operand, - Operand::Nothing, - Operand::Nothing - ]; } + inst.operands = [ + Operand::Reg(Reg::from_u8(Rd)), + last_operand, + Operand::Nothing, + Operand::Nothing + ]; + } - _ => { - inst.operands = [ - Operand::Reg(Reg::from_u8(Rd)), - Operand::Reg(Reg::from_u8(Rn)), - last_operand, - Operand::Nothing - ]; + Opcode::CMP | Opcode::CMN => { + if self.should_is_must { + if Rd != 0 { + return Err(DecodeError::Nonconforming); + } } + inst.operands = [ + Operand::Reg(Reg::from_u8(Rn)), + last_operand, + Operand::Nothing, + Operand::Nothing + ]; } - } else { - // known 0 because it and bit 5 are not both 1 --v - // |c o n d|0 0 0|1 0 x x|0|x x x x|x x x x|x x x x 0|x x|1|x x x x| - // interpret the operands as - // | Rn | Rd | Rs | 0 | shift | 1 | Rm | - let (Rn, Rd, shift_spec) = { - let shift_spec = (word & 0xfff) as u16; - let word = word >> 12; - let Rd = (word & 0x0f) as u8; - let Rn = ((word >> 4) & 0x0f) as u8; - (Rn, Rd, shift_spec) - }; - // page A5-200 indicates that saturating add and subtract should be - // here? - if (0b1101 & opcode) == 0b1101 { - // these are all invalid - inst.opcode = Opcode::Invalid; - return Err(DecodeError::InvalidOpcode); - } else { - // TODO: unsure about this RegShift... + + _ => { inst.operands = [ Operand::Reg(Reg::from_u8(Rd)), Operand::Reg(Reg::from_u8(Rn)), - Operand::RegShift(RegShift::from_raw(shift_spec)), - Operand::Nothing, + last_operand, + Operand::Nothing ]; } } diff --git a/tests/armv7/mod.rs b/tests/armv7/mod.rs index 58f7e0c..78d725b 100644 --- a/tests/armv7/mod.rs +++ b/tests/armv7/mod.rs @@ -740,8 +740,10 @@ fn test_cmp_immediate_decode() { fn test_cmp_register_decode() { test_all([0x01, 0x00, 0x52, 0xe1], "cmps r2, r1"); test_all([0x01, 0x03, 0x52, 0xe1], "cmps r2, r1, lsl 6"); + test_all([0x11, 0x03, 0x52, 0xe1], "cmps r2, r1, lsl r3"); test_all([0x01, 0x00, 0x72, 0xe1], "cmns r2, r1"); test_all([0x01, 0x03, 0x72, 0xe1], "cmns r2, r1, lsl 6"); + test_all([0x11, 0x03, 0x72, 0xe1], "cmns r2, r1, lsl r3"); } static INSTRUCTION_BYTES: [u8; 4 * 60] = [ |