Relocate and clarify barrel shifts.

With a view to independent testing.

InstructionSets/ARM/BarrelShifter.hpp

@@ -0,0 +1,122 @@
+//
+//  BarrelShifter.hpp
+//  Clock Signal
+//
+//  Created by Thomas Harte on 28/02/2024.
+//  Copyright © 2024 Thomas Harte. All rights reserved.
+//
+
+#pragma once
+
+namespace InstructionSet::ARM {
+
+enum class ShiftType {
+	LogicalLeft = 0b00,
+	LogicalRight = 0b01,
+	ArithmeticRight = 0b10,
+	RotateRight = 0b11,
+};
+
+template <bool writeable> struct Carry;
+template <> struct Carry<true> {
+	using type = uint32_t &;
+};
+template <> struct Carry<false> {
+	using type = uint32_t;
+};
+
+/// 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.
+template <ShiftType type, bool set_carry>
+void shift(uint32_t &source, uint32_t amount, typename Carry<set_carry>::type carry) {
+	switch(type) {
+		case ShiftType::LogicalLeft:
+			if(amount > 32) {
+				if constexpr (set_carry) carry = 0;
+				source = 0;
+			} else if(amount == 32) {
+				if constexpr (set_carry) carry = source & 1;
+				source = 0;
+			} else if(amount > 0) {
+				if constexpr (set_carry) carry = source & (0x8000'0000 >> (amount - 1));
+				source <<= amount;
+			}
+		break;
+
+		case ShiftType::LogicalRight:
+			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 {
+				// 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;
+			}
+		break;
+
+		case ShiftType::ArithmeticRight: {
+			const uint32_t sign = (source & 0x8000'0000) ? 0xffff'ffff : 0x0000'0000;
+
+			if(amount >= 32) {
+				if constexpr (set_carry) carry = sign;
+				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 constexpr (set_carry) carry = source & 0x8000'0000;
+			} else if(amount == 0) {
+				// 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;
+				if constexpr (set_carry) carry = source & (1 << (amount - 1));
+				source = (source >> amount) | (source << (32 - amount));
+			}
+		} break;
+
+		// TODO: upon adoption of C++20, use std::rotr.
+
+		default: break;
+	}
+}
+
+/// Acts as per @c shift above, but applies runtime shift-type selection.
+template <bool set_carry>
+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);
+		break;
+		case ShiftType::LogicalRight:
+			shift<ShiftType::LogicalRight, set_carry>(source, amount, carry);
+		break;
+		case ShiftType::ArithmeticRight:
+			shift<ShiftType::ArithmeticRight, set_carry>(source, amount, carry);
+		break;
+		case ShiftType::RotateRight:
+			shift<ShiftType::RotateRight, set_carry>(source, amount, carry);
+		break;
+	}
+}
+
+}

InstructionSets/ARM/OperationMapper.hpp

@@ -9,6 +9,7 @@
 #pragma once
 
 #include "../../Reflection/Dispatcher.hpp"
