From 11c4d2f09e96293bf47039980aa720fc8e7a4279 Mon Sep 17 00:00:00 2001
From: Thomas Harte <thomas.harte@gmail.com>
Date: Sun, 3 Mar 2024 21:38:27 -0500
Subject: [PATCH] Add further exposition.

---
 InstructionSets/ARM/Executor.hpp              |  9 +++--
 InstructionSets/ARM/Registers.hpp             | 12 +++++-
 .../Mac/Clock SignalTests/ARMDecoderTests.mm  | 40 +++++++++----------
 3 files changed, 35 insertions(+), 26 deletions(-)

diff --git a/InstructionSets/ARM/Executor.hpp b/InstructionSets/ARM/Executor.hpp
index 9c20adb93..0650371b2 100644
--- a/InstructionSets/ARM/Executor.hpp
+++ b/InstructionSets/ARM/Executor.hpp
@@ -544,8 +544,7 @@ struct Executor {
 		registers_.exception<Registers::Exception::UndefinedInstruction>();
 	}
 
-	MemoryT bus;
-
+	/// @returns The current registers state.
 	const Registers &registers() const {
 		return registers_;
 	}
@@ -563,12 +562,14 @@ struct Executor {
 		return registers_.pc(0);
 	}
 
+	MemoryT bus;
+
 private:
 	Registers registers_;
 };
 
-/// Provides an analogue of the @c OperationMapper -affiliated @c dispatch that also updates the
-/// executor's program counter appropriately.
+/// Executes the instruction @c instruction which should have been fetched from @c executor.pc(),
+/// modifying @c executor.
 template <Model model, typename MemoryT>
 void execute(uint32_t instruction, Executor<model, MemoryT> &executor) {
 	executor.set_pc(executor.pc() + 4);
diff --git a/InstructionSets/ARM/Registers.hpp b/InstructionSets/ARM/Registers.hpp
index 91589a472..a39c39228 100644
--- a/InstructionSets/ARM/Registers.hpp
+++ b/InstructionSets/ARM/Registers.hpp
@@ -66,6 +66,7 @@ struct Registers {
 			overflow_flag_ = value;
 		}
 
+		/// @returns The current status bits, separate from the PC — mode, NVCZ and the two interrupt flags.
 		uint32_t status() const {
 			return
 				uint32_t(mode_) |
@@ -100,6 +101,7 @@ struct Registers {
 			}
 		}
 
+		/// @returns The current mode.
 		Mode mode() const {
 			return mode_;
 		}
@@ -109,6 +111,7 @@ struct Registers {
 			active[15] = value & ConditionCode::Address;
 		}
 
+		/// @returns The stored PC plus @c offset limited to 26 bits.
 		uint32_t pc(uint32_t offset) const {
 			return (active[15] + offset) & ConditionCode::Address;
 		}
@@ -136,6 +139,7 @@ struct Registers {
 			FIQ = 0x1c,
 		};
 
+		/// Updates the program counter, interupt flags and link register if applicable to begin @c exception.
 		template <Exception exception>
 		void exception() {
 			interrupt_flags_ |= ConditionCode::IRQDisable;
@@ -163,6 +167,7 @@ struct Registers {
 
 		// MARK: - Condition tests.
 
+		/// @returns @c true if @c condition tests as true; @c false otherwise.
 		bool test(Condition condition) {
 			const auto ne = [&]() -> bool {
 				return zero_result_;
@@ -208,8 +213,7 @@ struct Registers {
 			}
 		}
 
-		std::array<uint32_t, 16> active{};
-
+		/// Sets current execution mode.
 		void set_mode(Mode target_mode) {
 			if(mode_ == target_mode) {
 				return;
@@ -258,6 +262,10 @@ struct Registers {
 			mode_ = target_mode;
 		}
 
+		/// The active register set. TODO: switch to an implementation of operator[], hiding the
+		/// current implementation decision to maintain this as a linear block of memory.
+		std::array<uint32_t, 16> active{};
+
 	private:
 		Mode mode_ = Mode::Supervisor;
 
diff --git a/OSBindings/Mac/Clock SignalTests/ARMDecoderTests.mm b/OSBindings/Mac/Clock SignalTests/ARMDecoderTests.mm
index cb1d4030c..576979694 100644
--- a/OSBindings/Mac/Clock SignalTests/ARMDecoderTests.mm	
+++ b/OSBindings/Mac/Clock SignalTests/ARMDecoderTests.mm	
@@ -211,25 +211,25 @@ struct Memory {
 }
 
 // TODO: turn the below into a trace-driven test case.
-- (void)testROM319 {
-	constexpr ROM::Name rom_name = ROM::Name::AcornRISCOS319;
-	ROM::Request request(rom_name);
-	const auto roms = CSROMFetcher()(request);
-
-	auto executor = std::make_unique<Executor<Model::ARMv2, Memory>>();
-	executor->bus.rom = roms.find(rom_name)->second;
-
-	for(int c = 0; c < 1000; c++) {
-		uint32_t instruction;
-		executor->bus.read(executor->pc(), instruction, executor->registers().mode(), false);
-
-		printf("%08x: %08x [", executor->pc(), instruction);
-		for(int c = 0; c < 15; c++) {
-			printf("r%d:%08x ", c, executor->registers().active[c]);
-		}
-		printf("psr:%08x]\n", executor->registers().status());
-		execute<Model::ARMv2>(instruction, *executor);
-	}
-}
+//- (void)testROM319 {
+//	constexpr ROM::Name rom_name = ROM::Name::AcornRISCOS319;
+//	ROM::Request request(rom_name);
+//	const auto roms = CSROMFetcher()(request);
+//
+//	auto executor = std::make_unique<Executor<Model::ARMv2, Memory>>();
+//	executor->bus.rom = roms.find(rom_name)->second;
+//
+//	for(int c = 0; c < 1000; c++) {
+//		uint32_t instruction;
+//		executor->bus.read(executor->pc(), instruction, executor->registers().mode(), false);
+//
+//		printf("%08x: %08x [", executor->pc(), instruction);
+//		for(int c = 0; c < 15; c++) {
+//			printf("r%d:%08x ", c, executor->registers().active[c]);
+//		}
+//		printf("psr:%08x]\n", executor->registers().status());
+//		execute<Model::ARMv2>(instruction, *executor);
+//	}
+//}
 
 @end