Eliminate MemoryMap access macros, fix tests target.

Machines/Apple/AppleIIgs/AppleIIgs.cpp

@@ -384,7 +384,7 @@ class ConcreteMachine:
 		// MARK: BusHandler.
 		uint64_t total = 0;
 		forceinline Cycles perform_bus_operation(const CPU::WDC65816::BusOperation operation, const uint32_t address, uint8_t *const value) {
-			const auto &region = MemoryMapRegion(memory_, address);
+			const auto &region = memory_.region(address);
 			static bool log = false;
 			bool is_1Mhz = false;
 
@@ -945,7 +945,7 @@ class ConcreteMachine:
 				is_1Mhz = region.flags & MemoryMap::Region::Is1Mhz;
 
 				if(isReadOperation(operation)) {
-					MemoryMapRead(region, address, value);
+					*value = memory_.read(region, address);
 				} else {
 					// Shadowed writes also occur "at 1Mhz".
 					// TODO: this is probably an approximation. I'm assuming that there's the ability asynchronously to post
@@ -954,7 +954,7 @@ class ConcreteMachine:
 					// get by adding periodic NOPs within their copy-to-shadow step.
 					//
 					// Maybe the interaction with 2.8Mhz refresh isn't as straightforward as I think?
-					const bool is_shadowed = IsShadowed(memory_, region, address);
+					const bool is_shadowed = memory_.is_shadowed(address);
 					is_1Mhz |= is_shadowed;
 
 					// Use a very broad test for flushing video: any write to $e0 or $e1, or any write that is shadowed.
@@ -963,7 +963,7 @@ class ConcreteMachine:
 						video_.flush();
 					}
 
-					MemoryMapWrite(memory_, region, address, value);
+					memory_.write(region, address, *value);
 				}
 			}
 

Machines/Apple/AppleIIgs/MemoryMap.hpp

@@ -631,55 +631,49 @@ class MemoryMap {
 		std::array<Region, 40> regions;	// An assert above ensures that this is large enough; there's no
 										// doctrinal reason for it to be whatever size it is now, just
 										// adjust as required.
+
+		// The below encapsulates an assumption that Apple intends to shadow physical addresses (i.e. after mapping).
+		// If the Apple shadows logical addresses (i.e. prior to mapping) then see commented out alternatives.
+
+		const Region &region(uint32_t address) {	return regions[region_map[address >> 8]];	}
+		uint8_t read(const Region &region, uint32_t address) {
+			return region.read ? region.read[address] : 0xff;
+		}
+
+		bool is_shadowed(uint32_t address) const {
+			// Logical mapping alternative:
+			// shadow_pages[((&region.write[address] - ram_base) >> 10) & 127] & shadow_banks[address >> 17]
+
+			return shadow_pages[(address >> 10) & 127] & shadow_banks[address >> 17];
+
+			// Quick notes on contortions above:
+			//
+			// The objective is to support shadowing:
+			//	1. without storing a whole extra pointer, and such that the shadowing flags
+			//		are orthogonal to the current auxiliary memory settings;
+			//	2. in such a way as to support shadowing both in banks $00/$01 and elsewhere; and
+			//	3. to do so without introducing too much in the way of branching.
+			//
+			// Hence the implemented solution: if shadowing is enabled then use the distance from the start of
+			// physical RAM modulo 128k indexed into the bank $e0/$e1 RAM.
+			//
+			// With a further twist: the modulo and pointer are indexed on ::IsShadowed to eliminate a branch
+			// even on that.
+		}
+		void write(const Region &region, uint32_t address, uint8_t value) {
+			if(!region.write) {
+				return;
+			}
+
+			region.write[address] = value;
+			const bool shadowed = is_shadowed(address);
+			shadow_base[shadowed][(&region.write[address] - ram_base) & shadow_mask[shadowed]] = value;
+
+			// Logical mapping alternative:
+			// shadow_base[shadowed][address & shadow_mask[shadowed]]
+		}
 };
 