+#include "BarrelShifter.hpp"
 
 namespace InstructionSet::ARM {
 
@@ -96,13 +97,6 @@ enum class Condition {
 	GT,	LE,	AL,	NV,
 };
 
-enum class ShiftType {
-	LogicalLeft = 0b00,
-	LogicalRight = 0b01,
-	ArithmeticRight = 0b10,
-	RotateRight = 0b11,
-};
-
 //
 // Implementation details.
 //

OSBindings/Mac/Clock Signal.xcodeproj/project.pbxproj

@@ -1336,6 +1336,7 @@
 		4B2005402B804AA300420C5C /* OperationMapper.hpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.h; path = OperationMapper.hpp; sourceTree = "<group>"; };
 		4B2005422B804D6400420C5C /* ARMDecoderTests.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = ARMDecoderTests.mm; sourceTree = "<group>"; };
 		4B2005462B8BD7A500420C5C /* Registers.hpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.h; path = Registers.hpp; sourceTree = "<group>"; };
+		4B2005472B8FB13D00420C5C /* BarrelShifter.hpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.h; path = BarrelShifter.hpp; sourceTree = "<group>"; };
 		4B2130E0273A7A0A008A77B4 /* Audio.cpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; path = Audio.cpp; sourceTree = "<group>"; };
 		4B2130E1273A7A0A008A77B4 /* Audio.hpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.h; path = Audio.hpp; sourceTree = "<group>"; };
 		4B228CD424D773B30077EF25 /* CSScanTarget.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = CSScanTarget.mm; sourceTree = "<group>"; };
@@ -2759,6 +2760,7 @@
 		4B20053D2B804A4F00420C5C /* ARM */ = {
 			isa = PBXGroup;
 			children = (
+				4B2005472B8FB13D00420C5C /* BarrelShifter.hpp */,
 				4B2005402B804AA300420C5C /* OperationMapper.hpp */,
 				4B2005462B8BD7A500420C5C /* Registers.hpp */,
 			);

OSBindings/Mac/Clock SignalTests/ARMDecoderTests.mm

@@ -8,6 +8,7 @@
 
 #import <XCTest/XCTest.h>
 
+#include "../../../InstructionSets/ARM/BarrelShifter.hpp"
 #include "../../../InstructionSets/ARM/OperationMapper.hpp"
 #include "../../../InstructionSets/ARM/Registers.hpp"
 #include "../../../Numeric/Carry.hpp"
@@ -16,94 +17,6 @@ using namespace InstructionSet::ARM;
 
 namespace {
 
-template <ShiftType type, bool set_carry>
-void shift(uint32_t &source, uint32_t amount, uint32_t *carry = nullptr) {
-	switch(type) {
-		case ShiftType::LogicalLeft:
-			if(amount > 32) {
-				if constexpr (set_carry) *carry = 0;
-				source = 0;
-			} else if(amount == 32) {
-				if constexpr (set_carry) *carry = source & 1;
-				source = 0;
-			} else if(amount > 0) {
-				if constexpr (set_carry) *carry = source & (0x8000'0000 >> (amount - 1));
-				source <<= amount;
-			}
-		break;
-
-		case ShiftType::LogicalRight:
-			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 {
-				// 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;
-			}
-		break;
-
-		case ShiftType::ArithmeticRight: {
-			const uint32_t sign = (source & 0x8000'0000) ? 0xffff'ffff : 0x0000'0000;
-
-			if(amount >= 32) {
-				if constexpr (set_carry) *carry = sign;
-				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 constexpr (set_carry) *carry = source & 0x8000'0000;
-			} else if(amount == 0) {
-				// 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;
-				if constexpr (set_carry) *carry = source & (1 << (amount - 1));
-				source = (source >> amount) | (source << (32 - amount));
-			}
-		} break;
-
-		default: break;
-	}
-}
-
-template <bool set_carry>
-void shift(ShiftType type, uint32_t &source, uint32_t amount, uint32_t *carry) {
-	switch(type) {
-		case ShiftType::LogicalLeft:
-			shift<ShiftType::LogicalLeft, set_carry>(source, amount, carry);
-		break;
-		case ShiftType::LogicalRight:
-			shift<ShiftType::LogicalRight, set_carry>(source, amount, carry);
-		break;
-		case ShiftType::ArithmeticRight:
-			shift<ShiftType::ArithmeticRight, set_carry>(source, amount, carry);
-		break;
-		case ShiftType::RotateRight:
-			shift<ShiftType::RotateRight, set_carry>(source, amount, carry);
-		break;
-	}
-}
-
 struct Scheduler {
 	bool should_schedule(Condition condition) {
 		return registers_.test(condition);
@@ -138,7 +51,7 @@ struct Scheduler {
 		if constexpr (flags.operand2_is_immediate()) {
 			operand2 = fields.immediate();
 			if(fields.rotate()) {
-				shift<ShiftType::RotateRight, shift_sets_carry>(operand2, fields.rotate(), &rotate_carry);
+				shift<ShiftType::RotateRight, shift_sets_carry>(operand2, fields.rotate(), rotate_carry);
 			}
 		} else {
 			uint32_t shift_amount;
@@ -167,7 +80,7 @@ struct Scheduler {
 			} else {
 				operand2 = registers_.active[fields.operand2()];
 			}
-			shift<shift_sets_carry>(fields.shift_type(), operand2, shift_amount, &rotate_carry);
+			shift<shift_sets_carry>(fields.shift_type(), operand2, shift_amount, rotate_carry);
 		}
 
 		// Perform the data processing operation.