Reimplement LDM and STM.

InstructionSets/ARM/Executor.hpp

@@ -366,32 +366,72 @@ struct Executor {
 	}
 	template <Flags f> void perform(BlockDataTransfer transfer) {
 		constexpr BlockDataTransferFlags flags(f);
+		constexpr bool is_ldm = flags.operation() == BlockDataTransferFlags::Operation::LDM;
 
-		// Grab a copy of the list of registers to transfer.
+		// Ensure that *base points to the base register if it can be written back;
-		const uint16_t list = transfer.register_list();
+		// also set address to the base.
+		uint32_t *base = nullptr;
+		uint32_t address;
+		if(transfer.base() == 15) {
+			address = registers_.pc_status(4);
+		} else {
+			base = &registers_[transfer.base()];
+			address = *base;
+		}
+
+		// For an LDM pc_proxy will receive any read R15 value;
+		// for an STM it'll hold the value to be written.
+		uint32_t pc_proxy = 0;
 
 		// Read the base address and take a copy in case a data abort means that
-		// it has to be restored later, and to write that value rather than
+		// it has to be restored later.
-		// the final address if the base register is first in the write-out list.
+		uint32_t initial_address = address;
-		uint32_t address = transfer.base() == 15 ?
-			registers_.pc_status(4) :
-			registers_[transfer.base()];
-		const uint32_t initial_address = address;
 
-		// Figure out what the final address will be, since that's what'll be
+		// Grab the register list and decide whether user registers are being used.
-		// in the output if the base register is second or beyond in the
+		const uint16_t list = transfer.register_list();
-		// write-out list.
+		const bool adopt_user_mode = flags.load_psr() && (!is_ldm || !(list & (1 << 15)));
+
+		// Write back will prima facie occur if:
+		//	(i) the instruction asks for it; and
+		//	(ii) the write-back register isn't R15.
+		bool write_back = base && flags.write_back_address();
+
+		// Collate a transfer list; this is a very long-winded way of implementing STM
+		// and LDM but right now the objective is merely correctness.
+		//
+		// If this is LDM and it turns out that base is also in the transfer list,
+		// disable write back.
+		uint32_t *transfer_sources[16];
+		uint32_t total = 0;
+		for(uint32_t c = 0; c < 15; c++) {
+			if(list & (1 << c)) {
+				uint32_t *const next = &registers_.reg(adopt_user_mode, c);
+				if(is_ldm && next == base) write_back = false;
+				transfer_sources[total++] = next;
+			}
+		}
+
+		// If the last thing in the list is R15, redirect it to the PC proxy,
+		// possibly populating with a meaningful value.
+		if(list & (1 << 15)) {
+			if(!is_ldm) {
+				pc_proxy = registers_.pc_status(8);
+			}
+			transfer_sources[total++] = &pc_proxy;
+		}
+
+		// If this is STM and the first thing in the list is the same as base,
+		// point it at initial_address instead.
+		if(!is_ldm && total && transfer_sources[0] == base) {
+			transfer_sources[0] = &initial_address;
+		}
+
+		// Calculate final_address, which is what will be written back if required;
+		// update address to point to the low end of the transfer block.
 		//
 		// Writes are always ordered from lowest address to highest; adjust the
 		// start address if this write is supposed to fill memory downward from
 		// the base.
-
-		// TODO: use std::popcount when adopting C++20.
-		uint32_t total = ((list & 0xaaaa) >> 1) + (list & 0x5555);
-		total = ((total & 0xcccc) >> 2) + (total & 0x3333);
-		total = ((total & 0xf0f0) >> 4) + (total & 0x0f0f);
-		total = ((total & 0xff00) >> 8) + (total & 0x00ff);
-
 		uint32_t final_address;
 		if constexpr (!flags.add_offset()) {
 			// Decrementing mode; final_address is the value the base register should
@@ -404,53 +444,37 @@ struct Executor {
 			final_address = address + total * 4;
 		}
 
-		// For loads, keep a record of the value replaced by the last load and
+		// Write back if enabled.
-		// where it came from. A data abort cancels both the current load and
+		if(write_back) {
-		// the one before it, so this is used by this implementation to undo
+			*base = final_address;
-		// the previous load in that case.
-		struct {
-			uint32_t *target = nullptr;
-			uint32_t value;
-		} last_replacement;
-
-		// Check whether access is forced ot the user bank; if so then switch
-		// to it now. Also keep track of the original mode to switch back at
-		// the end.
-		Mode original_mode = registers_.mode();
-		const bool adopt_user_mode =
-			flags.load_psr() && (
-				flags.operation() == BlockDataTransferFlags::Operation::STM ||
-				(
-					flags.operation() == BlockDataTransferFlags::Operation::LDM &&
-					!(list & (1 << 15))
-				)
-			);
-		if(adopt_user_mode) {
-			registers_.set_mode(Mode::User);
 		}
 
-		bool address_error = false;
+		// Update address in advance for:
+		//	* pre-indexed upward stores; and
+		//	* post-indxed downward stores.
+		if constexpr (flags.pre_index() == flags.add_offset()) {
+			address += 4;
+		}
+
+		// Perform all memory accesses, tracking whether either kind of abort will be
+		// required.
 		const bool trans = registers_.mode() == Mode::User;
-
+		const bool address_error = is_invalid_address(address);
-		// Keep track of whether all accesses succeeded in order potentially to
-		// throw a data abort later.
 		bool accesses_succeeded = true;
-		const auto access = [&](uint32_t &value) {
-			// Update address in advance for:
-			//	* pre-indexed upward stores; and
-			//	* post-indxed downward stores.
-			if constexpr (flags.pre_index() == flags.add_offset()) {
-				address += 4;
-			}
 
-			if constexpr (flags.operation() == BlockDataTransferFlags::Operation::STM) {
+		if constexpr (is_ldm) {
-				if(!address_error) {
+			// Keep a record of the value replaced by the last load and
-					// "If the abort occurs during a store multiple instruction, ARM takes little action until
+			// where it came from. A data abort cancels both the current load and
-					// the instruction completes, whereupon it enters the data abort trap. The memory manager is
+			// the one before it, so this might be used by this implementation to
-					// responsible for preventing erroneous writes to the memory."
+			// undo the previous load.
-					accesses_succeeded &= bus.template write<uint32_t>(address, value, registers_.mode(), trans);
+			struct {
-				}
+				uint32_t *target = nullptr;
-			} else {
+				uint32_t value;
+			} last_replacement;
+
+			for(uint32_t c = 0; c < total; c++) {
+				uint32_t &value = *transfer_sources[c];
+
 				// When ARM detects a data abort during a load multiple instruction, it modifies the operation of
 				// the instruction to ensure that recovery is possible.
 				//
@@ -466,18 +490,19 @@ struct Executor {
 					// address change.
 					if(accesses_succeeded) {
 						last_replacement.value = replaced;
-						last_replacement.target = &value;
+						last_replacement.target = transfer_sources[c];
 					} else {
 						if(last_replacement.target) {
 							*last_replacement.target = last_replacement.value;
 						}
 
-						// Also restore the base register.
+						// Also restore the base register, including to its original value
-						if(transfer.base() != 15) {
+						// if write back was disabled.
-							if constexpr (flags.write_back_address()) {
+						if(base) {
-								registers_[transfer.base()] = final_address;
+							if(write_back) {
+								*base = final_address;
 							} else {
-								registers_[transfer.base()] = initial_address;
+								*base = initial_address;
 							}
 						}
 					}
@@ -486,71 +511,43 @@ struct Executor {
 					uint32_t throwaway;
 					bus.template read<uint32_t>(address, throwaway, registers_.mode(), trans);
 				}
-			}
 
-			// Update address after the fact for:
+				// Advance.
-			//	* post-indexed upward stores; and
-			//	* pre-indxed downward stores.
-			if constexpr (flags.pre_index() != flags.add_offset()) {
 				address += 4;
 			}
-		};
+		} else {
+			for(uint32_t c = 0; c < total; c++) {
+				uint32_t &value = *transfer_sources[c];
 
-		// Check for an address exception.
+				if(!address_error) {
-		address_error = is_invalid_address(address);
+					// "If the abort occurs during a store multiple instruction, ARM takes little action until
-
+					// the instruction completes, whereupon it enters the data abort trap. The memory manager is
-		// Write out registers 1 to 14.
+					// responsible for preventing erroneous writes to the memory."
-		for(uint32_t c = 0; c < 15; c++) {
+					accesses_succeeded &= bus.template write<uint32_t>(address, value, registers_.mode(), trans);
-			if(list & (1 << c)) {
+				} else {
-				access(registers_[c]);
+					// Do a throwaway read.
-
+					uint32_t throwaway;
-				// Modify base register after each write if writeback is enabled.
+					bus.template read<uint32_t>(address, throwaway, registers_.mode(), trans);
-				// This'll ensure the unmodified value goes out if it was the
-				// first-selected register only.
-				if constexpr (flags.write_back_address()) {
-					if(transfer.base() != 15) {
-						registers_[transfer.base()] = final_address;
-					}
 				}
-			}
-		}
 
-		// Definitively write back, even if the earlier register list
+				// Advance.
-		// was empty.
+				address += 4;
-		if constexpr (flags.write_back_address()) {
-			if(transfer.base() != 15) {
-				registers_[transfer.base()] = final_address;
 			}
 		}
 
-		// Read or write the program counter as a special case if it was in the list.
-		if(list & (1 << 15)) {
-			uint32_t value;
-			if constexpr (flags.operation() == BlockDataTransferFlags::Operation::STM) {
-				value = registers_.pc_status(8);
-				access(value);
-			} else {
-				access(value);
-				registers_.set_pc(value);
-				if constexpr (flags.load_psr()) {
-					registers_.set_status(value);
-					original_mode = registers_.mode();	// Avoid switching back to the wrong mode
-														// in case user registers were exposed.
-				}
-			}
-		}
-
-		// If user mode was unnaturally forced, switch back to the actual
-		// current operating mode.
-		if(adopt_user_mode) {
-			registers_.set_mode(original_mode);
-		}
-
 		// Finally throw an exception if necessary.
 		if(address_error) {
 			registers_.exception<Registers::Exception::Address>();
 		} else if(!accesses_succeeded) {
 			registers_.exception<Registers::Exception::DataAbort>();
+		} else {
+			// If this was an LDM to R15 then apply it appropriately.
+			if(is_ldm && list & (1 << 15)) {
+				registers_.set_pc(pc_proxy);
+				if constexpr (flags.load_psr()) {
+					registers_.set_status(pc_proxy);
+				}
+			}
 		}
 	}
 

InstructionSets/ARM/OperationMapper.hpp

@@ -294,6 +294,17 @@ struct BlockDataTransfer: public WithShiftControlBits {
 
 	/// A bitfield indicating which registers to load or store.
 	uint16_t register_list() const		{	return static_cast<uint16_t>(opcode_);	}
+	uint32_t popcount() const {
+		const uint16_t list = register_list();
+
+		// TODO: just use std::popcount when adopting C++20.
+		uint32_t total = ((list & 0xaaaa) >> 1) + (list & 0x5555);
+		total = ((total & 0xcccc) >> 2) + (total & 0x3333);
+		total = ((total & 0xf0f0) >> 4) + (total & 0x0f0f);
+		total = ((total & 0xff00) >> 8) + (total & 0x00ff);
+
+		return total;
+	}
 };
 
 //

InstructionSets/ARM/Registers.hpp

@@ -345,9 +345,8 @@ struct Registers {
 		/// @returns A reference to the register at @c offset. If @c force_user_mode is true,
 		/// this will the the user-mode register. Otherwise it'll be that for the current mode. These references
 		/// are guaranteed to remain valid only until the next mode change.
-		template <bool force_user_mode>
+		uint32_t &reg(bool force_user_mode, uint32_t offset) {
-		uint32_t &reg(uint32_t offset) {
+			if(!force_user_mode) {
-			if constexpr (!force_user_mode) {
 				return active_[offset];
 			}
 

Machines/Acorn/Archimedes/Archimedes.cpp

@@ -76,19 +76,18 @@ struct HackyDebugger {
 //				last_r1 = executor_.registers()[1];
 //			}
 
-//			if(executor_.pc() == 0x03801ed8 || (executor_.registers()[9] == 0x00ff'0000 && executor_.registers()[9] != last_r9)) {
+			if(instruction == 0xe8fd7fff) {//0x5f706f74) {
-//				printf("At %08x; after last PC %08x and %zu ago was %08x; r9 is %08x [%d]\n",
+				printf("At %08x [%d]; after last PC %08x and %zu ago was %08x\n",
-//					executor_.pc(),
+					address,
-//					pc_history[(pc_history_ptr - 2 + pc_history.size()) % pc_history.size()],
+					instr_count,
-//					pc_history.size(),
+					pc_history[(pc_history_ptr - 2 + pc_history.size()) % pc_history.size()],
-//					pc_history[pc_history_ptr],
+					pc_history.size(),
-//					executor_.registers()[9],
+					pc_history[pc_history_ptr]);
-//					executor_.registers()[9] != last_r9);
+			}
-//			}
 //			last_r9 = executor_.registers()[9];
 
-//			log |= executor_.pc() == 0x03801ebc;
+//			log |= address == 0x038031c4;
-//			log |= instr_count == 72766815;
+//			log |= instr_count == 53552731 - 30;
 //			log &= executor_.pc() != 0x000000a0;
 
 //			log = (executor_.pc() == 0x038162afc) || (executor_.pc() == 0x03824b00);

OSBindings/Mac/Clock SignalTests/ARMDecoderTests.mm

@@ -90,19 +90,19 @@ struct MemoryLedger {
 
 	// Test a shift by 1 into carry.
 	value = 0x8000'0000;
-	shift<ShiftType::LogicalLeft, true>(value, 1, carry);
+	shift<ShiftType::LogicalLeft, true, true>(value, 1, carry);
 	XCTAssertEqual(value, 0);
 	XCTAssertNotEqual(carry, 0);
 
 	// Test a shift by 18 into carry.
 	value = 0x0000'4001;
-	shift<ShiftType::LogicalLeft, true>(value, 18, carry);
+	shift<ShiftType::LogicalLeft, true, true>(value, 18, carry);
 	XCTAssertEqual(value, 0x4'0000);
 	XCTAssertNotEqual(carry, 0);
 
 	// Test a shift by 17, not generating carry.
 	value = 0x0000'4001;
-	shift<ShiftType::LogicalLeft, true>(value, 17, carry);
+	shift<ShiftType::LogicalLeft, true, true>(value, 17, carry);
 	XCTAssertEqual(value, 0x8002'0000);
 	XCTAssertEqual(carry, 0);
 }
@@ -113,40 +113,40 @@ struct MemoryLedger {
 
 	// Test successive shifts by 4; one generating carry and one not.
 	value = 0x12345678;
-	shift<ShiftType::LogicalRight, true>(value, 4, carry);
+	shift<ShiftType::LogicalRight, true, true>(value, 4, carry);
 	XCTAssertEqual(value, 0x1234567);
 	XCTAssertNotEqual(carry, 0);
 
-	shift<ShiftType::LogicalRight, true>(value, 4, carry);
+	shift<ShiftType::LogicalRight, true, true>(value, 4, carry);
 	XCTAssertEqual(value, 0x123456);
 	XCTAssertEqual(carry, 0);
 
 	// Test shift by 1.
 	value = 0x8003001;
-	shift<ShiftType::LogicalRight, true>(value, 1, carry);
+	shift<ShiftType::LogicalRight, true, true>(value, 1, carry);
 	XCTAssertEqual(value, 0x4001800);
 	XCTAssertNotEqual(carry, 0);
 
 	// Test a shift by greater than 32.
 	value = 0xffff'ffff;
-	shift<ShiftType::LogicalRight, true>(value, 33, carry);
+	shift<ShiftType::LogicalRight, true, true>(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<ShiftType::LogicalRight, true>(value, 32, carry);
+	shift<ShiftType::LogicalRight, true, true>(value, 32, carry);
 	XCTAssertEqual(value, 0);
 	XCTAssertNotEqual(carry, 0);
 
 	value = 0x7fff'ffff;
-	shift<ShiftType::LogicalRight, true>(value, 32, carry);
+	shift<ShiftType::LogicalRight, true, true>(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<ShiftType::LogicalRight, true>(value, 0, carry);
+	shift<ShiftType::LogicalRight, true, true>(value, 0, carry);
 	XCTAssertEqual(value, 0);
 	XCTAssertNotEqual(carry, 0);
 }
@@ -157,31 +157,31 @@ struct MemoryLedger {
 
 	// Test a short negative shift.
 	value = 0x8000'0030;
-	shift<ShiftType::ArithmeticRight, true>(value, 1, carry);
+	shift<ShiftType::ArithmeticRight, true, true>(value, 1, carry);
 	XCTAssertEqual(value, 0xc000'0018);
 	XCTAssertEqual(carry, 0);
 
 	// Test a medium negative shift without carry.
 	value = 0xffff'0000;
-	shift<ShiftType::ArithmeticRight, true>(value, 11, carry);
+	shift<ShiftType::ArithmeticRight, true, true>(value, 11, carry);
 	XCTAssertEqual(value, 0xffff'ffe0);
 	XCTAssertEqual(carry, 0);
 
 	// Test a medium negative shift with carry.
 	value = 0xffc0'0000;
-	shift<ShiftType::ArithmeticRight, true>(value, 23, carry);
+	shift<ShiftType::ArithmeticRight, true, true>(value, 23, carry);
 	XCTAssertEqual(value, 0xffff'ffff);
 	XCTAssertNotEqual(carry, 0);
 
 	// Test a long negative shift.
 	value = 0x8000'0000;
-	shift<ShiftType::ArithmeticRight, true>(value, 32, carry);
+	shift<ShiftType::ArithmeticRight, true, true>(value, 32, carry);
 	XCTAssertEqual(value, 0xffff'ffff);
 	XCTAssertNotEqual(carry, 0);
 
 	// Test a positive shift.
 	value = 0x0123'0031;
-	shift<ShiftType::ArithmeticRight, true>(value, 3, carry);
+	shift<ShiftType::ArithmeticRight, true, true>(value, 3, carry);
 	XCTAssertEqual(value, 0x24'6006);
 	XCTAssertEqual(carry, 0);
 }
@@ -192,39 +192,39 @@ struct MemoryLedger {
 
 	// Test a short rotate by one hex digit.
 	value = 0xabcd'1234;
-	shift<ShiftType::RotateRight, true>(value, 4, carry);
+	shift<ShiftType::RotateRight, true, true>(value, 4, carry);
 	XCTAssertEqual(value, 0x4abc'd123);
 	XCTAssertEqual(carry, 0);
 
 	// Test a longer rotate, with carry.
 	value = 0xa5f9'6342;
-	shift<ShiftType::RotateRight, true>(value, 17, carry);
+	shift<ShiftType::RotateRight, true, true>(value, 17, carry);
 	XCTAssertEqual(value, 0xb1a1'52fc);
 	XCTAssertNotEqual(carry, 0);
 
 	// Test a rotate by 32 without carry.
 	value = 0x385f'7dce;
-	shift<ShiftType::RotateRight, true>(value, 32, carry);
+	shift<ShiftType::RotateRight, true, true>(value, 32, carry);
 	XCTAssertEqual(value, 0x385f'7dce);
 	XCTAssertEqual(carry, 0);
 
 	// Test a rotate by 32 with carry.
 	value = 0xfecd'ba12;
-	shift<ShiftType::RotateRight, true>(value, 32, carry);
+	shift<ShiftType::RotateRight, true, true>(value, 32, carry);
 	XCTAssertEqual(value, 0xfecd'ba12);
 	XCTAssertNotEqual(carry, 0);
 
 	// Test a rotate through carry, carry not set.
 	value = 0x123f'abcf;
 	carry = 0;
-	shift<ShiftType::RotateRight, true>(value, 0, carry);
+	shift<ShiftType::RotateRight, true, true>(value, 0, carry);
 	XCTAssertEqual(value, 0x091f'd5e7);
 	XCTAssertNotEqual(carry, 0);
 
 	// Test a rotate through carry, carry set.
 	value = 0x123f'abce;
 	carry = 1;
-	shift<ShiftType::RotateRight, true>(value, 0, carry);
+	shift<ShiftType::RotateRight, true, true>(value, 0, carry);
 	XCTAssertEqual(value, 0x891f'd5e7);
 	XCTAssertEqual(carry, 0);
 }