From 52f355db24206e998d2a9c683d0991c23c159c09 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Sat, 30 Apr 2022 14:08:51 -0400 Subject: [PATCH] Decision: operation is not a template parameter. Hence can use condition as fully typed. --- .../Implementation/PerformImplementation.hpp | 31 ++++---------- InstructionSets/M68k/Perform.hpp | 3 +- InstructionSets/M68k/Status.hpp | 40 ++++++++++--------- 3 files changed, 29 insertions(+), 45 deletions(-) diff --git a/InstructionSets/M68k/Implementation/PerformImplementation.hpp b/InstructionSets/M68k/Implementation/PerformImplementation.hpp index c4f450c85..ea1ec6718 100644 --- a/InstructionSets/M68k/Implementation/PerformImplementation.hpp +++ b/InstructionSets/M68k/Implementation/PerformImplementation.hpp @@ -28,41 +28,24 @@ namespace M68k { // TODO: decisions outstanding: // -// (1) for DBcc, etc, should this receive the opcode in order to decode the conditional, -// or is it a design error to do any _decoding_ here rather than in the decoder? If -// the latter, should the internal cc operations all treat conditional as another -// Q-style operand? -// -// (2) should I reintroduce the BTSTl/BTSTw-type distinctions, given that the only way to +// (1) should I reintroduce the BTSTl/BTSTw-type distinctions, given that the only way to // determine them otherwise is by operand types and I'm hoping to treat data into // here as a black box? // -// (3) to what extent, if any, should this function have responsibility for a MOVEM, MOVEP, +// (2) to what extent, if any, should this function have responsibility for a MOVEM, MOVEP, // etc? This factoring is inteded to separate the bus interface from internal logic so // is there much to do here in any case? As currently drafted, something else will // already have had to check the operation and cue up data. // -// (4) related to that, should the flow controller actually offer effective address calculation -// and load/stores, along with a flag indicating whether to stop after loads? By the -// magic of templates that'd avoid having a correlated sequencer — for the non-bus-accurate -// 68ks the loads and stores could be performed immediately, for the accurate they could -// be enqueued, then performed, then a second call to perform that now has the data loaded -// could be performed. -// -// (5) is it really helpful for operation to be a template parameter? I'm trying to avoid forcing -// an additional `switch` if it's likely that the caller has already applied one, but does -// that objective justify the syntax overhead for callers that don't inherently have their -// own `switch`? Do the first sort of callers really exist? template < - Operation operation, Model model, typename FlowController -> void perform(CPU::SlicedInt32 &src, CPU::SlicedInt32 &dest, Status &status, FlowController &flow_controller) { +> void perform(Preinstruction instruction, CPU::SlicedInt32 &src, CPU::SlicedInt32 &dest, Status &status, FlowController &flow_controller) { #define sub_overflow() ((result ^ destination) & (destination ^ source)) #define add_overflow() ((result ^ destination) & ~(destination ^ source)) - switch(operation) { + switch(instruction.operation) { /* ABCD adds the lowest bytes from the source and destination using BCD arithmetic, obeying the extend flag. @@ -298,7 +281,7 @@ template < case Operation::Bccw: case Operation::Bccl: { // Test the conditional, treating 'false' as true. - const bool should_branch = status.evaluate_condition(flow_controller.decode_conditional()); + const bool should_branch = status.evaluate_condition(instruction.condition()); // Schedule something appropriate, by rewriting the program for this instruction temporarily. if(should_branch) { @@ -310,7 +293,7 @@ template < case Operation::DBcc: // Decide what sort of DBcc this is. - if(!status.evaluate_condition(flow_controller.decode_conditional())) { + if(!status.evaluate_condition(instruction.condition())) { -- src.w; if(src.w == 0xffff) { @@ -327,7 +310,7 @@ template < break; case Operation::Scc: - dest.b = status.evaluate_condition(src.l) ? 0xff : 0x00; + dest.b = status.evaluate_condition(instruction.condition()) ? 0xff : 0x00; break; /* diff --git a/InstructionSets/M68k/Perform.hpp b/InstructionSets/M68k/Perform.hpp index d6038bb4a..404ec5833 100644 --- a/InstructionSets/M68k/Perform.hpp +++ b/InstructionSets/M68k/Perform.hpp @@ -27,10 +27,9 @@ struct NullFlowController { /// branch, or consumes additional cycles due to the particular value of the operands (on the 68000, think DIV or MUL), /// that'll be notified to the @c flow_controller. template < - Operation op, Model model, typename FlowController -> void perform(CPU::RegisterPair32 &source, CPU::RegisterPair32 &dest, Status &status, FlowController &flow_controller); +> void perform(Preinstruction instruction, CPU::RegisterPair32 &source, CPU::RegisterPair32 &dest, Status &status, FlowController &flow_controller); } } diff --git a/InstructionSets/M68k/Status.hpp b/InstructionSets/M68k/Status.hpp index eb6ccecd3..8e7652762 100644 --- a/InstructionSets/M68k/Status.hpp +++ b/InstructionSets/M68k/Status.hpp @@ -9,6 +9,8 @@ #ifndef InstructionSets_M68k_Status_h #define InstructionSets_M68k_Status_h +#include "Instruction.hpp" + namespace InstructionSet { namespace M68k { @@ -70,29 +72,29 @@ struct Status { return is_supervisor_; } - /// Evaluates the condition described in the low four bits of @c code - template bool evaluate_condition(IntT code) { - switch(code & 0xf) { + /// Evaluates @c condition. + bool evaluate_condition(Condition condition) { + switch(condition) { default: - case 0x00: return true; // true - case 0x01: return false; // false - case 0x02: return zero_result_ && !carry_flag_; // high - case 0x03: return !zero_result_ || carry_flag_; // low or same - case 0x04: return !carry_flag_; // carry clear - case 0x05: return carry_flag_; // carry set - case 0x06: return zero_result_; // not equal - case 0x07: return !zero_result_; // equal - case 0x08: return !overflow_flag_; // overflow clear - case 0x09: return overflow_flag_; // overflow set - case 0x0a: return !negative_flag_; // positive - case 0x0b: return negative_flag_; // negative - case 0x0c: // greater than or equal + case Condition::True: return true; + case Condition::False: return false; + case Condition::High: return zero_result_ && !carry_flag_; + case Condition::LowOrSame: return !zero_result_ || carry_flag_; + case Condition::CarryClear: return !carry_flag_; + case Condition::CarrySet: return carry_flag_; + case Condition::NotEqual: return zero_result_; + case Condition::Equal: return !zero_result_; + case Condition::OverflowClear: return !overflow_flag_; + case Condition::OverflowSet: return overflow_flag_; + case Condition::Positive: return !negative_flag_; + case Condition::Negative: return negative_flag_; + case Condition::GreaterThanOrEqual: return (negative_flag_ && overflow_flag_) || (!negative_flag_ && !overflow_flag_); - case 0x0d: // less than + case Condition::LessThan: return (negative_flag_ && !overflow_flag_) || (!negative_flag_ && overflow_flag_); - case 0x0e: // greater than + case Condition::GreaterThan: return zero_result_ && ((negative_flag_ && overflow_flag_) || (!negative_flag_ && !overflow_flag_)); - case 0x0f: // less than or equal + case Condition::LessThanOrEqual: return !zero_result_ || (negative_flag_ && !overflow_flag_) || (!negative_flag_ && overflow_flag_); } }