-// TODO: branching below on region.read/write is predicated on the idea that extra scratch space
-// would be less efficient. Verify that?
-
-#define MemoryMapRegion(map, address)			map.regions[map.region_map[address >> 8]]
-#define MemoryMapRead(region, address, value)	*value = region.read ? region.read[address] : 0xff
-
-// The below encapsulates the fact that I've yet to determine whether Apple intends to
-// indicate that logical addresses (i.e. those prior to being mapped per the current paging)
-// or physical addresses (i.e. after mapping) are subject to shadowing.
-#ifdef SHADOW_LOGICAL
-
-#define IsShadowed(map, region, address)	\
-	(map.shadow_pages[((&region.write[address] - map.ram_base) >> 10) & 127] & map.shadow_banks[address >> 17])
-
-#define MemoryMapWrite(map, region, address, value) \
-	if(region.write) {	\
-		region.write[address] = *value;	\
-		const bool _mm_is_shadowed = IsShadowed(map, region, address);	\
-		map.shadow_base[_mm_is_shadowed][address & map.shadow_mask[_mm_is_shadowed]] = *value;	\
-	}
-
-#else
-
-#define IsShadowed(map, region, address)	\
-	(map.shadow_pages[(address >> 10) & 127] & map.shadow_banks[address >> 17])
-
-#define MemoryMapWrite(map, region, address, value) \
-	if(region.write) {	\
-		region.write[address] = *value;	\
-		const bool _mm_is_shadowed = IsShadowed(map, region, address);	\
-		map.shadow_base[_mm_is_shadowed][(&region.write[address] - map.ram_base) & map.shadow_mask[_mm_is_shadowed]] = *value;	\
-	}
-
-#endif
-
-// Quick notes on ::IsShadowed contortions:
-//
-// The objective is to support shadowing:
-//	1. without storing a whole extra pointer, and such that the shadowing flags are orthogonal to the current auxiliary memory settings;
-//	2. in such a way as to support shadowing both in banks $00/$01 and elsewhere; and
-//	3. to do so without introducing too much in the way of branching.
-//
-// Hence the implemented solution: if shadowing is enabled then use the distance from the start of physical RAM
-// modulo 128k indexed into the bank $e0/$e1 RAM.
-//
-// With a further twist: the modulo and pointer are indexed on ::IsShadowed to eliminate a branch even on that.
-
 }
 
 #endif /* MemoryMap_h */

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

@@ -535,7 +535,6 @@
 		4B8334861F5DA3780097E338 /* 6502Storage.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 4B8334851F5DA3780097E338 /* 6502Storage.cpp */; };
 		4B83348A1F5DB94B0097E338 /* IRQDelegatePortHandler.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 4B8334891F5DB94B0097E338 /* IRQDelegatePortHandler.cpp */; };
 		4B8334951F5E25B60097E338 /* C1540.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 4B8334941F5E25B60097E338 /* C1540.cpp */; };
-		4B85322D227793CB00F26553 /* etos192uk.trace.txt.gz in Resources */ = {isa = PBXBuildFile; fileRef = 4B85322C227793CA00F26553 /* etos192uk.trace.txt.gz */; };
 		4B85322F2277ABDE00F26553 /* tos100.trace.txt.gz in Resources */ = {isa = PBXBuildFile; fileRef = 4B85322E2277ABDD00F26553 /* tos100.trace.txt.gz */; };
 		4B86E25B1F8C628F006FAA45 /* Keyboard.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 4B86E2591F8C628F006FAA45 /* Keyboard.cpp */; };
 		4B8805F01DCFC99C003085B1 /* Acorn.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 4B8805EE1DCFC99C003085B1 /* Acorn.cpp */; };
@@ -1622,7 +1621,6 @@
 		4B8334911F5E24FF0097E338 /* C1540Base.hpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.h; name = C1540Base.hpp; path = Implementation/C1540Base.hpp; sourceTree = "<group>"; };
 		4B8334941F5E25B60097E338 /* C1540.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = C1540.cpp; path = Implementation/C1540.cpp; sourceTree = "<group>"; };
 		4B85322922778E4200F26553 /* Comparative68000.hpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.h; path = Comparative68000.hpp; sourceTree = "<group>"; };
-		4B85322C227793CA00F26553 /* etos192uk.trace.txt.gz */ = {isa = PBXFileReference; lastKnownFileType = archive.gzip; path = etos192uk.trace.txt.gz; sourceTree = "<group>"; };
 		4B85322E2277ABDD00F26553 /* tos100.trace.txt.gz */ = {isa = PBXFileReference; lastKnownFileType = archive.gzip; path = tos100.trace.txt.gz; sourceTree = "<group>"; };
 		4B86E2591F8C628F006FAA45 /* Keyboard.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = Keyboard.cpp; sourceTree = "<group>"; };
 		4B86E25A1F8C628F006FAA45 /* Keyboard.hpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.h; path = Keyboard.hpp; sourceTree = "<group>"; };
@@ -3540,7 +3538,6 @@
 			isa = PBXGroup;
 			children = (
 				4B85322E2277ABDD00F26553 /* tos100.trace.txt.gz */,
