From 5c7f94d2efc592f2dc79e090ad7f3b7637597407 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Mon, 27 Nov 2023 11:48:34 -0500 Subject: [PATCH 1/5] Introduce the possibility of operation type as a template parameter. It's already proven possible to provide this for instruction fetch, so I think it'll immediately be a win. But more importantly it opens a path forwards for further improvement. --- Machines/Amiga/Amiga.cpp | 22 ++++--- Machines/Apple/Macintosh/Macintosh.cpp | 41 ++++++------ Machines/Atari/ST/AtariST.cpp | 29 +++++---- Processors/68000/68000.hpp | 5 ++ .../Implementation/68000Implementation.hpp | 65 ++++++++++--------- .../68000/Implementation/68000Storage.hpp | 42 ++++++++---- 6 files changed, 117 insertions(+), 87 deletions(-) diff --git a/Machines/Amiga/Amiga.cpp b/Machines/Amiga/Amiga.cpp index 8d08e63d1..775e89695 100644 --- a/Machines/Amiga/Amiga.cpp +++ b/Machines/Amiga/Amiga.cpp @@ -80,11 +80,13 @@ class ConcreteMachine: } // MARK: - MC68000::BusHandler. - template HalfCycles perform_bus_operation(const Microcycle &cycle, int) { + using Microcycle = CPU::MC68000::Microcycle; + template HalfCycles perform_bus_operation(const Microcycle &cycle, int) { + const auto operation = (op != Microcycle::DecodeDynamically) ? op : cycle.operation; // Do a quick advance check for Chip RAM access; add a suitable delay if required. HalfCycles total_length; - if(cycle.operation & Microcycle::NewAddress && *cycle.address < 0x20'0000) { + if(operation & Microcycle::NewAddress && *cycle.address < 0x20'0000) { total_length = chipset_.run_until_after_cpu_slot().duration; assert(total_length >= cycle.length); } else { @@ -94,19 +96,19 @@ class ConcreteMachine: mc68000_.set_interrupt_level(chipset_.get_interrupt_level()); // Check for assertion of reset. - if(cycle.operation & Microcycle::Reset) { + if(operation & Microcycle::Reset) { memory_.reset(); LOG("Reset; PC is around " << PADHEX(8) << mc68000_.get_state().registers.program_counter); } // Autovector interrupts. - if(cycle.operation & Microcycle::InterruptAcknowledge) { + if(operation & Microcycle::InterruptAcknowledge) { mc68000_.set_is_peripheral_address(true); return total_length - cycle.length; } // Do nothing if no address is exposed. - if(!(cycle.operation & (Microcycle::NewAddress | Microcycle::SameAddress))) return total_length - cycle.length; + if(!(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(); @@ -115,7 +117,7 @@ class ConcreteMachine: mc68000_.set_is_peripheral_address((address & 0xe0'0000) == 0xa0'0000); if(!memory_.regions[address >> 18].read_write_mask) { - if((cycle.operation & (Microcycle::SelectByte | Microcycle::SelectWord))) { + if((operation & (Microcycle::SelectByte | Microcycle::SelectWord))) { // Check for various potential chip accesses. // Per the manual: @@ -133,7 +135,7 @@ class ConcreteMachine: const bool select_a = !(address & 0x1000); const bool select_b = !(address & 0x2000); - if(cycle.operation & Microcycle::Read) { + if(operation & Microcycle::Read) { uint16_t result = 0xffff; if(select_a) result &= 0xff00 | (chipset_.cia_a.read(reg) << 0); if(select_b) result &= 0x00ff | (chipset_.cia_b.read(reg) << 8); @@ -143,7 +145,7 @@ class ConcreteMachine: if(select_b) chipset_.cia_b.write(reg, cycle.value8_high()); } -// LOG("CIA " << (((address >> 12) & 3)^3) << " " << (cycle.operation & Microcycle::Read ? "read " : "write ") << std::dec << (reg & 0xf) << " of " << PADHEX(4) << +cycle.value16()); +// LOG("CIA " << (((address >> 12) & 3)^3) << " " << (operation & Microcycle::Read ? "read " : "write ") << std::dec << (reg & 0xf) << " of " << PADHEX(4) << +cycle.value16()); } else if(address >= 0xdf'f000 && address <= 0xdf'f1be) { chipset_.perform(cycle); } else if(address >= 0xe8'0000 && address < 0xe9'0000) { @@ -155,13 +157,13 @@ class ConcreteMachine: memory_.perform(cycle); } else { // This'll do for open bus, for now. - if(cycle.operation & Microcycle::Read) { + if(operation & Microcycle::Read) { cycle.set_value16(0xffff); } // Don't log for the region that is definitely just ROM this machine doesn't have. if(address < 0xf0'0000) { - LOG("Unmapped " << (cycle.operation & Microcycle::Read ? "read from " : "write to ") << PADHEX(6) << ((*cycle.address)&0xffffff) << " of " << cycle.value16()); + LOG("Unmapped " << (operation & Microcycle::Read ? "read from " : "write to ") << PADHEX(6) << ((*cycle.address)&0xffffff) << " of " << cycle.value16()); } } } diff --git a/Machines/Apple/Macintosh/Macintosh.cpp b/Machines/Apple/Macintosh/Macintosh.cpp index 525ff8f75..24d6e9623 100644 --- a/Machines/Apple/Macintosh/Macintosh.cpp +++ b/Machines/Apple/Macintosh/Macintosh.cpp @@ -192,13 +192,14 @@ template class ConcreteMachin } using Microcycle = CPU::MC68000::Microcycle; + template HalfCycles perform_bus_operation(const Microcycle &cycle, int) { + const auto operation = (op != Microcycle::DecodeDynamically) ? op : cycle.operation; - HalfCycles perform_bus_operation(const Microcycle &cycle, int) { // Advance time. advance_time(cycle.length); // A null cycle leaves nothing else to do. - if(!(cycle.operation & (Microcycle::NewAddress | Microcycle::SameAddress))) return HalfCycles(0); + if(!(operation & (Microcycle::NewAddress | Microcycle::SameAddress))) return HalfCycles(0); // Grab the address. auto address = cycle.host_endian_byte_address(); @@ -218,7 +219,7 @@ template class ConcreteMachin // having set VPA above deals with those given that the generated address // for interrupt acknowledge cycles always has all bits set except the // lowest explicit address lines. - if(!cycle.data_select_active() || (cycle.operation & Microcycle::InterruptAcknowledge)) return HalfCycles(0); + if(!cycle.data_select_active() || (operation & Microcycle::InterruptAcknowledge)) return HalfCycles(0); // Grab the word-precision address being accessed. uint8_t *memory_base = nullptr; @@ -227,18 +228,18 @@ template class ConcreteMachin default: assert(false); case BusDevice::Unassigned: - fill_unmapped(cycle); + fill_unmapped(cycle); return delay; case BusDevice::VIA: { if(*cycle.address & 1) { - fill_unmapped(cycle); + fill_unmapped(cycle); } else { const int register_address = address >> 9; // VIA accesses are via address 0xefe1fe + register*512, // which at word precision is 0x77f0ff + register*256. - if(cycle.operation & Microcycle::Read) { + if(operation & Microcycle::Read) { cycle.set_value8_high(via_.read(register_address)); } else { via_.write(register_address, cycle.value8_high()); @@ -247,7 +248,7 @@ template class ConcreteMachin } return delay; case BusDevice::PhaseRead: { - if(cycle.operation & Microcycle::Read) { + if(operation & Microcycle::Read) { cycle.set_value8_low(phase_ & 7); } } return delay; @@ -257,13 +258,13 @@ template class ConcreteMachin const int register_address = address >> 9; // The IWM; this is a purely polled device, so can be run on demand. - if(cycle.operation & Microcycle::Read) { + if(operation & Microcycle::Read) { cycle.set_value8_low(iwm_->read(register_address)); } else { iwm_->write(register_address, cycle.value8_low()); } } else { - fill_unmapped(cycle); + fill_unmapped(cycle); } } return delay; @@ -274,14 +275,14 @@ template class ConcreteMachin // Even accesses = read; odd = write. if(*cycle.address & 1) { // Odd access => this is a write. Data will be in the upper byte. - if(cycle.operation & Microcycle::Read) { + if(operation & Microcycle::Read) { scsi_.write(register_address, 0xff, dma_acknowledge); } else { scsi_.write(register_address, cycle.value8_high()); } } else { // Even access => this is a read. - if(cycle.operation & Microcycle::Read) { + if(operation & Microcycle::Read) { cycle.set_value8_high(scsi_.read(register_address, dma_acknowledge)); } } @@ -289,19 +290,19 @@ template class ConcreteMachin case BusDevice::SCCReadResetPhase: { // Any word access here adjusts phase. - if(cycle.operation & Microcycle::SelectWord) { + if(operation & Microcycle::SelectWord) { adjust_phase(); } else { // A0 = 1 => reset; A0 = 0 => read. if(*cycle.address & 1) { scc_.reset(); - if(cycle.operation & Microcycle::Read) { + if(operation & Microcycle::Read) { cycle.set_value16(0xffff); } } else { const auto read = scc_.read(int(address >> 1)); - if(cycle.operation & Microcycle::Read) { + if(operation & Microcycle::Read) { cycle.set_value8_high(read); } } @@ -310,20 +311,20 @@ template class ConcreteMachin case BusDevice::SCCWrite: { // Any word access here adjusts phase. - if(cycle.operation & Microcycle::SelectWord) { + if(operation & Microcycle::SelectWord) { adjust_phase(); } else { // This is definitely a byte access; either it's to an odd address, in which // case it will reach the SCC, or it isn't, in which case it won't. if(*cycle.address & 1) { - if(cycle.operation & Microcycle::Read) { + if(operation & Microcycle::Read) { scc_.write(int(address >> 1), 0xff); cycle.value->b = 0xff; } else { scc_.write(int(address >> 1), cycle.value->b); } } else { - fill_unmapped(cycle); + fill_unmapped(cycle); } } } return delay; @@ -351,7 +352,7 @@ template class ConcreteMachin } break; case BusDevice::ROM: { - if(!(cycle.operation & Microcycle::Read)) return delay; + if(!(operation & Microcycle::Read)) return delay; memory_base = rom_; address &= rom_mask_; } break; @@ -543,8 +544,10 @@ template class ConcreteMachin ++phase_; } + template forceinline void fill_unmapped(const Microcycle &cycle) { - if(!(cycle.operation & Microcycle::Read)) return; + const auto operation = (op != Microcycle::DecodeDynamically) ? op : cycle.operation; + if(!(operation & Microcycle::Read)) return; cycle.set_value16(0xffff); } diff --git a/Machines/Atari/ST/AtariST.cpp b/Machines/Atari/ST/AtariST.cpp index f40e1de0a..f6a2d8163 100644 --- a/Machines/Atari/ST/AtariST.cpp +++ b/Machines/Atari/ST/AtariST.cpp @@ -174,7 +174,10 @@ class ConcreteMachine: } // MARK: MC68000::BusHandler - template HalfCycles perform_bus_operation(const Microcycle &cycle, int is_supervisor) { + using Microcycle = CPU::MC68000::Microcycle; + template HalfCycles perform_bus_operation(const Microcycle &cycle, int is_supervisor) { + const auto operation = (op != Microcycle::DecodeDynamically) ? op : cycle.operation; + // Just in case the last cycle was an interrupt acknowledge or bus error. TODO: find a better solution? mc68000_.set_is_peripheral_address(false); mc68000_.set_bus_error(false); @@ -183,15 +186,15 @@ class ConcreteMachine: advance_time(cycle.length); // Check for assertion of reset. - if(cycle.operation & Microcycle::Reset) { + if(operation & Microcycle::Reset) { LOG("Unhandled Reset"); } // A null cycle leaves nothing else to do. - if(!(cycle.operation & (Microcycle::NewAddress | Microcycle::SameAddress))) return HalfCycles(0); + if(!(operation & (Microcycle::NewAddress | Microcycle::SameAddress))) return HalfCycles(0); // An interrupt acknowledge, perhaps? - if(cycle.operation & Microcycle::InterruptAcknowledge) { + if(operation & Microcycle::InterruptAcknowledge) { // Current implementation: everything other than 6 (i.e. the MFP) is autovectored. const int interrupt_level = cycle.word_address()&7; if(interrupt_level != 6) { @@ -200,7 +203,7 @@ class ConcreteMachine: mc68000_.set_is_peripheral_address(true); return HalfCycles(0); } else { - if(cycle.operation & Microcycle::SelectByte) { + if(operation & Microcycle::SelectByte) { const int interrupt = mfp_->acknowledge_interrupt(); if(interrupt != Motorola::MFP68901::MFP68901::NoAcknowledgement) { cycle.value->b = uint8_t(interrupt); @@ -217,7 +220,7 @@ class ConcreteMachine: // If this is a new strobing of the address signal, test for bus error and pre-DTack delay. HalfCycles delay(0); - if(cycle.operation & Microcycle::NewAddress) { + if(operation & Microcycle::NewAddress) { // Bus error test. if( // Anything unassigned should generate a bus error. @@ -269,7 +272,7 @@ class ConcreteMachine: TOS 1.0 appears to attempt to read from the catridge before it has setup the bus error vector. Therefore I assume no bus error flows. */ - switch(cycle.operation & (Microcycle::SelectWord | Microcycle::SelectByte | Microcycle::Read)) { + switch(operation & (Microcycle::SelectWord | Microcycle::SelectByte | Microcycle::Read)) { default: break; case Microcycle::SelectWord | Microcycle::Read: cycle.value->w = 0xffff; @@ -313,7 +316,7 @@ class ConcreteMachine: case 0x8260: case 0x8262: if(!cycle.data_select_active()) return delay; - if(cycle.operation & Microcycle::Read) { + if(operation & Microcycle::Read) { cycle.set_value16(video_->read(int(address >> 1))); } else { video_->write(int(address >> 1), cycle.value16()); @@ -324,7 +327,7 @@ class ConcreteMachine: case 0x8604: case 0x8606: case 0x8608: case 0x860a: case 0x860c: if(!cycle.data_select_active()) return delay; - if(cycle.operation & Microcycle::Read) { + if(operation & Microcycle::Read) { cycle.set_value16(dma_->read(int(address >> 1))); } else { dma_->write(int(address >> 1), cycle.value16()); @@ -360,7 +363,7 @@ class ConcreteMachine: advance_time(HalfCycles(2)); update_audio(); - if(cycle.operation & Microcycle::Read) { + if(operation & Microcycle::Read) { cycle.set_value8_high(GI::AY38910::Utility::read(ay_)); } else { // Net effect here: addresses with bit 1 set write to a register, @@ -380,7 +383,7 @@ class ConcreteMachine: case 0xfa38: case 0xfa3a: case 0xfa3c: case 0xfa3e: if(!cycle.data_select_active()) return delay; - if(cycle.operation & Microcycle::Read) { + if(operation & Microcycle::Read) { cycle.set_value8_low(mfp_->read(int(address >> 1))); } else { mfp_->write(int(address >> 1), cycle.value8_low()); @@ -394,7 +397,7 @@ class ConcreteMachine: if(!cycle.data_select_active()) return delay; const auto acia_ = (address & 4) ? &midi_acia_ : &keyboard_acia_; - if(cycle.operation & Microcycle::Read) { + if(operation & Microcycle::Read) { cycle.set_value8_high((*acia_)->read(int(address >> 1))); } else { (*acia_)->write(int(address >> 1), cycle.value8_high()); @@ -405,7 +408,7 @@ class ConcreteMachine: } // If control has fallen through to here, the access is either a read from ROM, or a read or write to RAM. - switch(cycle.operation & (Microcycle::SelectWord | Microcycle::SelectByte | Microcycle::Read)) { + switch(operation & (Microcycle::SelectWord | Microcycle::SelectByte | Microcycle::Read)) { default: break; diff --git a/Processors/68000/68000.hpp b/Processors/68000/68000.hpp index 84c152662..659c59bf4 100644 --- a/Processors/68000/68000.hpp +++ b/Processors/68000/68000.hpp @@ -85,6 +85,11 @@ struct Microcycle { /// Provides the 68000's bus grant line — indicating whether a bus request has been acknowledged. static constexpr OperationT BusGrant = 1 << 12; + /// An otherwise invalid combination; used as the operaiton template parameter to @c perform_bus_operation if + /// the operation wasn't knowable in advance and the receiver should decode dynamically using the microcycle's + /// .operation field. + static constexpr OperationT DecodeDynamically = NewAddress | SameAddress; + /// Contains a valid combination of the various static constexpr int flags, describing the operation /// performed by this Microcycle. OperationT operation = 0; diff --git a/Processors/68000/Implementation/68000Implementation.hpp b/Processors/68000/Implementation/68000Implementation.hpp index 5a49bbb7e..402a4db3b 100644 --- a/Processors/68000/Implementation/68000Implementation.hpp +++ b/Processors/68000/Implementation/68000Implementation.hpp @@ -282,14 +282,17 @@ void Processor(x, is_supervisor_); \ Spend(x.length + delay) +// TODO: the templated operation type to perform_bus_operation is intended to allow a much +// cheaper through cost where the operation is knowable in advance. So use that pathway. + // Performs no bus activity for the specified number of microcycles. #define IdleBus(n) \ idle.length = HalfCycles((n) << 2); \ - PerformBusOperation(idle) + PerformBusOperation(idle, 0) // Spin until DTACK, VPA or BERR is asserted (unless DTACK is implicit), // holding the bus cycle provided. @@ -312,7 +315,7 @@ void Processor> 1) & 1) { \ - RaiseBusOrAddressError(AddressError, perform); \ - } \ - PerformBusOperation(announce); \ - WaitForDTACK(announce); \ - CompleteAccess(perform); +#define AccessPair(val, announce, announce_op, perform, perform_op) \ + perform.value = &val; \ + if constexpr (!dtack_is_implicit) { \ + announce.length = HalfCycles(4); \ + } \ + if(*perform.address & (perform.operation >> 1) & 1) { \ + RaiseBusOrAddressError(AddressError, perform); \ + } \ + PerformBusOperation(announce, announce_op); \ + WaitForDTACK(announce); \ + CompleteAccess(perform, perform_op); // Sets up the next data access size and read flags. #define SetupDataAccess(read_flag, select_flag) \ @@ -348,11 +351,11 @@ void Processor(read_program_announce, is_supervisor_); + bus_handler_.perform_bus_operation(read_program, is_supervisor_); program_counter_.l += 2; read_program.value = &prefetch_.low; - bus_handler_.perform_bus_operation(read_program_announce, is_supervisor_); - bus_handler_.perform_bus_operation(read_program, is_supervisor_); + bus_handler_.perform_bus_operation(read_program_announce, is_supervisor_); + bus_handler_.perform_bus_operation(read_program, is_supervisor_); program_counter_.l += 2; } diff --git a/Processors/68000/Implementation/68000Storage.hpp b/Processors/68000/Implementation/68000Storage.hpp index b46fe5a09..47626d90e 100644 --- a/Processors/68000/Implementation/68000Storage.hpp +++ b/Processors/68000/Implementation/68000Storage.hpp @@ -184,12 +184,12 @@ struct ProcessorBase: public InstructionSet::M68k::NullFlowController { Microcycle idle{0}; // Read a program word. All accesses via the program counter are word sized. - Microcycle read_program_announce { - Microcycle::Read | Microcycle::NewAddress | Microcycle::IsProgram - }; - Microcycle read_program { - Microcycle::Read | Microcycle::SameAddress | Microcycle::SelectWord | Microcycle::IsProgram - }; + static constexpr Microcycle::OperationT + ReadProgramAnnounceOperation = Microcycle::Read | Microcycle::NewAddress | Microcycle::IsProgram; + static constexpr Microcycle::OperationT + ReadProgramOperation = Microcycle::Read | Microcycle::SameAddress | Microcycle::SelectWord | Microcycle::IsProgram; + Microcycle read_program_announce { ReadProgramAnnounceOperation }; + Microcycle read_program { ReadProgramOperation }; // Read a data word or byte. Microcycle access_announce { @@ -200,21 +200,35 @@ struct ProcessorBase: public InstructionSet::M68k::NullFlowController { }; // TAS. + static constexpr Microcycle::OperationT + TASOperations[5] = { + Microcycle::Read | Microcycle::NewAddress | Microcycle::IsData, + Microcycle::Read | Microcycle::SameAddress | Microcycle::IsData | Microcycle::SelectByte, + Microcycle::SameAddress, + Microcycle::SameAddress | Microcycle::IsData, + Microcycle::SameAddress | Microcycle::IsData | Microcycle::SelectByte, + }; Microcycle tas_cycles[5] = { - { Microcycle::Read | Microcycle::NewAddress | Microcycle::IsData }, - { Microcycle::Read | Microcycle::SameAddress | Microcycle::IsData | Microcycle::SelectByte }, - { Microcycle::SameAddress }, - { Microcycle::SameAddress | Microcycle::IsData }, - { Microcycle::SameAddress | Microcycle::IsData | Microcycle::SelectByte }, + { TASOperations[0] }, + { TASOperations[1] }, + { TASOperations[2] }, + { TASOperations[3] }, + { TASOperations[4] }, }; // Reset. - Microcycle reset_cycle { Microcycle::Reset, HalfCycles(248) }; + static constexpr Microcycle::OperationT ResetOperation = Microcycle::Reset; + Microcycle reset_cycle { ResetOperation, HalfCycles(248) }; // Interrupt acknowledge. + static constexpr Microcycle::OperationT + InterruptCycleOperations[2] = { + Microcycle::InterruptAcknowledge | Microcycle::Read | Microcycle::NewAddress, + Microcycle::InterruptAcknowledge | Microcycle::Read | Microcycle::SameAddress | Microcycle::SelectByte + }; Microcycle interrupt_cycles[2] = { - { Microcycle::InterruptAcknowledge | Microcycle::Read | Microcycle::NewAddress }, - { Microcycle::InterruptAcknowledge | Microcycle::Read | Microcycle::SameAddress | Microcycle::SelectByte }, + { InterruptCycleOperations[0] }, + { InterruptCycleOperations[1] }, }; // Holding spot when awaiting DTACK/etc. From 032eeb475737a4228078b17d51fcbaef63ce9490 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Mon, 27 Nov 2023 14:57:41 -0500 Subject: [PATCH 2/5] Eliminate runtime switch. --- Components/68901/MFP68901.cpp | 59 +++++++++++-------- Components/68901/MFP68901.hpp | 6 +- Machines/Atari/ST/AtariST.cpp | 2 +- .../xcschemes/Clock Signal.xcscheme | 2 +- 4 files changed, 42 insertions(+), 27 deletions(-) diff --git a/Components/68901/MFP68901.cpp b/Components/68901/MFP68901.cpp index 7f2551fa2..454303504 100644 --- a/Components/68901/MFP68901.cpp +++ b/Components/68901/MFP68901.cpp @@ -179,32 +179,38 @@ void MFP68901::write(int address, uint8_t value) { update_clocking_observer(); } +template +void MFP68901::run_timer_for(int cycles) { + if(timers_[timer].mode >= TimerMode::Delay) { + // This code applies the timer prescaling only. prescale_count is used to count + // upwards rather than downwards for simplicity, but on the real hardware it's + // pretty safe to assume it actually counted downwards. So the clamp to 0 is + // because gymnastics may need to occur when the prescale value is altered, e.g. + // if a prescale of 256 is set and the prescale_count is currently 2 then the + // counter should roll over in 254 cycles. If the user at that point changes the + // prescale_count to 1 then the counter will need to be altered to -253 and + // allowed to keep counting up until it crosses both 0 and 1. + const int dividend = timers_[timer].prescale_count + cycles; + const int decrements = std::max(dividend / timers_[timer].prescale, 0); + if(decrements) { + decrement_timer(decrements); + timers_[timer].prescale_count = dividend % timers_[timer].prescale; + } else { + timers_[timer].prescale_count += cycles; + } + } +} + void MFP68901::run_for(HalfCycles time) { cycles_left_ += time; const int cycles = int(cycles_left_.flush().as_integral()); if(!cycles) return; - for(int c = 0; c < 4; ++c) { - if(timers_[c].mode >= TimerMode::Delay) { - // This code applies the timer prescaling only. prescale_count is used to count - // upwards rather than downwards for simplicity, but on the real hardware it's - // pretty safe to assume it actually counted downwards. So the clamp to 0 is - // because gymnastics may need to occur when the prescale value is altered, e.g. - // if a prescale of 256 is set and the prescale_count is currently 2 then the - // counter should roll over in 254 cycles. If the user at that point changes the - // prescale_count to 1 then the counter will need to be altered to -253 and - // allowed to keep counting up until it crosses both 0 and 1. - const int dividend = timers_[c].prescale_count + cycles; - const int decrements = std::max(dividend / timers_[c].prescale, 0); - if(decrements) { - decrement_timer(c, decrements); - timers_[c].prescale_count = dividend % timers_[c].prescale; - } else { - timers_[c].prescale_count += cycles; - } - } - } + run_timer_for<0>(cycles); + run_timer_for<1>(cycles); + run_timer_for<2>(cycles); + run_timer_for<3>(cycles); } HalfCycles MFP68901::next_sequence_point() { @@ -240,7 +246,8 @@ uint8_t MFP68901::get_timer_data(int timer) { return timers_[timer].value; } -void MFP68901::set_timer_event_input(int channel, bool value) { +template +void MFP68901::set_timer_event_input(bool value) { if(timers_[channel].event_input == value) return; timers_[channel].event_input = value; @@ -248,7 +255,7 @@ void MFP68901::set_timer_event_input(int channel, bool value) { // "The active state of the signal on TAI or TBI is dependent upon the associated // Interrupt Channel’s edge bit (GPIP 4 for TAI and GPIP 3 for TBI [...] ). // If the edge bit associated with the TAI or TBI input is a one, it will be active high. - decrement_timer(channel, 1); + decrement_timer(1); } // TODO: @@ -261,7 +268,13 @@ void MFP68901::set_timer_event_input(int channel, bool value) { // (the final bit probably explains 13 cycles of the DE to interrupt latency; not sure about the other ~15) } -void MFP68901::decrement_timer(int timer, int amount) { +template void MFP68901::set_timer_event_input<0>(bool); +template void MFP68901::set_timer_event_input<1>(bool); +template void MFP68901::set_timer_event_input<2>(bool); +template void MFP68901::set_timer_event_input<3>(bool); + +template +void MFP68901::decrement_timer(int amount) { while(amount--) { --timers_[timer].value; if(timers_[timer].value < 1) { diff --git a/Components/68901/MFP68901.hpp b/Components/68901/MFP68901.hpp index 9eabec622..5d268d93d 100644 --- a/Components/68901/MFP68901.hpp +++ b/Components/68901/MFP68901.hpp @@ -43,7 +43,8 @@ class MFP68901: public ClockingHint::Source { HalfCycles next_sequence_point(); /// Sets the current level of either of the timer event inputs — TAI and TBI in datasheet terms. - void set_timer_event_input(int channel, bool value); + template + void set_timer_event_input(bool value); /// Sets a port handler, a receiver that will be notified upon any change in GPIP output. /// @@ -86,7 +87,8 @@ class MFP68901: public ClockingHint::Source { void set_timer_mode(int timer, TimerMode, int prescale, bool reset_timer); void set_timer_data(int timer, uint8_t); uint8_t get_timer_data(int timer); - void decrement_timer(int timer, int amount); + template void decrement_timer(int amount); + template void run_timer_for(int cycles); struct Timer { TimerMode mode = TimerMode::Stopped; diff --git a/Machines/Atari/ST/AtariST.cpp b/Machines/Atari/ST/AtariST.cpp index f6a2d8163..143e15d1b 100644 --- a/Machines/Atari/ST/AtariST.cpp +++ b/Machines/Atari/ST/AtariST.cpp @@ -484,7 +484,7 @@ class ConcreteMachine: length -= video_.cycles_until_implicit_flush(); video_ += video_.cycles_until_implicit_flush(); - mfp_->set_timer_event_input(1, video_->display_enabled()); + mfp_->set_timer_event_input<1>(video_->display_enabled()); update_interrupt_input(); } diff --git a/OSBindings/Mac/Clock Signal.xcodeproj/xcshareddata/xcschemes/Clock Signal.xcscheme b/OSBindings/Mac/Clock Signal.xcodeproj/xcshareddata/xcschemes/Clock Signal.xcscheme index 474fa352e..19b54b90e 100644 --- a/OSBindings/Mac/Clock Signal.xcodeproj/xcshareddata/xcschemes/Clock Signal.xcscheme +++ b/OSBindings/Mac/Clock Signal.xcodeproj/xcshareddata/xcschemes/Clock Signal.xcscheme @@ -62,7 +62,7 @@ Date: Mon, 27 Nov 2023 15:16:22 -0500 Subject: [PATCH 3/5] Mildly adjust layout of inner loop. --- Components/68901/MFP68901.cpp | 48 +++++++++++++++++++++++++---------- 1 file changed, 34 insertions(+), 14 deletions(-) diff --git a/Components/68901/MFP68901.cpp b/Components/68901/MFP68901.cpp index 454303504..d1dcd782b 100644 --- a/Components/68901/MFP68901.cpp +++ b/Components/68901/MFP68901.cpp @@ -275,21 +275,41 @@ template void MFP68901::set_timer_event_input<3>(bool); template void MFP68901::decrement_timer(int amount) { - while(amount--) { - --timers_[timer].value; - if(timers_[timer].value < 1) { - switch(timer) { - case 0: begin_interrupts(Interrupt::TimerA); break; - case 1: begin_interrupts(Interrupt::TimerB); break; - case 2: begin_interrupts(Interrupt::TimerC); break; - case 3: begin_interrupts(Interrupt::TimerD); break; - } + while(amount) { + if(timers_[timer].value > amount) { + timers_[timer].value -= amount; + return; + } - // Re: reloading when in event counting mode; I found the data sheet thoroughly unclear on - // this, but it appears empirically to be correct. See e.g. Pompey Pirates menu 27. - if(timers_[timer].mode == TimerMode::Delay || timers_[timer].mode == TimerMode::EventCount) { - timers_[timer].value += timers_[timer].reload_value; // TODO: properly. - } + // Keep this check here to avoid the case where a decrement to zero occurs during one call to + // decrement_timer, triggering an interrupt, then the timer is already 0 at the next instance, + // causing a second interrupt. + // + // ... even though it would be nice to move it down below, after value has overtly been set to 0. + if(!timers_[timer].value) { + --timers_[timer].value; + --amount; + continue; + } + + // If here then amount is sufficient to, at least once, decrement the timer + // from 1 to 0. So there's an interrupt. + // + // (and, this switch is why this function is templated on timer ID) + switch(timer) { + case 0: begin_interrupts(Interrupt::TimerA); break; + case 1: begin_interrupts(Interrupt::TimerB); break; + case 2: begin_interrupts(Interrupt::TimerC); break; + case 3: begin_interrupts(Interrupt::TimerD); break; + } + + // Re: reloading when in event counting mode; I found the data sheet thoroughly unclear on + // this, but it appears empirically to be correct. See e.g. Pompey Pirates menu 27. + amount -= timers_[timer].value; + if(timers_[timer].mode == TimerMode::Delay || timers_[timer].mode == TimerMode::EventCount) { + timers_[timer].value = timers_[timer].reload_value; // TODO: properly. + } else { + timers_[timer].value = 0; } } } From 87eec47b798284664b8facea17dd8ee1907bcefd Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Mon, 27 Nov 2023 15:48:30 -0500 Subject: [PATCH 4/5] Mildly reduce cost of 8-byte ROM overlay. --- Machines/Atari/ST/AtariST.cpp | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/Machines/Atari/ST/AtariST.cpp b/Machines/Atari/ST/AtariST.cpp index 143e15d1b..935d26b63 100644 --- a/Machines/Atari/ST/AtariST.cpp +++ b/Machines/Atari/ST/AtariST.cpp @@ -100,8 +100,7 @@ class ConcreteMachine: Memory::PackBigEndian16(roms.find(rom_name)->second, rom_); // Set up basic memory map. - memory_map_[0] = BusDevice::MostlyRAM; - int c = 1; + int c = 0; for(; c < int(ram_.size() >> 16); ++c) memory_map_[c] = BusDevice::RAM; for(; c < 0x40; ++c) memory_map_[c] = BusDevice::Floating; for(; c < 0xff; ++c) memory_map_[c] = BusDevice::Unassigned; @@ -118,6 +117,9 @@ class ConcreteMachine: memory_map_[0xfa] = memory_map_[0xfb] = BusDevice::Cartridge; memory_map_[0xff] = BusDevice::IO; + // Copy the first 8 bytes of ROM into RAM. + reinstall_rom_vector(); + midi_acia_->set_interrupt_delegate(this); keyboard_acia_->set_interrupt_delegate(this); @@ -249,18 +251,15 @@ class ConcreteMachine: uint8_t *memory = nullptr; switch(memory_map_[address >> 16]) { default: - case BusDevice::MostlyRAM: - if(address < 8) { - memory = rom_.data(); - break; - } - [[fallthrough]]; case BusDevice::RAM: memory = ram_.data(); break; case BusDevice::ROM: memory = rom_.data(); + if(!(operation & Microcycle::Read)) { + return delay; + } address -= rom_start_; break; @@ -408,6 +407,9 @@ class ConcreteMachine: } // If control has fallen through to here, the access is either a read from ROM, or a read or write to RAM. + // + // In both write cases, immediately reinstall the first eight bytes of RAM from ROM, so that any write to + // that area is in effect a no-op. This is cheaper than the conditionality of actually checking. switch(operation & (Microcycle::SelectWord | Microcycle::SelectByte | Microcycle::Read)) { default: break; @@ -422,17 +424,23 @@ class ConcreteMachine: if(address >= video_range_.low_address && address < video_range_.high_address) video_.flush(); *reinterpret_cast(&memory[address]) = cycle.value->w; + reinstall_rom_vector(); break; case Microcycle::SelectByte: if(address >= video_range_.low_address && address < video_range_.high_address) video_.flush(); memory[address] = cycle.value->b; + reinstall_rom_vector(); break; } return HalfCycles(0); } + void reinstall_rom_vector() { + std::copy(rom_.begin(), rom_.begin() + 8, ram_.begin()); + } + void flush_output(int outputs) final { dma_.flush(); mfp_.flush(); @@ -520,8 +528,6 @@ class ConcreteMachine: uint32_t rom_start_ = 0; enum class BusDevice { - /// A mostly RAM page is one that returns ROM for the first 8 bytes, RAM elsewhere. - MostlyRAM, /// Allows reads and writes to ram_. RAM, /// Nothing is mapped to this area, and it also doesn't trigger an exception upon access. From 36a4629ce0abf1960652313b62b33717d8c303b5 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Mon, 27 Nov 2023 21:49:57 -0500 Subject: [PATCH 5/5] Explain new semantics. --- Processors/68000/68000.hpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Processors/68000/68000.hpp b/Processors/68000/68000.hpp index 659c59bf4..ba6732710 100644 --- a/Processors/68000/68000.hpp +++ b/Processors/68000/68000.hpp @@ -346,7 +346,12 @@ class BusHandler { FC0 and FC1 are provided inside the microcycle as the IsData and IsProgram flags; FC2 is provided here as is_supervisor — it'll be either 0 or 1. + + If @c operation is any value other than Microcycle::DecodeDynamically then it + can be used to select an appropriate execution path at compile time. Otherwise + cycle.operation must be inspected at runtime. */ + template HalfCycles perform_bus_operation([[maybe_unused]] const Microcycle &cycle, [[maybe_unused]] int is_supervisor) { return HalfCycles(0); }