From 607ddd2f78863a323c93b0ae5392a019964b4307 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Fri, 6 May 2022 09:45:06 -0400 Subject: [PATCH] Preserve MOVEM order in `Operation`. --- InstructionSets/M68k/Decoder.cpp | 26 ++-- InstructionSets/M68k/Decoder.hpp | 5 +- InstructionSets/M68k/Executor.hpp | 3 +- .../Implementation/ExecutorImplementation.hpp | 122 +++++++++--------- .../Implementation/PerformImplementation.hpp | 16 ++- InstructionSets/M68k/Instruction.hpp | 16 ++- .../68000ComparativeTests.mm | 2 +- .../Clock SignalTests/68000DecoderTests.mm | 7 +- 8 files changed, 104 insertions(+), 93 deletions(-) diff --git a/InstructionSets/M68k/Decoder.cpp b/InstructionSets/M68k/Decoder.cpp index e20f0577d..ce2359c19 100644 --- a/InstructionSets/M68k/Decoder.cpp +++ b/InstructionSets/M68k/Decoder.cpp @@ -76,8 +76,6 @@ constexpr Operation Predecoder::operation(OpT op) { } switch(op) { - case MOVEMtoRl: case MOVEMtoMl: return Operation::MOVEMl; - case MOVEMtoRw: case MOVEMtoMw: return Operation::MOVEMw; case MOVEPtoRl: case MOVEPtoMl: return Operation::MOVEPl; case MOVEPtoRw: case MOVEPtoMw: return Operation::MOVEPw; @@ -456,16 +454,16 @@ template uint32_t Predecoder::invalid_operands() { An >::value; - case MOVEMtoMw: case MOVEMtoMl: + case OpT(Operation::MOVEMtoMw): case OpT(Operation::MOVEMtoMl): return ~TwoOperandMask< Imm, Ind | PreDec | d16An | d8AnXn | XXXw | XXXl >::value; - case MOVEMtoRw: case MOVEMtoRl: + case OpT(Operation::MOVEMtoRw): case OpT(Operation::MOVEMtoRl): return ~TwoOperandMask< - Ind | PostInc | d16An | d8AnXn | XXXw | XXXl | d16PC | d8PCXn, - Imm + Imm, + Ind | PostInc | d16An | d8AnXn | XXXw | XXXl | d16PC | d8PCXn >::value; case MOVEPtoRl: case MOVEPtoRw: @@ -796,16 +794,12 @@ template Preinstruction Predecoder::decode(ui // b0–b2 and b3–b5: effective address. // [already decoded: b10: direction] // - case MOVEMtoMl: case MOVEMtoMw: + case OpT(Operation::MOVEMtoMl): case OpT(Operation::MOVEMtoMw): + case OpT(Operation::MOVEMtoRl): case OpT(Operation::MOVEMtoRw): return validated( AddressingMode::ImmediateData, 0, combined_mode(ea_mode, ea_register), ea_register); - case MOVEMtoRl: case MOVEMtoRw: - return validated( - combined_mode(ea_mode, ea_register), ea_register, - AddressingMode::ImmediateData, 0); - // // MARK: TRAP, BCCb, BSRb // @@ -1087,10 +1081,10 @@ Preinstruction Predecoder::decode4(uint16_t instruction) { case 0x840: Decode(Op::PEA); // 4-128 (p232) - case 0x880: Decode(MOVEMtoMw); - case 0x8c0: Decode(MOVEMtoMl); - case 0xc80: Decode(MOVEMtoRw); - case 0xcc0: Decode(MOVEMtoRl); + case 0x880: Decode(Op::MOVEMtoMw); + case 0x8c0: Decode(Op::MOVEMtoMl); + case 0xc80: Decode(Op::MOVEMtoRw); + case 0xcc0: Decode(Op::MOVEMtoRl); // 4-192 (p296) case 0xa00: Decode(Op::TSTb); diff --git a/InstructionSets/M68k/Decoder.hpp b/InstructionSets/M68k/Decoder.hpp index 59bae5cce..30052304a 100644 --- a/InstructionSets/M68k/Decoder.hpp +++ b/InstructionSets/M68k/Decoder.hpp @@ -66,10 +66,7 @@ template class Predecoder { // time that's knowable from the Operation alone, hence the rather awkward // extension of @c Operation. enum ExtendedOperation: OpT { - MOVEMtoRl = uint8_t(Operation::Max) + 1, MOVEMtoRw, - MOVEMtoMl, MOVEMtoMw, - - MOVEPtoRl, MOVEPtoRw, + MOVEPtoRl = uint8_t(Operation::Max) + 1, MOVEPtoRw, MOVEPtoMl, MOVEPtoMw, MOVEQ, diff --git a/InstructionSets/M68k/Executor.hpp b/InstructionSets/M68k/Executor.hpp index 9563c8efc..84a06667c 100644 --- a/InstructionSets/M68k/Executor.hpp +++ b/InstructionSets/M68k/Executor.hpp @@ -51,7 +51,8 @@ template class Executor { void link(uint32_t &address, uint32_t offset); void unlink(uint32_t &address); template void movep(Preinstruction instruction, uint32_t source, uint32_t dest); - template void movem(Preinstruction instruction, uint32_t source, uint32_t dest); + template void movem_toM(Preinstruction instruction, uint32_t source, uint32_t dest); + template void movem_toR(Preinstruction instruction, uint32_t source, uint32_t dest); // TODO: ownership of this shouldn't be here. struct Registers { diff --git a/InstructionSets/M68k/Implementation/ExecutorImplementation.hpp b/InstructionSets/M68k/Implementation/ExecutorImplementation.hpp index 11c7f28bc..1d1d88ab0 100644 --- a/InstructionSets/M68k/Implementation/ExecutorImplementation.hpp +++ b/InstructionSets/M68k/Implementation/ExecutorImplementation.hpp @@ -439,79 +439,81 @@ void Executor::movep(Preinstruction instruction, uint32_t sou template template -void Executor::movem(Preinstruction instruction, uint32_t source, uint32_t dest) { - if(instruction.mode<0>() == AddressingMode::ImmediateData) { - // Move registers to memory. This is the only permitted use of the predecrement mode, - // which reverses output order. - if(instruction.mode<1>() == AddressingMode::AddressRegisterIndirectWithPredecrement) { - // The structure of the code in the mainline part of the executor is such - // that the address register will already have been predecremented before - // reaching here, and it'll have been by two bytes per the operand size - // rather than according to the instruction size. That's not wanted, so undo it. - // - // TODO: with the caveat that the 68020+ have different behaviour: - // - // "For the MC68020, MC68030, MC68040, and CPU32, if the addressing register is also - // moved to memory, the value written is the initial register value decremented by the - // size of the operation. The MC68000 and MC68010 write the initial register value - // (not decremented)." - registers_[8 + instruction.reg<1>()].l += 2; +void Executor::movem_toM(Preinstruction instruction, uint32_t source, uint32_t dest) { + // Move registers to memory. This is the only permitted use of the predecrement mode, + // which reverses output order. - uint32_t reg = registers_[8 + instruction.reg<1>()].l; - int index = 15; + if(instruction.mode<1>() == AddressingMode::AddressRegisterIndirectWithPredecrement) { + // The structure of the code in the mainline part of the executor is such + // that the address register will already have been predecremented before + // reaching here, and it'll have been by two bytes per the operand size + // rather than according to the instruction size. That's not wanted, so undo it. + // + // TODO: with the caveat that the 68020+ have different behaviour: + // + // "For the MC68020, MC68030, MC68040, and CPU32, if the addressing register is also + // moved to memory, the value written is the initial register value decremented by the + // size of the operation. The MC68000 and MC68010 write the initial register value + // (not decremented)." + registers_[8 + instruction.reg<1>()].l += 2; - while(source) { - if(source & 1) { - reg -= sizeof(IntT); - bus_handler_.template write(reg, IntT(registers_[index].l)); - } - --index; - source >>= 1; - } + uint32_t reg = registers_[8 + instruction.reg<1>()].l; + int index = 15; - registers_[8 + instruction.reg<1>()].l = reg; - return; - } - - int index = 0; while(source) { if(source & 1) { - bus_handler_.template write(dest, IntT(registers_[index].l)); - dest += sizeof(IntT); + reg -= sizeof(IntT); + bus_handler_.template write(reg, IntT(registers_[index].l)); } - ++index; + --index; source >>= 1; } - } else { - // Move memory to registers. - // - // Another 68000 convention has been broken here; the instruction form is: - // MOVEM , # - // ... but the instruction is encoded as [MOVEM] [#] [ea]. - // - // TODO: solve this. - int index = 0; - while(dest) { - if(dest & 1) { - if constexpr (sizeof(IntT) == 2) { - registers_[index].l = int16_t(bus_handler_.template read(source)); - } else { - registers_[index].l = bus_handler_.template read(source); - } - source += sizeof(IntT); + registers_[8 + instruction.reg<1>()].l = reg; + return; + } + + int index = 0; + while(source) { + if(source & 1) { + bus_handler_.template write(dest, IntT(registers_[index].l)); + dest += sizeof(IntT); + } + ++index; + source >>= 1; + } +} + +template +template +void Executor::movem_toR(Preinstruction instruction, uint32_t source, uint32_t dest) { + // Move memory to registers. + // + // A 68000 convention has been broken here; the instruction form is: + // MOVEM , # + // ... but the instruction is encoded as [MOVEM] [#] [ea]. + // + // This project's decoder decodes as #, . + int index = 0; + while(source) { + if(source & 1) { + if constexpr (sizeof(IntT) == 2) { + registers_[index].l = int16_t(bus_handler_.template read(dest)); + } else { + registers_[index].l = bus_handler_.template read(dest); } - ++index; - dest >>= 1; + dest += sizeof(IntT); } + ++index; + source >>= 1; + } - if(instruction.mode<0>() == AddressingMode::AddressRegisterIndirectWithPostincrement) { - // If the effective address is specified by the postincrement mode ... - // [i]f the addressing register is also loaded from memory, the memory value is - // ignored and the register is written with the postincremented effective address. + if(instruction.mode<1>() == AddressingMode::AddressRegisterIndirectWithPostincrement) { + // "If the effective address is specified by the postincrement mode ... + // [i]f the addressing register is also loaded from memory, the memory value is + // ignored and the register is written with the postincremented effective address." - registers_[8 + instruction.reg<0>()].l = source; - } + registers_[8 + instruction.reg<1>()].l = source; } } diff --git a/InstructionSets/M68k/Implementation/PerformImplementation.hpp b/InstructionSets/M68k/Implementation/PerformImplementation.hpp index 65a57f446..2ace91b17 100644 --- a/InstructionSets/M68k/Implementation/PerformImplementation.hpp +++ b/InstructionSets/M68k/Implementation/PerformImplementation.hpp @@ -1184,12 +1184,20 @@ template < flow_controller.template movep(instruction, src.l, dest.l); break; - case Operation::MOVEMl: - flow_controller.template movem(instruction, src.l, dest.l); + case Operation::MOVEMtoRl: + flow_controller.template movem_toR(instruction, src.l, dest.l); break; - case Operation::MOVEMw: - flow_controller.template movem(instruction, src.l, dest.l); + case Operation::MOVEMtoMl: + flow_controller.template movem_toM(instruction, src.l, dest.l); + break; + + case Operation::MOVEMtoRw: + flow_controller.template movem_toR(instruction, src.l, dest.l); + break; + + case Operation::MOVEMtoMw: + flow_controller.template movem_toM(instruction, src.l, dest.l); break; /* diff --git a/InstructionSets/M68k/Instruction.hpp b/InstructionSets/M68k/Instruction.hpp index d55e2348d..40a72272d 100644 --- a/InstructionSets/M68k/Instruction.hpp +++ b/InstructionSets/M68k/Instruction.hpp @@ -72,7 +72,9 @@ enum class Operation: uint8_t { ROXLb, ROXLw, ROXLl, ROXLm, ROXRb, ROXRw, ROXRl, ROXRm, - MOVEMl, MOVEMw, + MOVEMtoRl, MOVEMtoRw, + MOVEMtoMl, MOVEMtoMw, + MOVEPl, MOVEPw, ANDb, ANDw, ANDl, @@ -192,7 +194,10 @@ constexpr DataSize operand_size(Operation operation) { case Operation::RORw: case Operation::RORm: case Operation::ROXLw: case Operation::ROXLm: case Operation::ROXRw: case Operation::ROXRm: - case Operation::MOVEMw: case Operation::MOVEMl: + case Operation::MOVEMtoRw: + case Operation::MOVEMtoRl: + case Operation::MOVEMtoMw: + case Operation::MOVEMtoMl: case Operation::MOVEPw: case Operation::ANDw: case Operation::EORw: case Operation::NOTw: case Operation::ORw: @@ -266,9 +271,10 @@ template uint8_t ope // (which means that source and destination will appear as their effective addresses) // case Operation::PEA: - case Operation::JMP: case Operation::JSR: - case Operation::MOVEPw: case Operation::MOVEPl: - case Operation::MOVEMw: case Operation::MOVEMl: + case Operation::JMP: case Operation::JSR: + case Operation::MOVEPw: case Operation::MOVEPl: + case Operation::MOVEMtoMw: case Operation::MOVEMtoMl: + case Operation::MOVEMtoRw: case Operation::MOVEMtoRl: return 0; // diff --git a/OSBindings/Mac/Clock SignalTests/68000ComparativeTests.mm b/OSBindings/Mac/Clock SignalTests/68000ComparativeTests.mm index 775b17658..e056ade84 100644 --- a/OSBindings/Mac/Clock SignalTests/68000ComparativeTests.mm +++ b/OSBindings/Mac/Clock SignalTests/68000ComparativeTests.mm @@ -73,7 +73,7 @@ - (void)setUp { // To limit tests run to a subset of files and/or of tests, uncomment and fill in below. _fileSet = [NSSet setWithArray:@[@"movem.json"]]; - _testSet = [NSSet setWithArray:@[@"MOVEM 00a8 (0)"]]; +// _testSet = [NSSet setWithArray:@[@"MOVEM 00a8 (0)"]]; // _fileSet = [NSSet setWithArray:@[@"jmp_jsr.json"]]; // _testSet = [NSSet setWithArray:@[@"CHK 41a8"]]; } diff --git a/OSBindings/Mac/Clock SignalTests/68000DecoderTests.mm b/OSBindings/Mac/Clock SignalTests/68000DecoderTests.mm index 123fbdaa2..0ba3faf16 100644 --- a/OSBindings/Mac/Clock SignalTests/68000DecoderTests.mm +++ b/OSBindings/Mac/Clock SignalTests/68000DecoderTests.mm @@ -246,8 +246,11 @@ template NSString *operand(Preinstruction instruction, uint16_t opco case Operation::ROXRl: instruction = @"ROXR.l"; break; case Operation::ROXRm: instruction = @"ROXR.w"; break; - case Operation::MOVEMl: instruction = @"MOVEM.l"; break; - case Operation::MOVEMw: instruction = @"MOVEM.w"; break; + // TODO: switch operand order for toR. + case Operation::MOVEMtoMl: instruction = @"MOVEM.l"; break; + case Operation::MOVEMtoMw: instruction = @"MOVEM.w"; break; + case Operation::MOVEMtoRl: instruction = @"MOVEM.l"; break; + case Operation::MOVEMtoRw: instruction = @"MOVEM.w"; break; case Operation::MOVEPl: instruction = @"MOVEP.l"; break; case Operation::MOVEPw: instruction = @"MOVEP.w"; break;