Take another run at shifts.

InstructionSets/ARM/BarrelShifter.hpp

@@ -30,7 +30,7 @@ template <> struct Carry<false> {
 /// 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 <ShiftType type, bool set_carry>
+template <ShiftType type, bool set_carry, bool is_immediate_shift>
 void shift(uint32_t &source, uint32_t amount, typename Carry<set_carry>::type carry) {
 	switch(type) {
 		case ShiftType::LogicalLeft:
@@ -47,48 +47,58 @@ void shift(uint32_t &source, uint32_t amount, typename Carry<set_carry>::type ca
 		break;
 
 		case ShiftType::LogicalRight:
+			if(!amount && is_immediate_shift) {
+				// An immediate logical shift right by '0' is treated as a shift by 32;
+				// assemblers are supposed to map LSR #0 to LSL #0.
+				amount = 32;
+			}
+
 			if(amount > 32) {
 				if constexpr (set_carry) carry = 0;
 				source = 0;
-			} else if(amount == 32 || !amount) {
+			} else if(amount == 32) {
-				// 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 {
+			} else if(amount > 0) {
 				if constexpr (set_carry) carry = source & (1 << (amount - 1));
 				source >>= amount;
 			}
 		break;
 
 		case ShiftType::ArithmeticRight: {
+			if(!amount && is_immediate_shift) {
+				// An immediate arithmetic shift of '0' is treated as a shift by 32.
+				amount = 32;
+			}
+
 			const uint32_t sign = (source & 0x8000'0000) ? 0xffff'ffff : 0x0000'0000;
 
 			if(amount >= 32) {
-				if constexpr (set_carry) carry = sign;
+				if constexpr (set_carry) carry = source & 0x8000'0000;
 				source = sign;
 			} else if(amount > 0) {
 				if constexpr (set_carry) carry = source & (1 << (amount - 1));
 				source = (source >> amount) | (sign << (32 - amount));
-			} else {
-				// As per logical right, an arithmetic shift of '0' is
-				// treated as a shift by 32.
-				if constexpr (set_carry) carry = source & 0x8000'0000;
-				source = sign;
 			}
 		} break;
 
 		case ShiftType::RotateRight: {
-			if(amount == 32) {
+			if(!amount) {
-				if constexpr (set_carry) carry = source & 0x8000'0000;
+				if(is_immediate_shift) {
-			} else if(amount == 0) {
+					// Immediate rotate right by 0 is treated as a rotate right by 1 through carry.
-				// Rotate right by 0 is treated as a rotate right by 1 through carry.
 					const uint32_t high = carry << 31;
 					if constexpr (set_carry) carry = source & 1;
 					source = (source >> 1) | high;
-			} else {
+				}
-				amount &= 31;
+				break;
+			}
+
+			// "ROR by 32 has result equal to Rm, carry out equal to bit 31 ...
+			// [for] ROR by n where n is greater than 32 ... repeatedly subtract 32 from n
+			// until the amount is in the range 1 to 32"
+			amount = ((amount - 1) & 31) + 1;
 			if constexpr (set_carry) carry = source & (1 << (amount - 1));
+			if(amount != 32) {
 				source = (source >> amount) | (source << (32 - amount));
 			}
 		} break;
@@ -100,20 +110,20 @@ void shift(uint32_t &source, uint32_t amount, typename Carry<set_carry>::type ca
 }
 
 /// Acts as per @c shift above, but applies runtime shift-type selection.
-template <bool set_carry>
+template <bool set_carry, bool is_immediate_shift>
 void shift(ShiftType type, uint32_t &source, uint32_t amount, typename Carry<set_carry>::type carry) {
 	switch(type) {
 		case ShiftType::LogicalLeft:
-			shift<ShiftType::LogicalLeft, set_carry>(source, amount, carry);
+			shift<ShiftType::LogicalLeft, set_carry, is_immediate_shift>(source, amount, carry);
 		break;
 		case ShiftType::LogicalRight:
-			shift<ShiftType::LogicalRight, set_carry>(source, amount, carry);
+			shift<ShiftType::LogicalRight, set_carry, is_immediate_shift>(source, amount, carry);
 		break;
 		case ShiftType::ArithmeticRight:
-			shift<ShiftType::ArithmeticRight, set_carry>(source, amount, carry);
+			shift<ShiftType::ArithmeticRight, set_carry, is_immediate_shift>(source, amount, carry);
 		break;
 		case ShiftType::RotateRight:
-			shift<ShiftType::RotateRight, set_carry>(source, amount, carry);
+			shift<ShiftType::RotateRight, set_carry, is_immediate_shift>(source, amount, carry);
 		break;
 	}
 }

InstructionSets/ARM/Executor.hpp

@@ -76,17 +76,12 @@ struct Executor {
 				// "The amount by which the register should be shifted may be contained in
 				// ... **the bottom byte** of another register".
 				shift_amount &= 0xff;
-
+				shift<set_carry, false>(fields.shift_type(), operand2, shift_amount, rotate_carry);
-				// A register shift amount of 0 has a different meaning than an in-instruction
-				// shift amount of 0.
-				if(shift_amount) {
-					shift<set_carry>(fields.shift_type(), operand2, shift_amount, rotate_carry);
-				}
 				return operand2;
 			}
 		}
 
-		shift<set_carry>(fields.shift_type(), operand2, fields.shift_amount(), rotate_carry);
+		shift<set_carry, true>(fields.shift_type(), operand2, fields.shift_amount(), rotate_carry);
 		return operand2;
 	}
 
@@ -118,10 +113,7 @@ struct Executor {
 		// Get operand 2.
 		if constexpr (flags.operand2_is_immediate()) {
 			operand2 = fields.immediate();
-			if(fields.rotate()) {
+			shift<ShiftType::RotateRight, shift_sets_carry, false>(operand2, fields.rotate(), rotate_carry);
-				shift<ShiftType::RotateRight, shift_sets_carry>(operand2, fields.rotate(), rotate_carry);
-			}
-			// Complete guess: carry is unaffected by an immediate value that doesn't rotate.
 		} else {
 			operand2 = decode_shift<true, shift_sets_carry>(fields, rotate_carry, shift_by_register ? 8 : 4);
 		}
@@ -361,7 +353,8 @@ struct Executor {
 		}
 
 		if constexpr (should_write_back) {
-			// Empirically: I think writeback occurs before the access, so shouldn't overwrite on a load.
+			// Empirically: I think order of operations for a load is: (i) write back; (ii) store value from bus.
+			// So if this is a load, don't allow write back to overwrite what was loaded.
 			if(flags.operation() == SingleDataTransferFlags::Operation::STR || transfer.base() != transfer.destination()) {
 				if(transfer.base() == 15) {
 					registers_.set_pc(offsetted_address);