From 42024c15737d613aa904b771fdb2a87e5be693a5 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Mon, 7 Aug 2023 09:19:20 -0400 Subject: [PATCH 1/9] Don't allow setting of an invalid S. --- Processors/65816/Implementation/65816Base.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Processors/65816/Implementation/65816Base.cpp b/Processors/65816/Implementation/65816Base.cpp index 3eaf614cd..eeaab17d8 100644 --- a/Processors/65816/Implementation/65816Base.cpp +++ b/Processors/65816/Implementation/65816Base.cpp @@ -30,7 +30,10 @@ uint16_t ProcessorBase::value_of(Register r) const { void ProcessorBase::set_value_of(Register r, uint16_t value) { switch (r) { case Register::ProgramCounter: registers_.pc = value; break; - case Register::StackPointer: registers_.s.full = value; break; + case Register::StackPointer: + registers_.s.full = value; + if(registers_.emulation_flag) registers_.s.halves.high = 1; + break; case Register::Flags: set_flags(uint8_t(value)); break; case Register::A: registers_.a.full = value; break; case Register::X: registers_.x.full = value & registers_.x_mask; break; From 0a336baae29643fa06376ff3e2e43eeae5db8cd1 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Thu, 17 Aug 2023 14:50:43 -0400 Subject: [PATCH 2/9] Perform minor generalisation. --- .../65816ComparativeTests.mm | 34 ++++++++++++------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/OSBindings/Mac/Clock SignalTests/65816ComparativeTests.mm b/OSBindings/Mac/Clock SignalTests/65816ComparativeTests.mm index 6017f35ac..3dcd9fd85 100644 --- a/OSBindings/Mac/Clock SignalTests/65816ComparativeTests.mm +++ b/OSBindings/Mac/Clock SignalTests/65816ComparativeTests.mm @@ -14,16 +14,21 @@ #include #include +#include "6502Selector.hpp" + namespace { struct StopException {}; -struct BusHandler: public CPU::MOS6502Esque::BusHandler { - // Use a map to store RAM contents, in order to preserve initialised state. - std::unordered_map ram; - std::unordered_map inventions; +template +struct BusHandler: public CPU::MOS6502Esque::BusHandlerT { + using AddressType = typename CPU::MOS6502Esque::BusHandlerT::AddressType; - Cycles perform_bus_operation(CPU::MOS6502Esque::BusOperation operation, uint32_t address, uint8_t *value) { + // Use a map to store RAM contents, in order to preserve initialised state. + std::unordered_map ram; + std::unordered_map inventions; + + Cycles perform_bus_operation(CPU::MOS6502Esque::BusOperation operation, AddressType address, uint8_t *value) { // Record the basics of the operation. auto &cycle = cycles.emplace_back(); cycle.operation = operation; @@ -92,25 +97,27 @@ struct BusHandler: public CPU::MOS6502Esque::BusHandler { allow_pc_repetition = opcode == 0x54 || opcode == 0x44; using Register = CPU::MOS6502Esque::Register; - const uint32_t pc = - processor.value_of(Register::ProgramCounter) | - (processor.value_of(Register::ProgramBank) << 16); + const auto pc = + AddressT( + processor.value_of(Register::ProgramCounter) | + (processor.value_of(Register::ProgramBank) << 16) + ); inventions[pc] = ram[pc] = opcode; } int pc_overshoot = 0; - std::optional initial_pc; + std::optional initial_pc; bool allow_pc_repetition = false; struct Cycle { CPU::MOS6502Esque::BusOperation operation; - std::optional address; + std::optional address; std::optional value; int extended_bus; }; std::vector cycles; - CPU::WDC65816::Processor processor; + CPU::MOS6502Esque::Processor, false> processor; BusHandler() : processor(*this) { // Never run the official reset procedure. @@ -132,7 +139,8 @@ template void print_registers(FILE *file, const Processor & fprintf(file, "\"e\": %d, ", processor.value_of(Register::EmulationFlag)); } -void print_ram(FILE *file, const std::unordered_map &data) { +template +void print_ram(FILE *file, const std::unordered_map &data) { fprintf(file, "\"ram\": ["); bool is_first = true; for(const auto &pair: data) { @@ -153,7 +161,7 @@ void print_ram(FILE *file, const std::unordered_map &data) { @implementation TestGenerator - (void)generate { - BusHandler handler; + BusHandler handler; NSString *const tempDir = NSTemporaryDirectory(); NSLog(@"Outputting to %@", tempDir); From 833613b68ae74faca0ee14788a0034e33a06853c Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Thu, 17 Aug 2023 14:50:55 -0400 Subject: [PATCH 3/9] Fix S top byte overwrite. --- Processors/65816/Implementation/65816Implementation.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Processors/65816/Implementation/65816Implementation.hpp b/Processors/65816/Implementation/65816Implementation.hpp index b980bbe73..be305499a 100644 --- a/Processors/65816/Implementation/65816Implementation.hpp +++ b/Processors/65816/Implementation/65816Implementation.hpp @@ -69,7 +69,7 @@ template void Processor Date: Thu, 17 Aug 2023 14:50:55 -0400 Subject: [PATCH 4/9] Fix S top byte overwrite. --- Processors/65816/Implementation/65816Implementation.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Processors/65816/Implementation/65816Implementation.hpp b/Processors/65816/Implementation/65816Implementation.hpp index b980bbe73..be305499a 100644 --- a/Processors/65816/Implementation/65816Implementation.hpp +++ b/Processors/65816/Implementation/65816Implementation.hpp @@ -69,7 +69,7 @@ template void Processor Date: Thu, 17 Aug 2023 15:24:08 -0400 Subject: [PATCH 5/9] Omit unsupported registers and flags. --- .../65816ComparativeTests.mm | 60 +++++++++++-------- Processors/6502Esque/6502Selector.hpp | 23 +++++++ 2 files changed, 57 insertions(+), 26 deletions(-) diff --git a/OSBindings/Mac/Clock SignalTests/65816ComparativeTests.mm b/OSBindings/Mac/Clock SignalTests/65816ComparativeTests.mm index 3dcd9fd85..b2a8137b3 100644 --- a/OSBindings/Mac/Clock SignalTests/65816ComparativeTests.mm +++ b/OSBindings/Mac/Clock SignalTests/65816ComparativeTests.mm @@ -98,7 +98,7 @@ struct BusHandler: public CPU::MOS6502Esque::BusHandlerT { using Register = CPU::MOS6502Esque::Register; const auto pc = - AddressT( + AddressType( processor.value_of(Register::ProgramCounter) | (processor.value_of(Register::ProgramBank) << 16) ); @@ -125,7 +125,7 @@ struct BusHandler: public CPU::MOS6502Esque::BusHandlerT { } }; -template void print_registers(FILE *file, const Processor &processor, int pc_offset) { +template void print_registers(FILE *file, const Processor &processor, int pc_offset) { using Register = CPU::MOS6502Esque::Register; fprintf(file, "\"pc\": %d, ", (processor.value_of(Register::ProgramCounter) + pc_offset) & 65535); fprintf(file, "\"s\": %d, ", processor.value_of(Register::StackPointer)); @@ -133,10 +133,12 @@ template void print_registers(FILE *file, const Processor & fprintf(file, "\"a\": %d, ", processor.value_of(Register::A)); fprintf(file, "\"x\": %d, ", processor.value_of(Register::X)); fprintf(file, "\"y\": %d, ", processor.value_of(Register::Y)); - fprintf(file, "\"dbr\": %d, ", processor.value_of(Register::DataBank)); - fprintf(file, "\"d\": %d, ", processor.value_of(Register::Direct)); - fprintf(file, "\"pbr\": %d, ", processor.value_of(Register::ProgramBank)); - fprintf(file, "\"e\": %d, ", processor.value_of(Register::EmulationFlag)); + if constexpr (has_emulation) { + fprintf(file, "\"dbr\": %d, ", processor.value_of(Register::DataBank)); + fprintf(file, "\"d\": %d, ", processor.value_of(Register::Direct)); + fprintf(file, "\"pbr\": %d, ", processor.value_of(Register::ProgramBank)); + fprintf(file, "\"e\": %d, ", processor.value_of(Register::EmulationFlag)); + } } template @@ -151,22 +153,18 @@ void print_ram(FILE *file, const std::unordered_map &data) { fprintf(file, "]"); } -} // MARK: - New test generator. -@interface TestGenerator : NSObject -@end -@implementation TestGenerator - -- (void)generate { - BusHandler handler; +template void generate() { + BusHandler handler; + constexpr bool has_emulation = has(type, CPU::MOS6502Esque::Register::EmulationFlag); NSString *const tempDir = NSTemporaryDirectory(); NSLog(@"Outputting to %@", tempDir); - for(int operation = 0; operation < 512; operation++) { + for(int operation = 0; operation < (has_emulation ? 512 : 256); operation++) { // Make tests repeatable, at least for any given instance of // the runtime. srand(65816 + operation); @@ -174,7 +172,10 @@ void print_ram(FILE *file, const std::unordered_map &data) { const bool is_emulated = operation & 256; const uint8_t opcode = operation & 255; - NSString *const targetName = [NSString stringWithFormat:@"%@%02x.%c.json", tempDir, opcode, is_emulated ? 'e' : 'n']; + NSString *const targetName = + has_emulation ? + [NSString stringWithFormat:@"%@%02x.%c.json", tempDir, opcode, is_emulated ? 'e' : 'n'] : + [NSString stringWithFormat:@"%@%02x.json", tempDir, opcode]; FILE *const target = fopen(targetName.UTF8String, "wt"); bool is_first_test = true; @@ -194,21 +195,28 @@ void print_ram(FILE *file, const std::unordered_map &data) { handler.processor.set_value_of(Register::Y, rand() >> 8); handler.processor.set_value_of(Register::ProgramCounter, rand() >> 8); handler.processor.set_value_of(Register::StackPointer, rand() >> 8); - handler.processor.set_value_of(Register::DataBank, rand() >> 8); - handler.processor.set_value_of(Register::ProgramBank, rand() >> 8); - handler.processor.set_value_of(Register::Direct, rand() >> 8); - // ... except for emulation mode, which is a given. - // And is set last to ensure proper internal state is applied. - handler.processor.set_value_of(Register::EmulationFlag, is_emulated); + if(has_emulation) { + handler.processor.set_value_of(Register::DataBank, rand() >> 8); + handler.processor.set_value_of(Register::ProgramBank, rand() >> 8); + handler.processor.set_value_of(Register::Direct, rand() >> 8); + + // ... except for emulation mode, which is a given. + // And is set last to ensure proper internal state is applied. + handler.processor.set_value_of(Register::EmulationFlag, is_emulated); + } // Establish the opcode. handler.setup(opcode); // Dump initial state. - fprintf(target, "{ \"name\": \"%02x %c %d\", ", opcode, is_emulated ? 'e' : 'n', test + 1); + if(has_emulation) { + fprintf(target, "{ \"name\": \"%02x %c %d\", ", opcode, is_emulated ? 'e' : 'n', test + 1); + } else { + fprintf(target, "{ \"name\": \"%02x %d\", ", opcode, test + 1); + } fprintf(target, "\"initial\": {"); - print_registers(target, handler.processor, 0); + print_registers(target, handler.processor, 0); // Run to the second opcode fetch. try { @@ -220,7 +228,7 @@ void print_ram(FILE *file, const std::unordered_map &data) { // Dump final state. fprintf(target, "}, \"final\": {"); - print_registers(target, handler.processor, handler.pc_overshoot); + print_registers(target, handler.processor, handler.pc_overshoot); print_ram(target, handler.ram); fprintf(target, "}, "); @@ -293,7 +301,7 @@ void print_ram(FILE *file, const std::unordered_map &data) { } } -@end +} // MARK: - Existing test evaluator. @@ -304,7 +312,7 @@ void print_ram(FILE *file, const std::unordered_map &data) { // A generator for tests; not intended to be a permanent fixture. //- (void)testGenerate { -// [[[TestGenerator alloc] init] generate]; +// generate(); //} @end diff --git a/Processors/6502Esque/6502Selector.hpp b/Processors/6502Esque/6502Selector.hpp index 2b75d33d6..642eb71eb 100644 --- a/Processors/6502Esque/6502Selector.hpp +++ b/Processors/6502Esque/6502Selector.hpp @@ -9,6 +9,7 @@ #ifndef _502Selector_h #define _502Selector_h +#include "6502Esque.hpp" #include "../6502/6502.hpp" #include "../65816/65816.hpp" @@ -45,6 +46,28 @@ template class Processor class BusHandlerT: public BusHandler {}; template <> class BusHandlerT: public BusHandler {}; +/* + Query for implemented registers. +*/ +constexpr bool has(Type processor_type, Register r) { + switch(r) { + case Register::LastOperationAddress: + case Register::ProgramCounter: + case Register::StackPointer: + case Register::Flags: + case Register::A: + case Register::X: + case Register::Y: + return true; + + case Register::EmulationFlag: + case Register::DataBank: + case Register::ProgramBank: + case Register::Direct: + return processor_type == Type::TWDC65816; + } +} + } #endif /* _502Selector_h */ From 26343148aea700166e4fbbc376dac1f43685862e Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Thu, 17 Aug 2023 15:32:02 -0400 Subject: [PATCH 6/9] Use simplified control lines when appropriate. --- .../65816ComparativeTests.mm | 47 ++++++++++++------- Processors/6502Esque/6502Selector.hpp | 4 ++ 2 files changed, 33 insertions(+), 18 deletions(-) diff --git a/OSBindings/Mac/Clock SignalTests/65816ComparativeTests.mm b/OSBindings/Mac/Clock SignalTests/65816ComparativeTests.mm index b2a8137b3..9426756d8 100644 --- a/OSBindings/Mac/Clock SignalTests/65816ComparativeTests.mm +++ b/OSBindings/Mac/Clock SignalTests/65816ComparativeTests.mm @@ -32,7 +32,9 @@ struct BusHandler: public CPU::MOS6502Esque::BusHandlerT { // Record the basics of the operation. auto &cycle = cycles.emplace_back(); cycle.operation = operation; - cycle.extended_bus = processor.get_extended_bus_output(); + if constexpr (has_extended_bus_output(type)) { + cycle.extended_bus = processor.get_extended_bus_output(); + } // Perform the operation, and fill in the cycle's value. using BusOperation = CPU::MOS6502Esque::BusOperation; @@ -113,7 +115,7 @@ struct BusHandler: public CPU::MOS6502Esque::BusHandlerT { CPU::MOS6502Esque::BusOperation operation; std::optional address; std::optional value; - int extended_bus; + int extended_bus = 0; }; std::vector cycles; @@ -263,12 +265,6 @@ template void generate() { assert(false); } - using ExtendedBusOutput = CPU::WDC65816::ExtendedBusOutput; - const bool emulation = cycle.extended_bus & ExtendedBusOutput::Emulation; - const bool memory_size = cycle.extended_bus & ExtendedBusOutput::MemorySize; - const bool index_size = cycle.extended_bus & ExtendedBusOutput::IndexSize; - const bool memory_lock = cycle.extended_bus & ExtendedBusOutput::MemoryLock; - fprintf(target, "["); if(cycle.address) { fprintf(target, "%d, ", *cycle.address); @@ -280,16 +276,31 @@ template void generate() { } else { fprintf(target, "null, "); } - fprintf(target, "\"%c%c%c%c%c%c%c%c\"]", - vda ? 'd' : '-', - vpa ? 'p' : '-', - vpb ? 'v' : '-', - wait ? '-' : (read ? 'r' : 'w'), - wait ? '-' : (emulation ? 'e' : '-'), - wait ? '-' : (memory_size ? 'm' : '-'), - wait ? '-' : (index_size ? 'x' : '-'), - wait ? '-' : (memory_lock ? 'l' : '-') - ); + + if(has_emulation) { + using ExtendedBusOutput = CPU::WDC65816::ExtendedBusOutput; + const bool emulation = cycle.extended_bus & ExtendedBusOutput::Emulation; + const bool memory_size = cycle.extended_bus & ExtendedBusOutput::MemorySize; + const bool index_size = cycle.extended_bus & ExtendedBusOutput::IndexSize; + const bool memory_lock = cycle.extended_bus & ExtendedBusOutput::MemoryLock; + + fprintf(target, "\"%c%c%c%c%c%c%c%c\"]", + vda ? 'd' : '-', + vpa ? 'p' : '-', + vpb ? 'v' : '-', + wait ? '-' : (read ? 'r' : 'w'), + wait ? '-' : (emulation ? 'e' : '-'), + wait ? '-' : (memory_size ? 'm' : '-'), + wait ? '-' : (index_size ? 'x' : '-'), + wait ? '-' : (memory_lock ? 'l' : '-') + ); + } else { + if(read) { + fprintf(target, "\"read\"]"); + } else { + fprintf(target, "\"write\"]"); + } + } } // Terminate object. diff --git a/Processors/6502Esque/6502Selector.hpp b/Processors/6502Esque/6502Selector.hpp index 642eb71eb..31bf93eaa 100644 --- a/Processors/6502Esque/6502Selector.hpp +++ b/Processors/6502Esque/6502Selector.hpp @@ -68,6 +68,10 @@ constexpr bool has(Type processor_type, Register r) { } } +constexpr bool has_extended_bus_output(Type processor_type) { + return processor_type == Type::TWDC65816; +} + } #endif /* _502Selector_h */ From d9df568dabf466a2c3cc0c61b02bba5225ed2147 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Thu, 17 Aug 2023 15:38:28 -0400 Subject: [PATCH 7/9] Add faulty restart_operation_fetch. --- Processors/6502/6502.hpp | 6 ++++++ Processors/6502/Implementation/6502Implementation.hpp | 4 ++++ 2 files changed, 10 insertions(+) diff --git a/Processors/6502/6502.hpp b/Processors/6502/6502.hpp index 8ed3ff0f9..15edc54cd 100644 --- a/Processors/6502/6502.hpp +++ b/Processors/6502/6502.hpp @@ -124,6 +124,12 @@ class ProcessorBase: public ProcessorStorage { @returns @c true if the 6502 is jammed; @c false otherwise. */ inline bool is_jammed() const; + + /*! + FOR TESTING PURPOSES ONLY: forces the processor into a state where + the next thing it intends to do is fetch a new opcode. + */ + inline void restart_operation_fetch(); }; /*! diff --git a/Processors/6502/Implementation/6502Implementation.hpp b/Processors/6502/Implementation/6502Implementation.hpp index d07bfe9ee..9fb8b7e9a 100644 --- a/Processors/6502/Implementation/6502Implementation.hpp +++ b/Processors/6502/Implementation/6502Implementation.hpp @@ -728,3 +728,7 @@ void ProcessorBase::set_value_of(Register r, uint16_t value) { default: break; } } + +void ProcessorBase::restart_operation_fetch() { + scheduled_program_counter_ = nullptr; +} From ca75822dbeb5881b97658fb23ae7a8bda3cb72d2 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Thu, 17 Aug 2023 15:42:34 -0400 Subject: [PATCH 8/9] Fix restart_operation_fetch. --- Processors/6502/Implementation/6502Implementation.hpp | 1 + 1 file changed, 1 insertion(+) diff --git a/Processors/6502/Implementation/6502Implementation.hpp b/Processors/6502/Implementation/6502Implementation.hpp index 9fb8b7e9a..80b0d7a00 100644 --- a/Processors/6502/Implementation/6502Implementation.hpp +++ b/Processors/6502/Implementation/6502Implementation.hpp @@ -731,4 +731,5 @@ void ProcessorBase::set_value_of(Register r, uint16_t value) { void ProcessorBase::restart_operation_fetch() { scheduled_program_counter_ = nullptr; + next_bus_operation_ = BusOperation::None; } From b34403164ecb151c5eda7fbae42410c9c8462ef6 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Fri, 18 Aug 2023 14:30:40 -0400 Subject: [PATCH 9/9] Abstract out VGC interrupt register; fix clearing bug. --- Machines/Apple/AppleIIgs/Video.cpp | 18 ++++------- Machines/Apple/AppleIIgs/Video.hpp | 52 ++++++++++++++++++++++++++++-- 2 files changed, 56 insertions(+), 14 deletions(-) diff --git a/Machines/Apple/AppleIIgs/Video.cpp b/Machines/Apple/AppleIIgs/Video.cpp index e78e6ff4b..3f6269193 100644 --- a/Machines/Apple/AppleIIgs/Video.cpp +++ b/Machines/Apple/AppleIIgs/Video.cpp @@ -285,7 +285,7 @@ void Video::output_row(int row, int start, int end) { // Post an interrupt if requested. if(line_control_ & 0x40) { - set_interrupts(0x20); + interrupts_.add(0x20); } // Set up appropriately for fill mode (or not). @@ -498,19 +498,19 @@ uint8_t Video::get_new_video() { } void Video::clear_interrupts(uint8_t mask) { - set_interrupts(interrupts_ & ~(mask & 0x60)); + interrupts_.clear(mask); } void Video::set_interrupt_register(uint8_t mask) { - set_interrupts(interrupts_ | (mask & 0x6)); + interrupts_.set_control(mask); } uint8_t Video::get_interrupt_register() { - return interrupts_; + return interrupts_.status(); } bool Video::get_interrupt_line() { - return (interrupts_&0x80) || (megaii_interrupt_mask_&megaii_interrupt_state_); + return interrupts_.active() || (megaii_interrupt_mask_ & megaii_interrupt_state_); } void Video::set_megaii_interrupts_enabled(uint8_t mask) { @@ -526,13 +526,7 @@ void Video::clear_megaii_interrupts() { } void Video::notify_clock_tick() { - set_interrupts(interrupts_ | 0x40); -} - -void Video::set_interrupts(uint8_t new_value) { - interrupts_ = new_value & 0x7f; - if((interrupts_ >> 4) & interrupts_ & 0x6) - interrupts_ |= 0x80; + interrupts_.add(0x40); } void Video::set_border_colour(uint8_t colour) { diff --git a/Machines/Apple/AppleIIgs/Video.hpp b/Machines/Apple/AppleIIgs/Video.hpp index a6f5c9788..9d30f2dff 100644 --- a/Machines/Apple/AppleIIgs/Video.hpp +++ b/Machines/Apple/AppleIIgs/Video.hpp @@ -123,8 +123,56 @@ class Video: public Apple::II::VideoSwitches { void advance(Cycles); uint8_t new_video_ = 0x01; - uint8_t interrupts_ = 0x00; - void set_interrupts(uint8_t); + + class Interrupts { + public: + void add(uint8_t value) { + // Taken literally, status accumulates regardless of being enabled, + // potentially to be polled, it simply doesn't trigger an interrupt. + value_ |= value; + test(); + } + + void clear(uint8_t value) { + // Zeroes in bits 5 or 6 clear the respective interrupts. + value_ &= value | ~0x60; + test(); + } + + void set_control(uint8_t value) { + // Ones in bits 1 or 2 enable the respective interrupts. + value_ = (value_ & ~0x6) | (value & 0x6); + test(); + } + + uint8_t status() const { + return value_; + } + + bool active() const { + return value_ & 0x80; + } + + private: + void test() { + value_ &= 0x7f; + if((value_ >> 4) & value_ & 0x6) { + value_ |= 0x80; + } + } + + // Overall meaning of value is as per the VGC interrupt register, i.e. + // + // b7: interrupt status; + // b6: 1-second interrupt status; + // b5: scan-line interrupt status; + // b4: reserved; + // b3: reserved; + // b2: 1-second interrupt enable; + // b1: scan-line interrupt enable; + // b0: reserved. + uint8_t value_ = 0x00; + } interrupts_; int cycles_into_frame_ = 0; const uint8_t *ram_ = nullptr;