From 5581435b4edc8836e172a5088d74d2ec97b2bc99 Mon Sep 17 00:00:00 2001 From: iximeow Date: Sun, 26 Jul 2026 17:58:42 +0000 Subject: thumb2: fix parallel addition/subtraction, (un)signed --- CHANGELOG | 3 + src/armv7/thumb.rs | 80 +++++++++--------------- tests/armv7/thumb.rs | 168 +++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 200 insertions(+), 51 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index c2cc889..33e85a7 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -7,6 +7,9 @@ * support non-`*2` forms of coprocessor instructions * collapse `*2` encoding choice for coprocessor instructions into a paramter on the opcode (rather than distinct opcodes) +* thumb2: use correct opcode table for parallel addition/subtractions + * the decoder reused the non-thumb encoding table, but thumb opcode/sizes are + encoded in a different order. several fixes from @Grond66: * ARMv7: support the RRX rotate mode. diff --git a/src/armv7/thumb.rs b/src/armv7/thumb.rs index 9e9c30a..675b9f2 100644 --- a/src/armv7/thumb.rs +++ b/src/armv7/thumb.rs @@ -2767,38 +2767,30 @@ pub fn decode_into::Address, ::Word>>(d let op2 = op2.load::(); if op2 < 0b0100 { // `Parallel addition and subtraction, signed` + // + // surprise! the encoding of op1/op2 in this table is similar to + // but entirely reordered from the A32 encodings. let op1 = instr2[4..7].load::(); let op2 = lower2[4..6].load::(); - if op1 == 0 || op1 > 0b100 || op2 == 0b11 { - return Err(DecodeError::InvalidOpcode); - } - let opcode_idx = (op1 - 1) * 3 + op2; + let opcode_idx = op1 * 4 + op2; let rn = instr2[0..4].load::(); let rd = lower2[8..12].load::(); let rm = lower2[0..4].load::(); - inst.opcode = [ - Opcode::SADD16, - Opcode::QADD16, - Opcode::SHADD16, - Opcode::SASX, - Opcode::QASX, - Opcode::SHASX, - Opcode::SSAX, - Opcode::QSAX, - Opcode::SHSAX, - Opcode::SSUB16, - Opcode::QSUB16, - Opcode::SHSUB16, - Opcode::SADD8, - Opcode::QADD8, - Opcode::SHADD8, - Opcode::SSUB8, - Opcode::QSUB8, - Opcode::SHSUB8, - ][opcode_idx]; + static TABLE_A6_25: [Option; 32] = [ + Some(Opcode::SADD8), Some(Opcode::QADD8), Some(Opcode::SHADD8), None, + Some(Opcode::SADD16), Some(Opcode::QADD16), Some(Opcode::SHADD16), None, + Some(Opcode::SASX), Some(Opcode::QASX), Some(Opcode::SHASX), None, + None, None, None, None, + Some(Opcode::SSUB8), Some(Opcode::QSUB8), Some(Opcode::SHSUB8), None, + Some(Opcode::SSUB16), Some(Opcode::QSUB16), Some(Opcode::SHSUB16), None, + Some(Opcode::SSAX), Some(Opcode::QSAX), Some(Opcode::SHSAX), None, + None, None, None, None, + ]; + + inst.opcode = TABLE_A6_25[opcode_idx].ok_or(DecodeError::InvalidOpcode)?; inst.operands = [ Operand::Reg(Reg::from_u8(rd)), Operand::Reg(Reg::from_u8(rn)), @@ -2809,39 +2801,25 @@ pub fn decode_into::Address, ::Word>>(d // `Parallel addition and subtraction, unsigned` (`A6-244`) let op1 = instr2[4..7].load::(); let op2 = lower2[4..6].load::(); - if op1 > 0b100 || op2 == 0b11 { - return Err(DecodeError::InvalidOpcode); - } - if op1 == 0 { - return Err(DecodeError::InvalidOpcode); - } - let opcode_idx = (op1 - 1) * 3 + op2; + let opcode_idx = op1 * 4 + op2; let rn = instr2[0..4].load::(); let rd = lower2[8..12].load::(); let rm = lower2[0..4].load::(); - inst.opcode = [ - Opcode::UADD16, - Opcode::UQADD16, - Opcode::UHADD16, - Opcode::UASX, - Opcode::UQASX, - Opcode::UHASX, - Opcode::USAX, - Opcode::UQSAX, - Opcode::UHSAX, - Opcode::USUB16, - Opcode::UQSUB16, - Opcode::UHSUB16, - Opcode::UADD8, - Opcode::UQADD8, - Opcode::UHADD8, - Opcode::USUB8, - Opcode::UQSUB8, - Opcode::UHSUB8, - ][opcode_idx]; + static TABLE_A6_26: [Option; 32] = [ + Some(Opcode::UADD8), Some(Opcode::UQADD8), Some(Opcode::UHADD8), None, + Some(Opcode::UADD16), Some(Opcode::UQADD16), Some(Opcode::UHADD16), None, + Some(Opcode::UASX), Some(Opcode::UQASX), Some(Opcode::UHASX), None, + None, None, None, None, + Some(Opcode::USUB8), Some(Opcode::UQSUB8), Some(Opcode::UHSUB8), None, + Some(Opcode::USUB16), Some(Opcode::UQSUB16), Some(Opcode::UHSUB16), None, + Some(Opcode::USAX), Some(Opcode::UQSAX), Some(Opcode::UHSAX), None, + None, None, None, None, + ]; + + inst.opcode = TABLE_A6_26[opcode_idx].ok_or(DecodeError::InvalidOpcode)?; inst.operands = [ Operand::Reg(Reg::from_u8(rd)), Operand::Reg(Reg::from_u8(rn)), diff --git a/tests/armv7/thumb.rs b/tests/armv7/thumb.rs index c8b0b11..2152d1e 100644 --- a/tests/armv7/thumb.rs +++ b/tests/armv7/thumb.rs @@ -4283,6 +4283,174 @@ fn test_decode_mov_reg_shift_cases() { } #[test] +fn test_parallel_addsub() { + // TODO: should these have a .w suffix? + test_display( + &[0x82, 0xfa, 0x03, 0xfb], + "sadd8.w fp, r2, r3" + ); + test_display( + &[0x92, 0xfa, 0x03, 0xfb], + "sadd16.w fp, r2, r3" + ); + test_display( + &[0xa2, 0xfa, 0x03, 0xfb], + "sasx.w fp, r2, r3" + ); + test_invalid(&[0xb2, 0xfa, 0x03, 0xfb]); + test_display( + &[0xc2, 0xfa, 0x03, 0xfb], + "ssub8.w fp, r2, r3" + ); + test_display( + &[0xd2, 0xfa, 0x03, 0xfb], + "ssub16.w fp, r2, r3" + ); + test_display( + &[0xe2, 0xfa, 0x03, 0xfb], + "ssax.w fp, r2, r3" + ); + test_invalid(&[0xf2, 0xfa, 0x03, 0xfb]); + + test_display( + &[0x82, 0xfa, 0x13, 0xfb], + "qadd8.w fp, r2, r3" + ); + test_display( + &[0x92, 0xfa, 0x13, 0xfb], + "qadd16.w fp, r2, r3" + ); + test_display( + &[0xa2, 0xfa, 0x13, 0xfb], + "qasx.w fp, r2, r3" + ); + test_invalid(&[0xb2, 0xfa, 0x13, 0xfb]); + test_display( + &[0xc2, 0xfa, 0x13, 0xfb], + "qsub8.w fp, r2, r3" + ); + test_display( + &[0xd2, 0xfa, 0x13, 0xfb], + "qsub16.w fp, r2, r3" + ); + test_display( + &[0xe2, 0xfa, 0x13, 0xfb], + "qsax.w fp, r2, r3" + ); + test_invalid(&[0xf2, 0xfa, 0x13, 0xfb]); + + test_display( + &[0x82, 0xfa, 0x23, 0xfb], + "shadd8.w fp, r2, r3" + ); + test_display( + &[0x92, 0xfa, 0x23, 0xfb], + "shadd16.w fp, r2, r3" + ); + test_display( + &[0xa2, 0xfa, 0x23, 0xfb], + "shasx.w fp, r2, r3" + ); + test_invalid(&[0xb2, 0xfa, 0x23, 0xfb]); + test_display( + &[0xc2, 0xfa, 0x23, 0xfb], + "shsub8.w fp, r2, r3" + ); + test_display( + &[0xd2, 0xfa, 0x23, 0xfb], + "shsub16.w fp, r2, r3" + ); + test_display( + &[0xe2, 0xfa, 0x23, 0xfb], + "shsax.w fp, r2, r3" + ); + test_invalid(&[0xf2, 0xfa, 0x23, 0xfb]); + + // and now for + // > Parallel addition and subtraction, unsigned + test_display( + &[0x82, 0xfa, 0x43, 0xfb], + "uadd8.w fp, r2, r3" + ); + test_display( + &[0x92, 0xfa, 0x43, 0xfb], + "uadd16.w fp, r2, r3" + ); + test_display( + &[0xa2, 0xfa, 0x43, 0xfb], + "uasx.w fp, r2, r3" + ); + test_invalid(&[0xb2, 0xfa, 0x43, 0xfb]); + test_display( + &[0xc2, 0xfa, 0x43, 0xfb], + "usub8.w fp, r2, r3" + ); + test_display( + &[0xd2, 0xfa, 0x43, 0xfb], + "usub16.w fp, r2, r3" + ); + test_display( + &[0xe2, 0xfa, 0x43, 0xfb], + "usax.w fp, r2, r3" + ); + test_invalid(&[0xf2, 0xfa, 0x43, 0xfb]); + + test_display( + &[0x82, 0xfa, 0x53, 0xfb], + "uqadd8.w fp, r2, r3" + ); + test_display( + &[0x92, 0xfa, 0x53, 0xfb], + "uqadd16.w fp, r2, r3" + ); + test_display( + &[0xa2, 0xfa, 0x53, 0xfb], + "uqasx.w fp, r2, r3" + ); + test_invalid(&[0xb2, 0xfa, 0x53, 0xfb]); + test_display( + &[0xc2, 0xfa, 0x53, 0xfb], + "uqsub8.w fp, r2, r3" + ); + test_display( + &[0xd2, 0xfa, 0x53, 0xfb], + "uqsub16.w fp, r2, r3" + ); + test_display( + &[0xe2, 0xfa, 0x53, 0xfb], + "uqsax.w fp, r2, r3" + ); + test_invalid(&[0xf2, 0xfa, 0x53, 0xfb]); + + test_display( + &[0x82, 0xfa, 0x63, 0xfb], + "uhadd8.w fp, r2, r3" + ); + test_display( + &[0x92, 0xfa, 0x63, 0xfb], + "uhadd16.w fp, r2, r3" + ); + test_display( + &[0xa2, 0xfa, 0x63, 0xfb], + "uhasx.w fp, r2, r3" + ); + test_invalid(&[0xb2, 0xfa, 0x63, 0xfb]); + test_display( + &[0xc2, 0xfa, 0x63, 0xfb], + "uhsub8.w fp, r2, r3" + ); + test_display( + &[0xd2, 0xfa, 0x63, 0xfb], + "uhsub16.w fp, r2, r3" + ); + test_display( + &[0xe2, 0xfa, 0x63, 0xfb], + "uhsax.w fp, r2, r3" + ); + test_invalid(&[0xf2, 0xfa, 0x63, 0xfb]); +} + +#[test] fn test_decode_ux_sx_32b_cases() { // rotation is hw1[5:4] scaled by 8, and omitted when zero test_display( -- cgit v1.1