From e6f77a9b80f4948b8a9005ba7502fd4da387417f Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Fri, 1 Mar 2024 18:06:54 -0500 Subject: [PATCH 1/2] Add logical right-shift tests. --- InstructionSets/ARM/BarrelShifter.hpp | 13 ++--- InstructionSets/ARM/Executor.hpp | 46 ++++++++------- .../Mac/Clock SignalTests/ARMDecoderTests.mm | 56 +++++++++++++++++-- 3 files changed, 82 insertions(+), 33 deletions(-) diff --git a/InstructionSets/ARM/BarrelShifter.hpp b/InstructionSets/ARM/BarrelShifter.hpp index 449ac3fc1..9c45c3059 100644 --- a/InstructionSets/ARM/BarrelShifter.hpp +++ b/InstructionSets/ARM/BarrelShifter.hpp @@ -28,6 +28,8 @@ template <> struct Carry { /// Apply a rotation of @c type to @c source of @c amount; @c carry should be either @c 1 or @c 0 /// at call to represent the current value of the carry flag. If @c set_carry is @c true then @c carry will /// receive the new value of the carry flag following the rotation — @c 0 for no carry, @c non-0 for carry. +/// +/// Shift amounts of 0 are given the meaning attributed to them for immediate shift counts. template void shift(uint32_t &source, uint32_t amount, typename Carry::type carry) { switch(type) { @@ -48,17 +50,14 @@ void shift(uint32_t &source, uint32_t amount, typename Carry::type ca if(amount > 32) { if constexpr (set_carry) carry = 0; source = 0; - } else if(amount == 32) { - if constexpr (set_carry) carry = source & 0x8000'0000; - source = 0; - } else if(amount > 0) { - if constexpr (set_carry) carry = source & (1 << (amount - 1)); - source >>= amount; - } else { + } else if(amount == 32 || !amount) { // A logical shift right by '0' is treated as a shift by 32; // assemblers are supposed to map LSR #0 to LSL #0. if constexpr (set_carry) carry = source & 0x8000'0000; source = 0; + } else { + if constexpr (set_carry) carry = source & (1 << (amount - 1)); + source >>= amount; } break; diff --git a/InstructionSets/ARM/Executor.hpp b/InstructionSets/ARM/Executor.hpp index 7ce54c1d2..e7bdc995e 100644 --- a/InstructionSets/ARM/Executor.hpp +++ b/InstructionSets/ARM/Executor.hpp @@ -26,25 +26,6 @@ struct Executor { template uint32_t decode_shift(FieldsT fields, uint32_t &rotate_carry, uint32_t pc_offset) { - uint32_t shift_amount; - if constexpr (allow_register) { - if(fields.shift_count_is_register()) { - // "When R15 appears in either of the Rn or Rs positions it will give the value - // of the PC alone, with the PSR bits replaced by zeroes. ... - // - // If a register is used to specify the shift amount, the - // PC will be 8 bytes ahead when used as Rs." - shift_amount = - fields.shift_register() == 15 ? - registers_.pc(8) : - registers_.active[fields.shift_register()]; - } else { - shift_amount = fields.shift_amount(); - } - } else { - shift_amount = fields.shift_amount(); - } - // "When R15 appears in the Rm position it will give the value of the PC together // with the PSR flags to the barrel shifter. ... // @@ -57,8 +38,33 @@ struct Executor { } else { operand2 = registers_.active[fields.operand2()]; } - shift(fields.shift_type(), operand2, shift_amount, rotate_carry); + uint32_t shift_amount; + if constexpr (allow_register) { + if(fields.shift_count_is_register()) { + // "When R15 appears in either of the Rn or Rs positions it will give the value + // of the PC alone, with the PSR bits replaced by zeroes. ... + // + // If a register is used to specify the shift amount, the + // PC will be 8 bytes ahead when used as Rs." + shift_amount = + fields.shift_register() == 15 ? + registers_.pc(8) : + registers_.active[fields.shift_register()]; + + // A register shift amount of 0 has a different meaning than an in-instruction + // shift amount of 0. + if(!shift_amount) { + return operand2; + } + } else { + shift_amount = fields.shift_amount(); + } + } else { + shift_amount = fields.shift_amount(); + } + + shift(fields.shift_type(), operand2, shift_amount, rotate_carry); return operand2; } diff --git a/OSBindings/Mac/Clock SignalTests/ARMDecoderTests.mm b/OSBindings/Mac/Clock SignalTests/ARMDecoderTests.mm index cdc3e72ae..dc96c0ccf 100644 --- a/OSBindings/Mac/Clock SignalTests/ARMDecoderTests.mm +++ b/OSBindings/Mac/Clock SignalTests/ARMDecoderTests.mm @@ -41,13 +41,57 @@ struct Memory { @implementation ARMDecoderTests -- (void)testXYX { - Executor scheduler; +//- (void)testXYX { +// Executor scheduler; +// +// for(int c = 0; c < 65536; c++) { +// InstructionSet::ARM::dispatch(c << 16, scheduler); +// } +// InstructionSet::ARM::dispatch(0xEAE06900, scheduler); +//} - for(int c = 0; c < 65536; c++) { - InstructionSet::ARM::dispatch(c << 16, scheduler); - } - InstructionSet::ARM::dispatch(0xEAE06900, scheduler); +- (void)testBarrelShifterLogicalRight { + uint32_t value; + uint32_t carry; + + // Test successive shifts by 4; one generating carry and one not. + value = 0x12345678; + shift(value, 4, carry); + XCTAssertEqual(value, 0x1234567); + XCTAssertNotEqual(carry, 0); + + shift(value, 4, carry); + XCTAssertEqual(value, 0x123456); + XCTAssertEqual(carry, 0); + + // Test shift by 1. + value = 0x8003001; + shift(value, 1, carry); + XCTAssertEqual(value, 0x4001800); + XCTAssertNotEqual(carry, 0); + + // Test a shift by greater than 32. + value = 0xffff'ffff; + shift(value, 33, carry); + XCTAssertEqual(value, 0); + XCTAssertEqual(carry, 0); + + // Test shifts by 32: result is always 0, carry is whatever was in bit 31. + value = 0xffff'ffff; + shift(value, 32, carry); + XCTAssertEqual(value, 0); + XCTAssertNotEqual(carry, 0); + + value = 0x7fff'ffff; + shift(value, 32, carry); + XCTAssertEqual(value, 0); + XCTAssertEqual(carry, 0); + + // Test that a logical right shift by 0 is the same as a shift by 32. + value = 0xffff'ffff; + shift(value, 0, carry); + XCTAssertEqual(value, 0); + XCTAssertNotEqual(carry, 0); } @end From c865da67e0bf8f15d803ab1a9caf50b48a0a984f Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Sat, 2 Mar 2024 15:12:03 -0500 Subject: [PATCH 2/2] Introduce further barrel-shifter tests. --- .../Mac/Clock SignalTests/ARMDecoderTests.mm | 101 ++++++++++++++++++ 1 file changed, 101 insertions(+) diff --git a/OSBindings/Mac/Clock SignalTests/ARMDecoderTests.mm b/OSBindings/Mac/Clock SignalTests/ARMDecoderTests.mm index dc96c0ccf..352de1907 100644 --- a/OSBindings/Mac/Clock SignalTests/ARMDecoderTests.mm +++ b/OSBindings/Mac/Clock SignalTests/ARMDecoderTests.mm @@ -50,6 +50,29 @@ struct Memory { // InstructionSet::ARM::dispatch(0xEAE06900, scheduler); //} +- (void)testBarrelShifterLogicalLeft { + uint32_t value; + uint32_t carry; + + // Test a shift by 1 into carry. + value = 0x8000'0000; + shift(value, 1, carry); + XCTAssertEqual(value, 0); + XCTAssertNotEqual(carry, 0); + + // Test a shift by 18 into carry. + value = 0x0000'4001; + shift(value, 18, carry); + XCTAssertEqual(value, 0x4'0000); + XCTAssertNotEqual(carry, 0); + + // Test a shift by 17, not generating carry. + value = 0x0000'4001; + shift(value, 17, carry); + XCTAssertEqual(value, 0x8002'0000); + XCTAssertEqual(carry, 0); +} + - (void)testBarrelShifterLogicalRight { uint32_t value; uint32_t carry; @@ -94,4 +117,82 @@ struct Memory { XCTAssertNotEqual(carry, 0); } +- (void)testBarrelShifterArithmeticRight { + uint32_t value; + uint32_t carry; + + // Test a short negative shift. + value = 0x8000'0030; + shift(value, 1, carry); + XCTAssertEqual(value, 0xc000'0018); + XCTAssertEqual(carry, 0); + + // Test a medium negative shift without carry. + value = 0xffff'0000; + shift(value, 11, carry); + XCTAssertEqual(value, 0xffff'ffe0); + XCTAssertEqual(carry, 0); + + // Test a medium negative shift with carry. + value = 0xffc0'0000; + shift(value, 23, carry); + XCTAssertEqual(value, 0xffff'ffff); + XCTAssertNotEqual(carry, 0); + + // Test a long negative shift. + value = 0x8000'0000; + shift(value, 32, carry); + XCTAssertEqual(value, 0xffff'ffff); + XCTAssertNotEqual(carry, 0); + + // Test a positive shift. + value = 0x0123'0031; + shift(value, 3, carry); + XCTAssertEqual(value, 0x24'6006); + XCTAssertEqual(carry, 0); +} + +- (void)testBarrelShifterRotateRight { + uint32_t value; + uint32_t carry; + + // Test a short rotate by one hex digit. + value = 0xabcd'1234; + shift(value, 4, carry); + XCTAssertEqual(value, 0x4abc'd123); + XCTAssertEqual(carry, 0); + + // Test a longer rotate, with carry. + value = 0xa5f9'6342; + shift(value, 17, carry); + XCTAssertEqual(value, 0xb1a1'52fc); + XCTAssertNotEqual(carry, 0); + + // Test a rotate by 32 without carry. + value = 0x385f'7dce; + shift(value, 32, carry); + XCTAssertEqual(value, 0x385f'7dce); + XCTAssertEqual(carry, 0); + + // Test a rotate by 32 with carry. + value = 0xfecd'ba12; + shift(value, 32, carry); + XCTAssertEqual(value, 0xfecd'ba12); + XCTAssertNotEqual(carry, 0); + + // Test a rotate through carry, carry not set. + value = 0x123f'abcf; + carry = 0; + shift(value, 0, carry); + XCTAssertEqual(value, 0x091f'd5e7); + XCTAssertNotEqual(carry, 0); + + // Test a rotate through carry, carry set. + value = 0x123f'abce; + carry = 1; + shift(value, 0, carry); + XCTAssertEqual(value, 0x891f'd5e7); + XCTAssertEqual(carry, 0); +} + @end