From d9598b35c25a68cdeaec97bf28dae1aee0c0955a Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Thu, 23 Dec 2021 16:27:54 -0500 Subject: [PATCH 1/4] Add some additional metrics. --- Processors/68000/Implementation/68000Storage.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/Processors/68000/Implementation/68000Storage.cpp b/Processors/68000/Implementation/68000Storage.cpp index 96932a669..4c6c9a969 100644 --- a/Processors/68000/Implementation/68000Storage.cpp +++ b/Processors/68000/Implementation/68000Storage.cpp @@ -3151,9 +3151,11 @@ struct ProcessorStorageConstructor { // }; // Finalise micro-op and program pointers. + int total_instructions = 0; for(size_t instruction = 0; instruction < 65536; ++instruction) { if(micro_op_pointers[instruction] != std::numeric_limits::max()) { storage_.instructions[instruction].micro_operations = uint32_t(micro_op_pointers[instruction]); + ++total_instructions; // link_operations(&storage_.all_micro_ops_[micro_op_pointers[instruction]], &arbitrary_base); } } @@ -3162,8 +3164,10 @@ struct ProcessorStorageConstructor { storage_.interrupt_micro_ops_ = &storage_.all_micro_ops_[interrupt_pointer]; // link_operations(storage_.interrupt_micro_ops_, &arbitrary_base); - std::cout << storage_.all_bus_steps_.size() << " total bus steps" << std::endl; - std::cout << storage_.all_micro_ops_.size() << " total micro ops" << std::endl; + std::cout << total_instructions << " total opcodes" << std::endl; + std::cout << storage_.all_bus_steps_.size() << " total bus steps; at " << sizeof(BusStep) << " bytes per bus step implies " << storage_.all_bus_steps_.size() * sizeof(BusStep) << " bytes" << std::endl; + std::cout << storage_.all_micro_ops_.size() << " total micro ops; at " << sizeof(MicroOp) << " bytes per micro-op implies " << storage_.all_micro_ops_.size() * sizeof(MicroOp) << " bytes" << std::endl; + std::cout << "(Microcycles being " << sizeof(Microcycle) << " bytes; ProcessorStorage being " << sizeof(ProcessorStorage) << " bytes, or " << sizeof(ProcessorStorage) - sizeof(ProcessorStorage::instructions) << " bytes without the instructions table)" << std::endl; } private: From 32e0a666108e422d90eae9a538f471806c601ea1 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Thu, 23 Dec 2021 16:28:55 -0500 Subject: [PATCH 2/4] Trust the compiler with this bit field. --- .../Implementation/68000Implementation.hpp | 6 ++-- .../68000/Implementation/68000Storage.hpp | 35 +++++++++++++------ 2 files changed, 27 insertions(+), 14 deletions(-) diff --git a/Processors/68000/Implementation/68000Implementation.hpp b/Processors/68000/Implementation/68000Implementation.hpp index ca8449fb4..380e6a977 100644 --- a/Processors/68000/Implementation/68000Implementation.hpp +++ b/Processors/68000/Implementation/68000Implementation.hpp @@ -333,7 +333,7 @@ template void Proces #endif if(instructions[decoded_instruction_.full].micro_operations != std::numeric_limits::max()) { - if((instructions[decoded_instruction_.full].source_dest & 0x80) && !is_supervisor_) { + if((instructions[decoded_instruction_.full].requires_supervisor) && !is_supervisor_) { // A privilege violation has been detected. active_program_ = nullptr; active_micro_op_ = short_exception_micro_ops_; @@ -387,9 +387,9 @@ template void Proces #define offset_pointer(x) reinterpret_cast(&reinterpret_cast(static_cast(this))[x]) #define source() offset_pointer(active_program_->source_offset) -#define source_address() address_[(active_program_->source_dest >> 4) & 7] +#define source_address() address_[active_program_->source] #define destination() offset_pointer(active_program_->destination_offset) -#define destination_address() address_[active_program_->source_dest & 7] +#define destination_address() address_[active_program_->dest] case int_type(MicroOp::Action::PerformOperation): #define sub_overflow() ((result ^ destination) & (destination ^ source)) diff --git a/Processors/68000/Implementation/68000Storage.hpp b/Processors/68000/Implementation/68000Storage.hpp index e2b03d948..c87f40b55 100644 --- a/Processors/68000/Implementation/68000Storage.hpp +++ b/Processors/68000/Implementation/68000Storage.hpp @@ -359,6 +359,15 @@ class ProcessorStorage { remaining fields, with no additional padding being inserted by the compiler. */ struct Program { + Program() { + // Initialisers for bitfields aren't available until C++20. So, yuck, do it manually. + requires_supervisor = 0; + source = 0; + dest = 0; + destination_offset = 0; + source_offset = 0; + } + /// The offset into the all_micro_ops_ at which micro-ops for this instruction begin, /// or std::numeric_limits::max() if this is an invalid Program. uint32_t micro_operations = std::numeric_limits::max(); @@ -366,26 +375,30 @@ class ProcessorStorage { Operation operation; /// The number of bytes after the beginning of an instance of ProcessorStorage that the RegisterPair32 containing /// a source value for this operation lies at. - uint8_t source_offset = 0; + uint8_t source_offset: 7; /// The number of bytes after the beginning of an instance of ProcessorStorage that the RegisterPair32 containing /// a destination value for this operation lies at. - uint8_t destination_offset = 0; - /// A bitfield comprised of: - /// b7 = set if this program requires supervisor mode; - /// b0–b2 = the source address register (for pre-decrement and post-increment actions); and - /// b4-b6 = destination address register. - uint8_t source_dest = 0; + uint8_t destination_offset: 7; + /// Set if this program requires supervisor mode. + bool requires_supervisor: 1; + /// The source address register (for pre-decrement and post-increment actions). + uint8_t source: 3; + /// Destination address register. + uint8_t dest: 3; void set_source_address([[maybe_unused]] ProcessorStorage &storage, int index) { - source_dest = uint8_t((source_dest & 0x0f) | (index << 4)); + source = uint8_t(index); + assert(int(source) == index); } void set_destination_address([[maybe_unused]] ProcessorStorage &storage, int index) { - source_dest = uint8_t((source_dest & 0xf0) | index); + dest = uint8_t(index); + assert(int(dest) == index); } - void set_requires_supervisor(bool requires_supervisor) { - source_dest = (source_dest & 0x7f) | (requires_supervisor ? 0x80 : 0x00); + void set_requires_supervisor(bool req) { + requires_supervisor = req; + assert(requires_supervisor == req); } void set_source(ProcessorStorage &storage, RegisterPair32 *target) { From f20940a37b9c28c4eec69b21a2f6269f51512539 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Thu, 23 Dec 2021 16:32:21 -0500 Subject: [PATCH 3/4] Give `Program` full ownership of the sentinel value. In case I want to reduce the size of this field later. --- Processors/68000/Implementation/68000Implementation.hpp | 2 +- Processors/68000/Implementation/68000Storage.hpp | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/Processors/68000/Implementation/68000Implementation.hpp b/Processors/68000/Implementation/68000Implementation.hpp index 380e6a977..aee4c7de2 100644 --- a/Processors/68000/Implementation/68000Implementation.hpp +++ b/Processors/68000/Implementation/68000Implementation.hpp @@ -332,7 +332,7 @@ template void Proces // should_log = (fetched_pc >= 0x408D66) && (fetched_pc <= 0x408D84); #endif - if(instructions[decoded_instruction_.full].micro_operations != std::numeric_limits::max()) { + if(instructions[decoded_instruction_.full].micro_operations != Program::NoSuchProgram) { if((instructions[decoded_instruction_.full].requires_supervisor) && !is_supervisor_) { // A privilege violation has been detected. active_program_ = nullptr; diff --git a/Processors/68000/Implementation/68000Storage.hpp b/Processors/68000/Implementation/68000Storage.hpp index c87f40b55..6f5deeb50 100644 --- a/Processors/68000/Implementation/68000Storage.hpp +++ b/Processors/68000/Implementation/68000Storage.hpp @@ -368,9 +368,11 @@ class ProcessorStorage { source_offset = 0; } + static constexpr uint32_t NoSuchProgram = std::numeric_limits::max(); + /// The offset into the all_micro_ops_ at which micro-ops for this instruction begin, /// or std::numeric_limits::max() if this is an invalid Program. - uint32_t micro_operations = std::numeric_limits::max(); + uint32_t micro_operations = NoSuchProgram; /// The overarching operation applied by this program when the moment comes. Operation operation; /// The number of bytes after the beginning of an instance of ProcessorStorage that the RegisterPair32 containing From ee625cb8a8f34cfb03106361a76a61a2db97fe1d Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Sat, 25 Dec 2021 14:05:38 -0500 Subject: [PATCH 4/4] Minor style improvements; especially: don't assume value of NoBusProgram. --- Processors/68000/Implementation/68000Storage.hpp | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/Processors/68000/Implementation/68000Storage.hpp b/Processors/68000/Implementation/68000Storage.hpp index 6f5deeb50..c3634d17d 100644 --- a/Processors/68000/Implementation/68000Storage.hpp +++ b/Processors/68000/Implementation/68000Storage.hpp @@ -326,22 +326,24 @@ class ProcessorStorage { // steps detail appropriately. PrepareINTVector, }; - static constexpr int SourceMask = 1 << 7; + static_assert(uint8_t(Action::PrepareINTVector) < 32); // i.e. will fit into five bits. + + static constexpr int SourceMask = 1 << 5; static constexpr int DestinationMask = 1 << 6; - uint8_t action = uint8_t(Action::None); + uint8_t action = uint8_t(Action::None); // Requires 7 bits at present; sizeof(Action) + the two flags above. static constexpr uint16_t NoBusProgram = std::numeric_limits::max(); - uint16_t bus_program = NoBusProgram; + uint16_t bus_program = NoBusProgram; // Empirically requires 11 bits at present. - MicroOp() {} - MicroOp(uint8_t action) : action(action) {} + MicroOp(): action(uint8_t(Action::None)), bus_program(NoBusProgram) {} + MicroOp(uint8_t action) : action(action), bus_program(NoBusProgram) {} MicroOp(uint8_t action, uint16_t bus_program) : action(action), bus_program(bus_program) {} MicroOp(Action action) : MicroOp(uint8_t(action)) {} MicroOp(Action action, uint16_t bus_program) : MicroOp(uint8_t(action), bus_program) {} forceinline bool is_terminal() const { - return bus_program == std::numeric_limits::max(); + return bus_program == NoBusProgram; } };