Attempt a complete block data transfer.

OSBindings/Mac/Clock SignalTests/ARMDecoderTests.mm

@@ -360,76 +360,50 @@ struct Scheduler {
 	template <Flags f> void perform(BlockDataTransfer transfer) {
 		constexpr BlockDataTransferFlags flags(f);
 
-		// TODO: inclusion of the base in the register list.
+		// Grab a copy of the list of registers to transfer.
+		const uint16_t list = transfer.register_list();
 
+		// 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
+		// the final address if the base register is first in the write-out list.
 		uint32_t address = transfer.base() == 15 ?
 			registers_.pc_status(8) :
 			registers_.active[transfer.base()];
-
 		const uint32_t initial_address = address;
-		const uint16_t list = transfer.register_list();
 
+		// Figure out what the final address will be, since that's what'll be
+		// in the output if the base register is second or beyond in the
+		// write-out list.
+		//
+		// 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 & 0xa) >> 1) + (list & 0x5);
+		total = ((list & 0xc) >> 2) + (list & 0x3);
+
+		uint32_t final_address;
+		if constexpr (!flags.add_offset()) {
+			final_address = address + total * 4;
+			address = final_address;
+		} else {
+			final_address = address + total * 4;
+		}
+
+		// For loads, keep a record of the value replaced by the last load and
+		// where it came from. A data abort cancels both the current load and
+		// the one before it, so this is used by this implementation to undo
+		// the previous load in that case.
 		struct {
 			uint32_t *target = nullptr;
 			uint32_t value;
 		} last_replacement;
 
-		// Writes are always from lowest address to highest; asking for storage downward
+		// Check whether access is forced ot the user bank; if so then switch
-		// just results in predecrementation of the address.
+		// to it now. Also keep track of the original mode to switch back at
-		if constexpr (!flags.add_offset()) {
+		// the end.
-			uint32_t total = ((list & 0xa) >> 1) + (list & 0x5);
+		const Mode original_mode = registers_.mode();
-			total = ((list & 0xc) >> 2) + (list & 0x3);
-			address -= total * 4;
-		}
-
-		[[maybe_unused]] uint32_t final_address = address;
-
-		bool visits_succeeded = true;
-		const auto visit = [&](uint32_t &value) {
-			if constexpr (flags.pre_index() == flags.add_offset()) {
-				address += 4;
-			}
-
-			if constexpr (flags.operation() == BlockDataTransferFlags::Operation::STM) {
-				// "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
-				// responsible for preventing erroneous writes to the memory."
-				visits_succeeded &= bus_.template write<uint32_t>(address, value, registers_.mode(), false);
-			} else {
-				// When ARM detects a data abort during a load multiple instruction, it modifies the operation of
-				// the instruction to ensure that recovery is possible.
-				//
-				//	*	Overwriting of registers stops when the abort happens. The aborting load will not
-				//		take place, nor will the preceding one ...
-				//	*	The base register is restored, to its modified value if write-back was requested.
-				if(visits_succeeded) {
-					const uint32_t replaced = value;
-					visits_succeeded &= bus_.template read<uint32_t>(address, value, registers_.mode(), false);
-
-					if(visits_succeeded) {
-						last_replacement.value = replaced;
-						last_replacement.target = &value;
-					} else {
-						if(last_replacement.target) {
-							*last_replacement.target = last_replacement.value;
-						}
-
-						if constexpr (!flags.write_back_address()) {
-							if(transfer.base() != 15) {
-								registers_.active[transfer.base()] = initial_address;
-							}
-						}
-					}
-				}
-			}
-
-			if constexpr (!flags.pre_index() != flags.add_offset()) {
-				address += 4;
-			}
-		};
-
-		// Handle forcing transfer of the user bank.
-		Mode original_mode = registers_.mode();
 		const bool adopt_user_mode =
 			(
 				flags.operation() == BlockDataTransferFlags::Operation::STM &&
@@ -443,19 +417,107 @@ struct Scheduler {
 			registers_.set_mode(Mode::User);
 		}
 
+		bool address_error = false;
+
+		// 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(!address_error) {
+					// "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
+					// responsible for preventing erroneous writes to the memory."
+					accesses_succeeded &= bus_.template write<uint32_t>(address, value, registers_.mode(), false);
+				}
+			} else {
+				// When ARM detects a data abort during a load multiple instruction, it modifies the operation of
+				// the instruction to ensure that recovery is possible.
+				//
+				//	*	Overwriting of registers stops when the abort happens. The aborting load will not
+				//		take place, nor will the preceding one ...
+				//	*	The base register is restored, to its modified value if write-back was requested.
+				if(accesses_succeeded) {
+					const uint32_t replaced = value;
+					accesses_succeeded &= bus_.template read<uint32_t>(address, value, registers_.mode(), false);
+
+					// Update the last-modified slot if the access succeeded; otherwise
+					// undo the last modification if there was one, and undo the base
+					// address change.
+					if(accesses_succeeded) {
+						last_replacement.value = replaced;
+						last_replacement.target = &value;
+					} else {
+						if(last_replacement.target) {
+							*last_replacement.target = last_replacement.value;
+						}
+
+						// Also restore the base register.
+						if(transfer.base() != 15) {
+							if constexpr (flags.write_back_address()) {
+								registers_.active[transfer.base()] = final_address;
+							} else {
+								registers_.active[transfer.base()] = initial_address;
+							}
+						}
+					}
+				} else {
+					// Implicitly: do the access anyway, but don't store the value. I think.
+					uint32_t throwaway;
+					bus_.template read<uint32_t>(address, throwaway, registers_.mode(), false);
+				}
+			}
+
+			// Update address after the fact for:
+			//	* post-indexed upward stores; and
+			//	* pre-indxed downward stores.
+			if constexpr (flags.pre_index() != flags.add_offset()) {
+				address += 4;
+			}
+		};
+
+		// Check for an address exception.
+		address_error = address >= (1 << 26);
+
+		// Write out registers 1 to 14.
 		for(int c = 0; c < 15; c++) {
 			if(list & (1 << c)) {
-				visit(registers_.active[c]);
+				access(registers_.active[c]);
+
+				// Modify base register after each write if writeback is enabled.
+				// 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_.active[transfer.base()] = final_address;
+					}
+				}
 			}
 		}
 
+		// Definitively write back, even if the earlier register list
+		// was empty.
+		if constexpr (flags.write_back_address()) {
+			if(transfer.base() != 15) {
+				registers_.active[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(12);
-				visit(value);
+				access(value);
 			} else {
-				visit(value);
+				access(value);
 				registers_.set_pc(value);
 				if constexpr (flags.load_psr()) {
 					registers_.set_status(value);
@@ -463,21 +525,16 @@ struct Scheduler {
 			}
 		}
 
-		if constexpr (flags.write_back_address()) {
+		// If user mode was unnaturally forced, switch back to the actual
-			if(transfer.base() != 15) {
+		// current operating mode.
-				if constexpr (flags.add_offset()) {
-					registers_.active[transfer.base()] = address;
-				} else {
-					registers_.active[transfer.base()] = final_address;
-				}
-			}
-		}
-
 		if(adopt_user_mode) {
 			registers_.set_mode(original_mode);
 		}
 
-		if(!visits_succeeded) {
+		// Finally throw an exception if necessary.
+		if(address_error) {
+			registers_.exception<Registers::Exception::Address>();
+		} else if(!accesses_succeeded) {
 			registers_.exception<Registers::Exception::DataAbort>();
 		}
 	}