From 37950143fc551a4d59561a0f49af0a7c6074f595 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Thu, 27 Jul 2017 20:17:13 -0400 Subject: [PATCH 1/3] Attempted to nudge wait timing onto half-cycle boundaries, which expands the number of partial machine cycles the Z80 can post but pleasingly also regularises them. Switched the AllRAMProcessor to reporting half cycles by default and corrected all Z80 tests. --- .../Bridges/TestMachineZ80.h | 2 +- .../Bridges/TestMachineZ80.mm | 12 +++--- .../Mac/Clock SignalTests/FUSETests.swift | 4 +- .../Z80MachineCycleTests.swift | 4 +- Processors/6502/6502AllRAM.cpp | 2 +- Processors/AllRAMProcessor.cpp | 2 +- Processors/AllRAMProcessor.hpp | 6 ++- Processors/Z80/Z80.hpp | 42 ++++++++++++------- Processors/Z80/Z80AllRAM.cpp | 10 ++--- Processors/Z80/Z80AllRAM.hpp | 4 +- 10 files changed, 49 insertions(+), 39 deletions(-) diff --git a/OSBindings/Mac/Clock SignalTests/Bridges/TestMachineZ80.h b/OSBindings/Mac/Clock SignalTests/Bridges/TestMachineZ80.h index 1abdf63d8..dd6ab350b 100644 --- a/OSBindings/Mac/Clock SignalTests/Bridges/TestMachineZ80.h +++ b/OSBindings/Mac/Clock SignalTests/Bridges/TestMachineZ80.h @@ -61,7 +61,7 @@ typedef NS_ENUM(NSInteger, CSTestMachineZ80Register) { @property(nonatomic, readonly, nonnull) NSArray *busOperationCaptures; @property(nonatomic, readonly) BOOL isHalted; -@property(nonatomic, readonly) int completedCycles; +@property(nonatomic, readonly) int completedHalfCycles; @property(nonatomic) BOOL nmiLine; @property(nonatomic) BOOL irqLine; diff --git a/OSBindings/Mac/Clock SignalTests/Bridges/TestMachineZ80.mm b/OSBindings/Mac/Clock SignalTests/Bridges/TestMachineZ80.mm index ffb03ae0b..a2d4889ff 100644 --- a/OSBindings/Mac/Clock SignalTests/Bridges/TestMachineZ80.mm +++ b/OSBindings/Mac/Clock SignalTests/Bridges/TestMachineZ80.mm @@ -14,7 +14,7 @@ - (void)testMachineDidPerformBusOperation:(CPU::Z80::PartialMachineCycle::Operation)operation address:(uint16_t)address value:(uint8_t)value - timeStamp:(int)time_stamp; + timeStamp:(HalfCycles)time_stamp; @end #pragma mark - C++ delegate handlers @@ -23,7 +23,7 @@ class BusOperationHandler: public CPU::Z80::AllRAMProcessor::MemoryAccessDelegat public: BusOperationHandler(CSTestMachineZ80 *targetMachine) : target_(targetMachine) {} - void z80_all_ram_processor_did_perform_bus_operation(CPU::Z80::AllRAMProcessor &processor, CPU::Z80::PartialMachineCycle::Operation operation, uint16_t address, uint8_t value, int time_stamp) { + void z80_all_ram_processor_did_perform_bus_operation(CPU::Z80::AllRAMProcessor &processor, CPU::Z80::PartialMachineCycle::Operation operation, uint16_t address, uint8_t value, HalfCycles time_stamp) { [target_ testMachineDidPerformBusOperation:operation address:address value:value timeStamp:time_stamp]; } @@ -154,8 +154,8 @@ static CPU::Z80::Register registerForRegister(CSTestMachineZ80Register reg) { return _processor->get_halt_line() ? YES : NO; } -- (int)completedCycles { - return _processor->get_timestamp(); +- (int)completedHalfCycles { + return _processor->get_timestamp().as_int(); } - (void)setNmiLine:(BOOL)nmiLine { @@ -184,7 +184,7 @@ static CPU::Z80::Register registerForRegister(CSTestMachineZ80Register reg) { _processor->set_memory_access_delegate(captureBusActivity ? _busOperationHandler : nullptr); } -- (void)testMachineDidPerformBusOperation:(CPU::Z80::PartialMachineCycle::Operation)operation address:(uint16_t)address value:(uint8_t)value timeStamp:(int)timeStamp { +- (void)testMachineDidPerformBusOperation:(CPU::Z80::PartialMachineCycle::Operation)operation address:(uint16_t)address value:(uint8_t)value timeStamp:(HalfCycles)timeStamp { if(self.captureBusActivity) { CSTestMachineZ80BusOperationCapture *capture = [[CSTestMachineZ80BusOperationCapture alloc] init]; switch(operation) { @@ -216,7 +216,7 @@ static CPU::Z80::Register registerForRegister(CSTestMachineZ80Register reg) { } capture.address = address; capture.value = value; - capture.timeStamp = timeStamp; + capture.timeStamp = timeStamp.as_int(); [_busOperationCaptures addObject:capture]; } diff --git a/OSBindings/Mac/Clock SignalTests/FUSETests.swift b/OSBindings/Mac/Clock SignalTests/FUSETests.swift index c8c193b25..c3bd83c3a 100644 --- a/OSBindings/Mac/Clock SignalTests/FUSETests.swift +++ b/OSBindings/Mac/Clock SignalTests/FUSETests.swift @@ -195,8 +195,8 @@ class FUSETests: XCTestCase { machine.runForNumber(ofCycles: Int32(targetState.tStates)) // Verify that exactly the right number of cycles was hit; this is a primitive cycle length tester. - let cyclesRun = machine.completedCycles - XCTAssert(cyclesRun == Int32(targetState.tStates), "Instruction length off; was \(machine.completedCycles) but should be \(targetState.tStates): \(name)") + let halfCyclesRun = machine.completedHalfCycles + XCTAssert(halfCyclesRun == Int32(targetState.tStates) * 2, "Instruction length off; was \(machine.completedHalfCycles) but should be \(targetState.tStates * 2): \(name)") let finalState = RegisterState(machine: machine) diff --git a/OSBindings/Mac/Clock SignalTests/Z80MachineCycleTests.swift b/OSBindings/Mac/Clock SignalTests/Z80MachineCycleTests.swift index a8a115827..086eb1bcf 100644 --- a/OSBindings/Mac/Clock SignalTests/Z80MachineCycleTests.swift +++ b/OSBindings/Mac/Clock SignalTests/Z80MachineCycleTests.swift @@ -61,7 +61,9 @@ class Z80MachineCycleTests: XCTestCase { // array access break } else { - if length != busCycles[index].length || cycle.operation != busCycles[index].operation { + XCTAssert(length & Int32(1) == 0, "While performing \(machine.busOperationCaptures) Z80 ended on a half cycle") + + if length != busCycles[index].length*2 || cycle.operation != busCycles[index].operation { matches = false break; } diff --git a/Processors/6502/6502AllRAM.cpp b/Processors/6502/6502AllRAM.cpp index 11ec5cfda..8bc1b315d 100644 --- a/Processors/6502/6502AllRAM.cpp +++ b/Processors/6502/6502AllRAM.cpp @@ -21,7 +21,7 @@ class ConcreteAllRAMProcessor: public AllRAMProcessor, public Processor #include +#include "../ClockReceiver/ClockReceiver.hpp" + namespace CPU { class AllRAMProcessor { public: AllRAMProcessor(size_t memory_size); - virtual uint32_t get_timestamp(); + HalfCycles get_timestamp(); void set_data_at_address(uint16_t startAddress, size_t length, const uint8_t *data); void get_data_at_address(uint16_t startAddress, size_t length, uint8_t *data); @@ -31,7 +33,7 @@ class AllRAMProcessor { protected: std::vector memory_; - uint32_t timestamp_; + HalfCycles timestamp_; inline void check_address_for_trap(uint16_t address) { if(traps_[address]) { diff --git a/Processors/Z80/Z80.hpp b/Processors/Z80/Z80.hpp index acff26426..ce4165264 100644 --- a/Processors/Z80/Z80.hpp +++ b/Processors/Z80/Z80.hpp @@ -67,8 +67,7 @@ enum Flag: uint8_t { */ struct PartialMachineCycle { enum Operation { - ReadOpcodeStart = 0, - ReadOpcodeWait, + ReadOpcode = 0, Read, Write, Input, @@ -79,16 +78,19 @@ struct PartialMachineCycle { Internal, BusAcknowledge, + ReadOpcodeWait, ReadWait, WriteWait, InputWait, OutputWait, InterruptWait, + ReadOpcodeStart, ReadStart, WriteStart, InputStart, OutputStart, + InterruptStart, } operation; HalfCycles length; uint16_t *address; @@ -107,28 +109,32 @@ struct PartialMachineCycle { }; // Elemental bus operations -#define ReadOpcodeStart() {PartialMachineCycle::ReadOpcodeStart, HalfCycles(4), &pc_.full, &operation_, false} +#define ReadOpcodeStart() {PartialMachineCycle::ReadOpcodeStart, HalfCycles(3), &pc_.full, &operation_, false} #define ReadOpcodeWait(f) {PartialMachineCycle::ReadOpcodeWait, HalfCycles(2), &pc_.full, &operation_, f} +#define ReadOpcodeEnd() {PartialMachineCycle::ReadOpcode, HalfCycles(1), &pc_.full, &operation_, false} + #define Refresh(len) {PartialMachineCycle::Refresh, HalfCycles(len), &refresh_addr_.full, nullptr, false} -#define ReadStart(addr, val) {PartialMachineCycle::ReadStart, HalfCycles(4), &addr.full, &val, false} +#define ReadStart(addr, val) {PartialMachineCycle::ReadStart, HalfCycles(3), &addr.full, &val, false} #define ReadWait(l, addr, val, f) {PartialMachineCycle::ReadWait, HalfCycles(l), &addr.full, &val, f} -#define ReadEnd(addr, val) {PartialMachineCycle::Read, HalfCycles(2), &addr.full, &val, false} +#define ReadEnd(addr, val) {PartialMachineCycle::Read, HalfCycles(3), &addr.full, &val, false} -#define WriteStart(addr, val) {PartialMachineCycle::WriteStart,HalfCycles(4), &addr.full, &val, false} +#define WriteStart(addr, val) {PartialMachineCycle::WriteStart,HalfCycles(3), &addr.full, &val, false} #define WriteWait(l, addr, val, f) {PartialMachineCycle::WriteWait, HalfCycles(l), &addr.full, &val, f} -#define WriteEnd(addr, val) {PartialMachineCycle::Write, HalfCycles(2), &addr.full, &val, false} +#define WriteEnd(addr, val) {PartialMachineCycle::Write, HalfCycles(3), &addr.full, &val, false} -#define InputStart(addr, val) {PartialMachineCycle::InputStart, HalfCycles(4), &addr.full, &val, false} +#define InputStart(addr, val) {PartialMachineCycle::InputStart, HalfCycles(3), &addr.full, &val, false} #define InputWait(addr, val, f) {PartialMachineCycle::InputWait, HalfCycles(2), &addr.full, &val, f} -#define InputEnd(addr, val) {PartialMachineCycle::Input, HalfCycles(2), &addr.full, &val, false} +#define InputEnd(addr, val) {PartialMachineCycle::Input, HalfCycles(3), &addr.full, &val, false} -#define OutputStart(addr, val) {PartialMachineCycle::OutputStart, HalfCycles(4), &addr.full, &val, false} +#define OutputStart(addr, val) {PartialMachineCycle::OutputStart, HalfCycles(3), &addr.full, &val, false} #define OutputWait(addr, val, f) {PartialMachineCycle::OutputWait, HalfCycles(2), &addr.full, &val, f} -#define OutputEnd(addr, val) {PartialMachineCycle::Output, HalfCycles(2), &addr.full, &val, false} +#define OutputEnd(addr, val) {PartialMachineCycle::Output, HalfCycles(3), &addr.full, &val, false} -#define IntAck(length, val) {PartialMachineCycle::Interrupt, HalfCycles(length), nullptr, &val, false} +#define IntAckStart(length, val) {PartialMachineCycle::InterruptStart, HalfCycles(length), nullptr, &val, false} #define IntWait(val) {PartialMachineCycle::InterruptWait, HalfCycles(2), nullptr, &val, true} +#define IntAckEnd(val) {PartialMachineCycle::Interrupt, HalfCycles(3), nullptr, &val, false} + // A wrapper to express a bus operation as a micro-op #define BusOp(op) {MicroOp::BusOperation, nullptr, nullptr, op} @@ -741,12 +747,14 @@ template class Processor { const MicroOp normal_fetch_decode_execute[] = { BusOp(ReadOpcodeStart()), BusOp(ReadOpcodeWait(true)), + BusOp(ReadOpcodeEnd()), { MicroOp::DecodeOperation } }; const MicroOp short_fetch_decode_execute[] = { BusOp(ReadOpcodeStart()), BusOp(ReadOpcodeWait(false)), BusOp(ReadOpcodeWait(true)), + BusOp(ReadOpcodeEnd()), { MicroOp::DecodeOperation } }; copy_program((length == 4) ? normal_fetch_decode_execute : short_fetch_decode_execute, target.fetch_decode_execute); @@ -813,6 +821,7 @@ template class Processor { { MicroOp::BeginNMI }, BusOp(ReadOpcodeStart()), BusOp(ReadOpcodeWait(true)), + BusOp(ReadOpcodeEnd()), BusOp(Refresh(6)), Push(pc_), { MicroOp::JumpTo66, nullptr, nullptr}, @@ -820,14 +829,16 @@ template class Processor { }; MicroOp irq_mode0_program[] = { { MicroOp::BeginIRQMode0 }, - BusOp(IntAck(8, operation_)), + BusOp(IntAckStart(5, operation_)), BusOp(IntWait(operation_)), + BusOp(IntAckEnd(operation_)), { MicroOp::DecodeOperationNoRChange } }; MicroOp irq_mode1_program[] = { { MicroOp::BeginIRQ }, - BusOp(IntAck(10, operation_)), + BusOp(IntAckStart(7, operation_)), BusOp(IntWait(operation_)), + BusOp(IntAckEnd(operation_)), BusOp(Refresh(4)), Push(pc_), { MicroOp::Move16, &temp16_.full, &pc_.full }, @@ -835,8 +846,9 @@ template class Processor { }; MicroOp irq_mode2_program[] = { { MicroOp::BeginIRQ }, - BusOp(IntAck(10, temp16_.bytes.low)), + BusOp(IntAckStart(7, temp16_.bytes.low)), BusOp(IntWait(temp16_.bytes.low)), + BusOp(IntAckEnd(temp16_.bytes.low)), BusOp(Refresh(4)), Push(pc_), { MicroOp::Move8, &ir_.bytes.high, &temp16_.bytes.high }, diff --git a/Processors/Z80/Z80AllRAM.cpp b/Processors/Z80/Z80AllRAM.cpp index 63a6744df..57c14a48f 100644 --- a/Processors/Z80/Z80AllRAM.cpp +++ b/Processors/Z80/Z80AllRAM.cpp @@ -17,14 +17,14 @@ class ConcreteAllRAMProcessor: public AllRAMProcessor, public Processorz80_all_ram_processor_did_perform_bus_operation(*this, cycle.operation, address, cycle.value ? *cycle.value : 0x00, timestamp_ >> 1); + delegate_->z80_all_ram_processor_did_perform_bus_operation(*this, cycle.operation, address, cycle.value ? *cycle.value : 0x00, timestamp_); } return Cycles(0); @@ -94,10 +94,6 @@ class ConcreteAllRAMProcessor: public AllRAMProcessor, public Processor::set_wait_line(value); } - - uint32_t get_timestamp() { - return timestamp_ >> 1; - } }; } diff --git a/Processors/Z80/Z80AllRAM.hpp b/Processors/Z80/Z80AllRAM.hpp index 972401fbd..2fe5a0e3b 100644 --- a/Processors/Z80/Z80AllRAM.hpp +++ b/Processors/Z80/Z80AllRAM.hpp @@ -22,7 +22,7 @@ class AllRAMProcessor: static AllRAMProcessor *Processor(); struct MemoryAccessDelegate { - virtual void z80_all_ram_processor_did_perform_bus_operation(CPU::Z80::AllRAMProcessor &processor, CPU::Z80::PartialMachineCycle::Operation operation, uint16_t address, uint8_t value, int time_stamp) = 0; + virtual void z80_all_ram_processor_did_perform_bus_operation(CPU::Z80::AllRAMProcessor &processor, CPU::Z80::PartialMachineCycle::Operation operation, uint16_t address, uint8_t value, HalfCycles time_stamp) = 0; }; inline void set_memory_access_delegate(MemoryAccessDelegate *delegate) { delegate_ = delegate; @@ -38,8 +38,6 @@ class AllRAMProcessor: virtual void set_non_maskable_interrupt_line(bool value) = 0; virtual void set_wait_line(bool value) = 0; - virtual uint32_t get_timestamp() = 0; - protected: MemoryAccessDelegate *delegate_; AllRAMProcessor() : ::CPU::AllRAMProcessor(65536), delegate_(nullptr) {} From 8848ebbd4fd940925b2a1592eed79881143e49de Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Thu, 27 Jul 2017 21:10:14 -0400 Subject: [PATCH 2/3] Formalised set_interrupt_line's optional parameter as being a count of HalfCycles; corrected PartialMachineCycle.is_wait and effected the proper timing for counter reset on a ZX81. --- Machines/ZX8081/ZX8081.cpp | 11 +++++++---- Processors/Z80/Z80.hpp | 6 +++--- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/Machines/ZX8081/ZX8081.cpp b/Machines/ZX8081/ZX8081.cpp index 6778c6187..8aa0c4e6c 100644 --- a/Machines/ZX8081/ZX8081.cpp +++ b/Machines/ZX8081/ZX8081.cpp @@ -99,8 +99,12 @@ HalfCycles Machine::perform_machine_cycle(const CPU::Z80::PartialMachineCycle &c } break; case CPU::Z80::PartialMachineCycle::Interrupt: + // resetting event is M1 and IOREQ both simultaneously having leading edges; + // that happens 2 cycles before the end of INTACK. So the timer was reset and + // now has advanced twice. + horizontal_counter_ = HalfCycles(2); + *cycle.value = 0xff; - horizontal_counter_ = 0; break; case CPU::Z80::PartialMachineCycle::Refresh: @@ -109,7 +113,7 @@ HalfCycles Machine::perform_machine_cycle(const CPU::Z80::PartialMachineCycle &c // final two cycles of an opcode fetch. Therefore communicate a transient signalling // of the IRQ line if necessary. if(!(address & 0x40)) { - set_interrupt_line(true, -2); + set_interrupt_line(true, Cycles(-2)); set_interrupt_line(false); } if(has_latched_video_byte_) { @@ -126,8 +130,7 @@ HalfCycles Machine::perform_machine_cycle(const CPU::Z80::PartialMachineCycle &c } break; - case CPU::Z80::PartialMachineCycle::ReadOpcodeStart: - case CPU::Z80::PartialMachineCycle::ReadOpcodeWait: + case CPU::Z80::PartialMachineCycle::ReadOpcode: // Check for use of the fast tape hack. if(use_fast_tape_hack_ && address == tape_trap_address_ && tape_player_.has_tape()) { uint64_t prior_offset = tape_player_.get_tape()->get_offset(); diff --git a/Processors/Z80/Z80.hpp b/Processors/Z80/Z80.hpp index ce4165264..bf6eae7d9 100644 --- a/Processors/Z80/Z80.hpp +++ b/Processors/Z80/Z80.hpp @@ -104,7 +104,7 @@ struct PartialMachineCycle { return operation <= Operation::BusAcknowledge; } inline bool is_wait() const { - return operation >= Operation::ReadWait && operation <= Operation::InterruptWait; + return operation >= Operation::ReadOpcodeWait && operation <= Operation::InterruptWait; } }; @@ -1894,7 +1894,7 @@ template class Processor { how many cycles before now the line changed state. The value may not be longer than the current machine cycle. If called at any other time, this must be zero. */ - void set_interrupt_line(bool value, int offset = 0) { + void set_interrupt_line(bool value, HalfCycles offset = 0) { if(irq_line_ == value) return; // IRQ requests are level triggered and masked. @@ -1908,7 +1908,7 @@ template class Processor { // If this change happened at least one cycle ago then: (i) we're promised that this is a machine // cycle per the contract on supplying an offset; and (ii) that means it happened before the lines // were sampled. So adjust the most recent sample. - if(offset < 0) { + if(offset <= HalfCycles(-2)) { last_request_status_ = (last_request_status_ & ~Interrupt::IRQ) | (request_status_ & Interrupt::IRQ); } } From 761afad1189f60227045f7facb7fc6028030f771 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Thu, 27 Jul 2017 21:19:16 -0400 Subject: [PATCH 3/3] Corrected timestamp return, and its testing by the 6502 timing tests. --- OSBindings/Mac/Clock SignalTests/6502TimingTests.swift | 3 ++- OSBindings/Mac/Clock SignalTests/Bridges/TestMachine6502.mm | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/OSBindings/Mac/Clock SignalTests/6502TimingTests.swift b/OSBindings/Mac/Clock SignalTests/6502TimingTests.swift index 98031b971..45c520a61 100644 --- a/OSBindings/Mac/Clock SignalTests/6502TimingTests.swift +++ b/OSBindings/Mac/Clock SignalTests/6502TimingTests.swift @@ -34,6 +34,7 @@ class MOS6502TimingTests: XCTestCase, CSTestMachineTrapHandler { 0xbd, 0x00, 0x00, // [4] LDA $0000, x (no wrap) 0xbd, 0x02, 0x00, // [5] LDA $0002, x (wrap) 0xb9, 0x00, 0x00, // [4] LDA $0000, y (no wrap) + 0xb9, 0x10, 0x00, // [5] LDA $0010, y (wrap) 0xa1, 0x44, // [6] LDA ($44, x) 0xb1, 0x00, // [5] LDA ($00), y (no wrap) @@ -222,7 +223,7 @@ class MOS6502TimingTests: XCTestCase, CSTestMachineTrapHandler { func testMachine(_ testMachine: CSTestMachine, didTrapAtAddress address: UInt16) { if self.endTime == 0 { - self.endTime = machine.timestamp - 1 + self.endTime = (machine.timestamp / 2) - 1 } } } diff --git a/OSBindings/Mac/Clock SignalTests/Bridges/TestMachine6502.mm b/OSBindings/Mac/Clock SignalTests/Bridges/TestMachine6502.mm index a9ad4c69f..50aff4217 100644 --- a/OSBindings/Mac/Clock SignalTests/Bridges/TestMachine6502.mm +++ b/OSBindings/Mac/Clock SignalTests/Bridges/TestMachine6502.mm @@ -78,7 +78,7 @@ static CPU::MOS6502::Register registerForRegister(CSTestMachine6502Register reg) } - (uint32_t)timestamp { - return _processor->get_timestamp(); + return _processor->get_timestamp().as_int(); } - (void)setIrqLine:(BOOL)irqLine {