-				4B85322C227793CA00F26553 /* etos192uk.trace.txt.gz */,
 			);
 			path = "TOS Startup";
 			sourceTree = "<group>";
@@ -5411,7 +5408,6 @@
 				4BB298FB1B587D8400A49093 /* ancb in Resources */,
 				4BB299431B587D8400A49093 /* dcma in Resources */,
 				4BB298FD1B587D8400A49093 /* andax in Resources */,
-				4B85322D227793CB00F26553 /* etos192uk.trace.txt.gz in Resources */,
 				4B8DF6262550D91600F3433C /* CPUEOR-trace_compare.log in Resources */,
 				4BB299401B587D8400A49093 /* cpya in Resources */,
 				4BB299BE1B587D8400A49093 /* rraix in Resources */,

OSBindings/Mac/Clock SignalTests/EmuTOSTests.mm

@@ -36,7 +36,7 @@ class EmuTOS: public ComparativeBusHandler {
 			return m68000_.get_state();
 		}
 
-		template <typename Microcycle> perform_bus_operation(const Microcycle &cycle, int) {
+		template <typename Microcycle> HalfCycles perform_bus_operation(const Microcycle &cycle, int) {
 			const uint32_t address = cycle.word_address();
 			uint32_t word_address = address;
 
@@ -56,7 +56,6 @@ class EmuTOS: public ComparativeBusHandler {
 				word_address %= ram_.size();
 			}
 
-			using Microcycle = CPU::MC68000::Microcycle;
 			if(cycle.data_select_active()) {
 				uint16_t peripheral_result = 0xffff;
 				if(is_peripheral) {
@@ -68,20 +67,20 @@ class EmuTOS: public ComparativeBusHandler {
 					}
 				}
 
-				const auto operation = (op != Microcycle::DecodeDynamically) ? op : cycle.operation;
+				using namespace CPU::MC68000;
-				switch(operation & (Microcycle::SelectWord | Microcycle::SelectByte | Microcycle::Read)) {
+				switch(cycle.operation & (Operation::SelectWord | Operation::SelectByte | Operation::Read)) {
 					default: break;
 
-					case Microcycle::SelectWord | Microcycle::Read:
+					case Operation::SelectWord | Operation::Read:
 						cycle.value->w = is_peripheral ? peripheral_result : base[word_address];
 					break;
-					case Microcycle::SelectByte | Microcycle::Read:
+					case Operation::SelectByte | Operation::Read:
 						cycle.value->b = (is_peripheral ? peripheral_result : base[word_address]) >> cycle.byte_shift();
 					break;
-					case Microcycle::SelectWord:
+					case Operation::SelectWord:
 						base[word_address] = cycle.value->w;
 					break;
-					case Microcycle::SelectByte:
+					case Operation::SelectByte:
 						base[word_address] = (cycle.value->b << cycle.byte_shift()) | (base[word_address] & (0xffff ^ cycle.byte_mask()));
 					break;
 				}

OSBindings/Mac/Clock SignalTests/IIgsMemoryMapTests.mm

@@ -38,16 +38,14 @@ namespace {
 }
 
 - (void)write:(uint8_t)value address:(uint32_t)address {
-	const auto &region = MemoryMapRegion(_memoryMap, address);
+	const auto &region = _memoryMap.region(address);
 	XCTAssertFalse(region.flags & MemoryMap::Region::IsIO);
-	MemoryMapWrite(_memoryMap, region, address, &value);
+	_memoryMap.write(region, address, value);
 }
 
 - (uint8_t)readAddress:(uint32_t)address {
-	const auto &region = MemoryMapRegion(_memoryMap, address);
+	const auto &region = _memoryMap.region(address);
-	uint8_t value;
+	return _memoryMap.read(region, address);
-	MemoryMapRead(region, address, &value);
-	return value;
 }
 
 - (void)testAllRAM {
@@ -371,8 +369,7 @@ namespace {
 			while(logical < [next intValue]) {
 				[[maybe_unused]] const auto &region =
 					self->_memoryMap.regions[self->_memoryMap.region_map[logical]];
-				const bool isShadowed =
+				const bool isShadowed = _memoryMap.is_shadowed(logical << 8);
-					IsShadowed(_memoryMap, region, (logical << 8));
 
 				XCTAssertEqual(
 					isShadowed,

OSBindings/Mac/Clock SignalTests/QLTests.mm

@@ -59,20 +59,21 @@ class QL: public ComparativeBusHandler {
 			if(cycle.data_select_active()) {
 				uint16_t peripheral_result = 0xffff;
 
-				switch(cycle.operation & (Microcycle::SelectWord | Microcycle::SelectByte | Microcycle::Read)) {
+				using namespace CPU::MC68000;
+				switch(cycle.operation & (Operation::SelectWord | Operation::SelectByte | Operation::Read)) {
 					default: break;
 
-					case Microcycle::SelectWord | Microcycle::Read:
+					case Operation::SelectWord | Operation::Read:
 						cycle.value->w = is_peripheral ? peripheral_result : base[word_address];
 					break;
-					case Microcycle::SelectByte | Microcycle::Read:
+					case Operation::SelectByte | Operation::Read:
 						cycle.value->b = (is_peripheral ? peripheral_result : base[word_address]) >> cycle.byte_shift();
 					break;
-					case Microcycle::SelectWord:
+					case Operation::SelectWord:
 						assert(!(is_rom && !is_peripheral));
 						if(!is_peripheral) base[word_address] = cycle.value->w;
 					break;
-					case Microcycle::SelectByte:
+					case Operation::SelectByte:
 						assert(!(is_rom && !is_peripheral));
 						if(!is_peripheral) base[word_address] = (cycle.value->b << cycle.byte_shift()) | (base[word_address] & (0xffff ^ cycle.byte_mask()));
 					break;

OSBindings/Mac/Clock SignalTests/TestRunner68000.hpp

@@ -82,24 +82,23 @@ class RAM68000: public CPU::MC68000::BusHandler {
 			const uint32_t word_address = cycle.word_address();
 			duration_ += cycle.length;
 
-			const auto operation = (op != Microcycle::DecodeDynamically) ? op : cycle.operation;
 			if(cycle.data_select_active()) {
-				if(operation & Microcycle::InterruptAcknowledge) {
+				if(cycle.operation & CPU::MC68000::Operation::InterruptAcknowledge) {
 					cycle.value->b = 10;
 				} else {
-					switch(operation & (Microcycle::SelectWord | Microcycle::SelectByte | Microcycle::Read)) {
+					switch(cycle.operation & (CPU::MC68000::Operation::SelectWord | CPU::MC68000::Operation::SelectByte | CPU::MC68000::Operation::Read)) {
 						default: break;
 
-						case Microcycle::SelectWord | Microcycle::Read:
+						case CPU::MC68000::Operation::SelectWord | CPU::MC68000::Operation::Read:
 							cycle.value->w = ram_[word_address % ram_.size()];
 						break;
-						case Microcycle::SelectByte | Microcycle::Read:
+						case CPU::MC68000::Operation::SelectByte | CPU::MC68000::Operation::Read:
 							cycle.value->b = ram_[word_address % ram_.size()] >> cycle.byte_shift();
 						break;
-						case Microcycle::SelectWord:
+						case CPU::MC68000::Operation::SelectWord:
 							ram_[word_address % ram_.size()] = cycle.value->w;
 						break;
-						case Microcycle::SelectByte:
+						case CPU::MC68000::Operation::SelectByte:
 							ram_[word_address % ram_.size()] = uint16_t(
 								(cycle.value->b << cycle.byte_shift()) |
 								(ram_[word_address % ram_.size()] & cycle.untouched_byte_mask())