Merge pull request #985 from TomHarte/68000Improvements

68000: fix E alignment, expand Microcycle::apply.

Processors/68000/68000.hpp

@@ -49,51 +49,55 @@ namespace MC68000 {
 	avoid the runtime cost of actual DTack emulation. But such as the bus allows.)
 */
 struct Microcycle {
+	using OperationT = unsigned int;
+
 	/// Indicates that the address strobe and exactly one of the data strobes are active; you can determine
 	/// which by inspecting the low bit of the provided address. The RW line indicates a read.
-	static constexpr int SelectByte				= 1 << 0;
+	static constexpr OperationT SelectByte				= 1 << 0;
 	// Maintenance note: this is bit 0 to reduce the cost of getting a host-endian
 	// bytewise address. The assumption that it is bit 0 is also used for branchless
 	// selection in a few places. See implementation of host_endian_byte_address(),
 	// value8_high(), value8_low() and value16().
 
 	/// Indicates that the address and both data select strobes are active.
-	static constexpr int SelectWord				= 1 << 1;
+	static constexpr OperationT SelectWord				= 1 << 1;
+
+	/// If set, indicates a read. Otherwise, a write.
+	static constexpr OperationT Read 					= 1 << 2;
+
+	// Two-bit gap deliberately left here for PermitRead/Write below.
 
 	/// A NewAddress cycle is one in which the address strobe is initially low but becomes high;
 	/// this correlates to states 0 to 5 of a standard read/write cycle.
-	static constexpr int NewAddress				= 1 << 2;
+	static constexpr OperationT NewAddress				= 1 << 5;
 
 	/// A SameAddress cycle is one in which the address strobe is continuously asserted, but neither
 	/// of the data strobes are.
-	static constexpr int SameAddress			= 1 << 3;
+	static constexpr OperationT SameAddress				= 1 << 6;
 
 	/// A Reset cycle is one in which the RESET output is asserted.
-	static constexpr int Reset					= 1 << 4;
+	static constexpr OperationT Reset					= 1 << 7;
-
-	/// If set, indicates a read. Otherwise, a write.
-	static constexpr int Read 					= 1 << 5;
 
 	/// Contains the value of line FC0 if it is not implicit via InterruptAcknowledge.
-	static constexpr int IsData 				= 1 << 6;
+	static constexpr OperationT IsData 					= 1 << 8;
 
 	/// Contains the value of line FC1 if it is not implicit via InterruptAcknowledge.
-	static constexpr int IsProgram 				= 1 << 7;
+	static constexpr OperationT IsProgram 				= 1 << 9;
 
 	/// The interrupt acknowledge cycle is that during which the 68000 seeks to obtain the vector for
 	/// an interrupt it plans to observe. Noted on a real 68000 by all FCs being set to 1.
-	static constexpr int InterruptAcknowledge	= 1 << 8;
+	static constexpr OperationT InterruptAcknowledge	= 1 << 10;
 
 	/// Represents the state of the 68000's valid memory address line — indicating whether this microcycle
 	/// is synchronised with the E clock to satisfy a valid peripheral address request.
-	static constexpr int IsPeripheral 			= 1 << 9;
+	static constexpr OperationT IsPeripheral 			= 1 << 11;
 
 	/// Provides the 68000's bus grant line — indicating whether a bus request has been acknowledged.
-	static constexpr int BusGrant				= 1 << 10;
+	static constexpr OperationT BusGrant				= 1 << 12;
 
 	/// Contains a valid combination of the various static constexpr int flags, describing the operation
 	/// performed by this Microcycle.
-	int operation = 0;
+	OperationT operation = 0;
 
 	/// Describes the duration of this Microcycle.
 	HalfCycles length = HalfCycles(4);
@@ -252,6 +256,7 @@ struct Microcycle {
 		currently being read. Assumes this is a read cycle.
 	*/
 	forceinline void set_value16(uint16_t v) const {
+		assert(operation & Microcycle::Read);
 		if(operation & Microcycle::SelectWord) {
 			value->full = v;
 		} else {
@@ -263,6 +268,7 @@ struct Microcycle {
 		Equivalent to set_value16((v << 8) | 0x00ff).
 	*/
 	forceinline void set_value8_high(uint8_t v) const {
+		assert(operation & Microcycle::Read);
 		if(operation & Microcycle::SelectWord) {
 			value->full = uint16_t(0x00ff | (v << 8));
 		} else {
@@ -274,6 +280,7 @@ struct Microcycle {
 		Equivalent to set_value16((v) | 0xff00).
 	*/
 	forceinline void set_value8_low(uint8_t v) const {
+		assert(operation & Microcycle::Read);
 		if(operation & Microcycle::SelectWord) {
 			value->full = 0xff00 | v;
 		} else {
@@ -289,29 +296,41 @@ struct Microcycle {
 		return ((*address) & 0x00fffffe) >> 1;
 	}
 
+	// PermitRead and PermitWrite are used as part of the read/write mask
+	// supplied to @c apply; they are picked to be small enough values that
+	// a byte can be used for storage.
+	static constexpr OperationT PermitRead 	= 1 << 3;
+	static constexpr OperationT PermitWrite	= 1 << 4;
+
 	/*!
-		Assuming this to be a cycle with a data select active, applies it to @c target,
+		Assuming this to be a cycle with a data select active, applies it to @c target
-		where 'applies' means:
+		subject to the read_write_mask, where 'applies' means:
 
 			* if this is a byte read, reads a single byte from @c target;
 			* if this is a word read, reads a word (in the host platform's endianness) from @c target; and
 			* if this is a write, does the converse of a read.
 	*/
-	forceinline void apply(uint8_t *target) const {
+	forceinline void apply(uint8_t *target, OperationT read_write_mask = PermitRead | PermitWrite) const {
-		switch(operation & (SelectWord | SelectByte | Read)) {
+		assert( (operation & (SelectWord | SelectByte)) != (SelectWord | SelectByte));
+
+		switch((operation | read_write_mask) & (SelectWord | SelectByte | Read | PermitRead | PermitWrite)) {
 			default:
 			break;
 
-			case SelectWord | Read:
+			case SelectWord | Read | PermitRead:
+			case SelectWord | Read | PermitRead | PermitWrite:
 				value->full = *reinterpret_cast<uint16_t *>(target);
 			break;
-			case SelectByte | Read:
+			case SelectByte | Read | PermitRead:
+			case SelectByte | Read | PermitRead | PermitWrite:
 				value->halves.low = *target;
 			break;
-			case Microcycle::SelectWord:
+			case SelectWord | PermitWrite:
+			case SelectWord | PermitWrite | PermitRead:
 				*reinterpret_cast<uint16_t *>(target) = value->full;
 			break;
-			case Microcycle::SelectByte:
+			case SelectByte | PermitWrite:
+			case SelectByte | PermitWrite | PermitRead:
 				*target = value->halves.low;
 			break;
 		}
@@ -434,6 +453,15 @@ template <class T, bool dtack_is_implicit, bool signal_will_perform = false> cla
 			halt_ = halt;
 		}
 
+		/// @returns The current phase of the E clock; this will be a number of
+		/// half-cycles between 0 and 19 inclusive, indicating how far the 68000
+		/// is into the current E cycle.
+		///
+		/// This is guaranteed to be 0 at initial 68000 construction.
+		HalfCycles get_e_clock_phase() {
+			return e_clock_phase_;
+		}
+
 	private:
 		T &bus_handler_;
 };

Processors/68000/Implementation/68000Implementation.hpp

@@ -136,10 +136,11 @@ template <class T, bool dtack_is_implicit, bool signal_will_perform> void Proces
 							auto cycle_copy = active_step_->microcycle;
 							cycle_copy.operation |= Microcycle::IsPeripheral;
 
-							// Extend length by: (i) distance to next E low, plus (ii) difference between
+							// Length will be: (i) distance to next E cycle, plus (ii) difference between
 							// current length and a whole E cycle.
-							cycle_copy.length = HalfCycles(20);	// i.e. one E cycle in length.
+							const auto phase_now = (e_clock_phase_ + cycles_run_for) % 20;
-							cycle_copy.length += (e_clock_phase_ + cycles_run_for) % 10;
+							const auto time_to_boundary = (HalfCycles(20) - phase_now) % HalfCycles(20);
+							cycle_copy.length = HalfCycles(20) + time_to_boundary;
 
 							cycles_run_for +=
 								cycle_copy.length +
@@ -189,6 +190,11 @@ template <class T, bool dtack_is_implicit, bool signal_will_perform> void Proces
 							case BusStep::Action::DecrementEffectiveAddress1:	effective_address_[1].full -= 2;	break;
 							case BusStep::Action::IncrementProgramCounter:		program_counter_.full += 2;			break;
 
+							case BusStep::Action::IncrementEffectiveAddress0AlignStackPointer:
+								effective_address_[0].full += 2;
+								address_[7].full &= 0xffff'fffe;
+							break;
+
 							case BusStep::Action::AdvancePrefetch:
 								prefetch_queue_.halves.high = prefetch_queue_.halves.low;
 
@@ -332,6 +338,9 @@ template <class T, bool dtack_is_implicit, bool signal_will_perform> void Proces
 									active_program_ = nullptr;
 									active_micro_op_ = short_exception_micro_ops_;
 									populate_trap_steps(8, status());
+
+									// The location of the failed instruction is what should end up on the stack.
+									program_counter_.full -= 4;
 								} else {
 									// Standard instruction dispatch.
 									active_program_ = &instructions[decoded_instruction_.full];
@@ -991,7 +1000,7 @@ template <class T, bool dtack_is_implicit, bool signal_will_perform> void Proces
 	populate_trap_steps(5, status());								\
 	bus_program->microcycle.length = HalfCycles(20);				\
 																	\
-	program_counter_.full -= 2;
+	program_counter_.full -= 6;
 
 								case Operation::DIVU: {
 									carry_flag_ = 0;
@@ -2188,7 +2197,7 @@ template <class T, bool dtack_is_implicit, bool signal_will_perform> void Proces
 #undef destination_address
 
 	bus_handler_.flush();
-	e_clock_phase_ = (e_clock_phase_ + cycles_run_for) % 10;
+	e_clock_phase_ = (e_clock_phase_ + cycles_run_for) % 20;
 	half_cycles_left_to_run_ = remaining_duration - cycles_run_for;
 }
 

Processors/68000/Implementation/68000Storage.cpp

@@ -261,7 +261,7 @@ struct ProcessorStorageConstructor {
 					steps.push_back(step);
 
 					step.microcycle.operation = Microcycle::SameAddress | Microcycle::Read | Microcycle::IsProgram | Microcycle::SelectWord;
-					step.action = Action::IncrementEffectiveAddress0;
+					step.action = isupper(access_pattern[1]) ? Action::IncrementEffectiveAddress0 : Action::IncrementEffectiveAddress0AlignStackPointer;
 					steps.push_back(step);
 
 					continue;

Processors/68000/Implementation/68000Storage.hpp

@@ -196,6 +196,9 @@ class ProcessorStorage {
 				/// Copies prefetch_queue_[1] to prefetch_queue_[0].
 				AdvancePrefetch,
 
+				/// Performs effective_address_[0] += 2 and zeroes the final bit of the stack pointer.
+				IncrementEffectiveAddress0AlignStackPointer,
+
 				/*!
 					Terminates an atomic program; if nothing else is pending, schedules the next instruction.
 					This action is special in that it usurps any included microcycle. So any Step with this