Merge pull request #1351 from TomHarte/PositiveExpression

Express offset test as positive logic.

InstructionSets/ARM/Executor.hpp

@@ -248,14 +248,14 @@ struct Executor {
 
 		// Calculate offset.
 		uint32_t offset;
-		if constexpr (flags.offset_is_immediate()) {
+		if constexpr (flags.offset_is_register()) {
-			offset = transfer.immediate();
-		} else {
 			// The 8 shift control bits are described in 6.2.3, but
 			// the register specified shift amounts are not available
 			// in this instruction class.
 			uint32_t carry = registers_.c();
 			offset = decode_shift<false, false>(transfer, carry, 4);
+		} else {
+			offset = transfer.immediate();
 		}
 
 		// Obtain base address.
@@ -544,7 +544,10 @@ struct Executor {
 		registers_.exception<Registers::Exception::UndefinedInstruction>();
 	}
 
-	MemoryT bus;
+	/// @returns The current registers state.
+	const Registers &registers() const {
+		return registers_;
+	}
 
 	/// Sets the expected address of the instruction after whichever  is about to be executed.
 	/// So it's PC+4 compared to most other systems.
@@ -559,17 +562,14 @@ struct Executor {
 		return registers_.pc(0);
 	}
 
-	/// @returns The current processor mode.
+	MemoryT bus;
-	Mode mode() const {
-		return registers_.mode();
-	}
 
 private:
 	Registers registers_;
 };
 
-/// Provides an analogue of the @c OperationMapper -affiliated @c dispatch that also updates the
+/// Executes the instruction @c instruction which should have been fetched from @c executor.pc(),
-/// executor's program counter appropriately.
+/// modifying @c executor.
 template <Model model, typename MemoryT>
 void execute(uint32_t instruction, Executor<model, MemoryT> &executor) {
 	executor.set_pc(executor.pc() + 4);

InstructionSets/ARM/OperationMapper.hpp

@@ -232,7 +232,7 @@ struct SingleDataTransferFlags {
 		return flag_bit<20>(flags_) ? Operation::LDR : Operation::STR;
 	}
 
-	constexpr bool offset_is_immediate() const	{	return !flag_bit<25>(flags_);	}
+	constexpr bool offset_is_register() const	{	return flag_bit<25>(flags_);	}
 	constexpr bool pre_index() const			{	return flag_bit<24>(flags_);	}
 	constexpr bool add_offset() const			{	return flag_bit<23>(flags_);	}
 	constexpr bool transfer_byte() const		{	return flag_bit<22>(flags_);	}
@@ -254,7 +254,7 @@ struct SingleDataTransfer: public WithShiftControlBits {
 	/// The base register index. i.e. 'Rn'.
 	int base() const					{	return (opcode_ >> 16) & 0xf;	}
 
-	/// The immediate offset, if @c offset_is_immediate() was @c true; meaningless otherwise.
+	/// The immediate offset, if @c offset_is_register() was @c false; meaningless otherwise.
 	int immediate() const				{	return opcode_ & 0xfff;			}
 };
 

InstructionSets/ARM/Registers.hpp

@@ -66,11 +66,10 @@ struct Registers {
 			overflow_flag_ = value;
 		}
 
-		/// @returns The full PC + status bits.
+		/// @returns The current status bits, separate from the PC — mode, NVCZ and the two interrupt flags.
-		uint32_t pc_status(uint32_t offset) const {
+		uint32_t status() const {
 			return
 				uint32_t(mode_) |
-				((active[15] + offset) & ConditionCode::Address) |
 				(negative_flag_ & ConditionCode::Negative) |
 				(zero_result_ ? 0 : ConditionCode::Zero) |
 				(carry_flag_ ? ConditionCode::Carry : 0) |
@@ -78,6 +77,13 @@ struct Registers {
 				interrupt_flags_;
 		}
 
+		/// @returns The full PC + status bits.
+		uint32_t pc_status(uint32_t offset) const {
+			return
+				((active[15] + offset) & ConditionCode::Address) |
+				status();
+		}
+
 		/// Sets status bits only, subject to mode.
 		void set_status(uint32_t status) {
 			// ... in user mode the other flags (I, F, M1, M0) are protected from direct change
@@ -95,16 +101,17 @@ struct Registers {
 			}
 		}
 
+		/// @returns The current mode.
 		Mode mode() const {
 			return mode_;
 		}
 
 		/// Sets a new PC.
-		/// TODO: communicate this onward.
 		void set_pc(uint32_t value) {
 			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;
 		}
@@ -132,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;
@@ -159,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_;
@@ -204,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;
@@ -254,20 +262,24 @@ 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;
 
-		uint32_t zero_result_ = 0;
+		uint32_t zero_result_ = 1;
 		uint32_t negative_flag_ = 0;
-		uint32_t interrupt_flags_ = 0;
+		uint32_t interrupt_flags_ = ConditionCode::IRQDisable | ConditionCode::FIQDisable;
 		uint32_t carry_flag_ = 0;
 		uint32_t overflow_flag_ = 0;
 
 		// Various shadow registers.
-		std::array<uint32_t, 7> user_registers_;
+		std::array<uint32_t, 7> user_registers_{};
-		std::array<uint32_t, 7> fiq_registers_;
+		std::array<uint32_t, 7> fiq_registers_{};
-		std::array<uint32_t, 2> irq_registers_;
+		std::array<uint32_t, 2> irq_registers_{};
-		std::array<uint32_t, 2> supervisor_registers_;
+		std::array<uint32_t, 2> supervisor_registers_{};
 };
 
 }

OSBindings/Mac/Clock SignalTests/ARMDecoderTests.mm

@@ -20,33 +20,42 @@ struct Memory {
 
 	template <typename IntT>
 	bool write(uint32_t address, IntT source, Mode mode, bool trans) {
-		(void)address;
-		(void)source;
 		(void)mode;
 		(void)trans;
+
 		printf("W of %08x to %08x [%lu]\n", source, address, sizeof(IntT));
+
+		if(has_moved_rom_ && address < ram_.size()) {
+			*reinterpret_cast<IntT *>(&ram_[address]) = source;
+		}
+
 		return true;
 	}
 
 	template <typename IntT>
 	bool read(uint32_t address, IntT &source, Mode mode, bool trans) {
-		if(address > 0x3800000) {
+		(void)mode;
+		(void)trans;
+
+		if(address >= 0x3800000) {
 			has_moved_rom_ = true;
 			source = *reinterpret_cast<const IntT *>(&rom[address - 0x3800000]);
 		} else if(!has_moved_rom_) {
 			// TODO: this is true only very transiently.
 			source = *reinterpret_cast<const IntT *>(&rom[address]);
+		} else if(address < ram_.size()) {
+			source = *reinterpret_cast<const IntT *>(&ram_[address]);
 		} else {
+			source = 0;
 			printf("Unknown read from %08x [%lu]\n", address, sizeof(IntT));
 		}
 
-		(void)mode;
-		(void)trans;
 		return true;
 	}
 
 	private:
 		bool has_moved_rom_ = false;
+		std::array<uint8_t, 4*1024*1024> ram_{};
 };
 
 }
@@ -202,21 +211,25 @@ struct Memory {
 }
 
 // TODO: turn the below into a trace-driven test case.
-/*- (void)testROM319 {
+//- (void)testROM319 {
-	constexpr ROM::Name rom_name = ROM::Name::AcornRISCOS319;
+//	constexpr ROM::Name rom_name = ROM::Name::AcornRISCOS319;
-	ROM::Request request(rom_name);
+//	ROM::Request request(rom_name);
-	const auto roms = CSROMFetcher()(request);
+//	const auto roms = CSROMFetcher()(request);
-
+//
-	Executor<Model::ARMv2, Memory> executor;
+//	auto executor = std::make_unique<Executor<Model::ARMv2, Memory>>();
-	executor.bus.rom = roms.find(rom_name)->second;
+//	executor->bus.rom = roms.find(rom_name)->second;
-
+//
-	for(int c = 0; c < 1000; c++) {
+//	for(int c = 0; c < 1000; c++) {
-		uint32_t instruction;
+//		uint32_t instruction;
-		executor.bus.read(executor.pc(), instruction, executor.mode(), false);
+//		executor->bus.read(executor->pc(), instruction, executor->registers().mode(), false);
-
+//
-		printf("%08x: %08x\n", executor.pc(), instruction);
+//		printf("%08x: %08x [", executor->pc(), instruction);
-		execute<Model::ARMv2>(instruction, executor);
+//		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