From f3ec7d54bb6c44ed52f24871eb124a2f85f24184 Mon Sep 17 00:00:00 2001
From: Thomas Harte <thomas.harte@gmail.com>
Date: Thu, 9 Dec 2021 19:17:44 -0500
Subject: [PATCH] Clarifies wait-for-CPU-slot semantics.

Big bonus: this guarantees `advance_dma`s will be called at most once per output cycle, even if they return `false`.
---
 Machines/Amiga/Amiga.cpp   | 21 +++++++++++----------
 Machines/Amiga/Chipset.cpp | 26 ++++++++++++++------------
 Machines/Amiga/Chipset.hpp |  4 ++--
 3 files changed, 27 insertions(+), 24 deletions(-)

diff --git a/Machines/Amiga/Amiga.cpp b/Machines/Amiga/Amiga.cpp
index a0aed3471..e41c4a11d 100644
--- a/Machines/Amiga/Amiga.cpp
+++ b/Machines/Amiga/Amiga.cpp
@@ -26,6 +26,8 @@
 #include "Keyboard.hpp"
 #include "MemoryMap.hpp"
 
+#include <cassert>
+
 namespace {
 
 // NTSC clock rate: 2*3.579545 = 7.15909Mhz.
@@ -81,15 +83,14 @@ class ConcreteMachine:
 		HalfCycles perform_bus_operation(const CPU::MC68000::Microcycle &cycle, int) {
 
 			// Do a quick advance check for Chip RAM access; add a suitable delay if required.
-			HalfCycles access_delay;
+			HalfCycles total_length;
 			if(cycle.operation & Microcycle::NewAddress && *cycle.address < 0x20'0000) {
-				access_delay = chipset_.run_until_cpu_slot().duration;
+				total_length = chipset_.run_until_after_cpu_slot().duration;
+				assert(total_length >= cycle.length);
+			} else {
+				total_length = cycle.length;
+				chipset_.run_for(total_length);
 			}
-
-			// Compute total length.
-			const HalfCycles total_length = cycle.length + access_delay;
-
-			chipset_.run_for(total_length);
 			mc68000_.set_interrupt_level(chipset_.get_interrupt_level());
 
 			// Check for assertion of reset.
@@ -101,11 +102,11 @@ class ConcreteMachine:
 			// Autovector interrupts.
 			if(cycle.operation & Microcycle::InterruptAcknowledge) {
 				mc68000_.set_is_peripheral_address(true);
-				return access_delay;
+				return total_length - cycle.length;
 			}
 
 			// Do nothing if no address is exposed.
-			if(!(cycle.operation & (Microcycle::NewAddress | Microcycle::SameAddress))) return access_delay;
+			if(!(cycle.operation & (Microcycle::NewAddress | Microcycle::SameAddress))) return total_length - cycle.length;
 
 			// Grab the target address to pick a memory source.
 			const uint32_t address = cycle.host_endian_byte_address();
@@ -165,7 +166,7 @@ class ConcreteMachine:
 				);
 			}
 
-			return access_delay;
+			return total_length - cycle.length;
 		}
 
 		void flush() {
diff --git a/Machines/Amiga/Chipset.cpp b/Machines/Amiga/Chipset.cpp
index 1ee225673..a72203c94 100644
--- a/Machines/Amiga/Chipset.cpp
+++ b/Machines/Amiga/Chipset.cpp
@@ -71,7 +71,7 @@ Chipset::Changes Chipset::run_for(HalfCycles length) {
 	return run<false>(length);
 }
 
-Chipset::Changes Chipset::run_until_cpu_slot() {
+Chipset::Changes Chipset::run_until_after_cpu_slot() {
 	return run<true>();
 }
 
@@ -589,17 +589,19 @@ template <bool stop_on_cpu> int Chipset::advance_slots(int first_slot, int last_
 	}
 	assert(last_slot > first_slot);
 
-#define C(x) \
-	case x: \
-		if constexpr(stop_on_cpu) {\
-			if(perform_cycle<x, stop_on_cpu>()) {\
-				return x - first_slot;\
-			}\
-		} else {\
-			perform_cycle<x, stop_on_cpu>(); \
-		} \
-		output<x>();	\
-		if((x + 1) == last_slot) break;	\
+#define C(x) 										\
+	case x: 										\
+		output<x>();								\
+													\
+		if constexpr (stop_on_cpu) {				\
+			if(perform_cycle<x, stop_on_cpu>()) {	\
+				return 1 + x - first_slot;			\
+			}										\
+		} else {									\
+			perform_cycle<x, stop_on_cpu>(); 		\
+		} 											\
+													\
+		if((x + 1) == last_slot) break;				\
 		[[fallthrough]]
 
 #define C10(x)	C(x); C(x+1); C(x+2); C(x+3); C(x+4); C(x+5); C(x+6); C(x+7); C(x+8); C(x+9);
diff --git a/Machines/Amiga/Chipset.hpp b/Machines/Amiga/Chipset.hpp
index 555855902..057eb484a 100644
--- a/Machines/Amiga/Chipset.hpp
+++ b/Machines/Amiga/Chipset.hpp
@@ -54,8 +54,8 @@ class Chipset: private ClockingHint::Observer {
 		/// Advances the stated amount of time.
 		Changes run_for(HalfCycles);
 
-		/// Advances to the next available CPU slot.
-		Changes run_until_cpu_slot();
+		/// Advances to the end of the next available CPU slot.
+		Changes run_until_after_cpu_slot();
 
 		/// Performs the provided microcycle, which the caller guarantees to be a memory access.
 		void perform(const CPU::MC68000::Microcycle &);