2
\$\begingroup\$

I've written the larger part of an emulator for a 6502 CPU but my method of op-code decoding is giving me concern, mainly because of how long the method is getting but also because I need a variable for each code, I need to declare them three times and I've stuck them all in a very non-OO class. I wouldn't mind some scathing critique.

I've written it in a TDD fashion (in an effort to understand pure TDD) which has led to the simplest implementation with refactors only when necessary. Even though I think this is probably the most efficient solution (making full use of the benefits of a switch statement), I'm thinking there's maybe a better way.

Extract:-

public void step() {
    System.out.println("\n*** STEP >>>");

    int accumulatorBeforeOperation = registers.getRegister(Registers.REG_ACCUMULATOR);
    int opCode = nextProgramByte();

    //Execute the opcode
    System.out.println("Instruction: " + InstructionSet.getOpCodeName(opCode) + "...");
    switch (opCode){
        case InstructionSet.OP_ASL_A: {
            int newFakeByte = registers.getRegister(Registers.REG_ACCUMULATOR) << 1;
            setCarryFlagBasedOn(newFakeByte);
            registers.setRegisterAndFlags(Registers.REG_ACCUMULATOR, newFakeByte);
        }
        break;

        case InstructionSet.OP_ASL_Z: {
            int location = nextProgramByte();
            int newFakeByte = memory.getByte(location) << 1;
            setCarryFlagBasedOn(newFakeByte);
            registers.setFlagsBasedOn(newFakeByte);
            memory.setByteAt(location, newFakeByte);
        }
        break;
        ...
        case InstructionSet.OP_DEY:
            registers.decrementRegisterWithFlags(Registers.REG_Y_INDEX);
            break;

        case InstructionSet.OP_LDX_I:
            registers.setRegisterAndFlags(Registers.REG_X_INDEX, nextProgramByte());
            break;

        case InstructionSet.OP_LDX_Z:
            registers.setRegisterAndFlags(Registers.REG_X_INDEX, getByteOfMemoryAt(nextProgramByte()));
            break;
        ...
        default:
            throw new UnknownOpCodeException("Unknown 6502 OpCode:" + opCode + " encountered.", opCode);
    }

Thoughts

I've looked at storing op-codes as Enums but it means reading bytes (in Javas case this is int represented bytes) and writing conversion code from raw byte to Enum but that seems unnecessary for the small maintainability improvement.

It seems excessive and not very OO to do it with a class per op-code.

\$\endgroup\$
  • \$\begingroup\$ Have you considered implementing this more like the instruction decoder works in the actual hardware? That usually means looking at a few bits at a time to match against entire families of instructions - in fact that's the whole reason you see repeating patterns in the instruction set. I've only used Z80 and 6800, but I wouldn't expect 6502 to be any different in that respect. \$\endgroup\$ – Toby Speight Nov 5 '18 at 10:37
  • \$\begingroup\$ It's not different at all @TobySpeight. I did consider it but it means tearing each byte apart into sections then writing lookup tables. I'd be sacrificing flexibility for authenticity. I want it to function like a 6502 but be good Java design. It's a tightrope :) \$\endgroup\$ – Ross Drew Nov 5 '18 at 11:25
2
\$\begingroup\$

I figured a much nicer way to deal with this. Break it down into the repeatable elements, i.e. Addressing Mode and Operation then code them all separately and immutably, combining them in the right way to get all OpCodes. Effectively doing functional programming in Java.

First: If you represent an operation, say ASL(arithmatic shift left) as an Enum (Operation), which has a parameter that is a lamba of a @FunctionalInterface (in this case called AddressedValueInstruction) and accepts two bytes and returns a result. (Note: RoxByte is just my custom implementation of a byte)

public enum Operation implements AddressedValueInstruction {
/** Shift all bits in byte left by one place, setting flags based on the result */
    ASL((a,r,m,v) -> {
        final RoxByte newValue = a.asl(v);
        r.setFlagsBasedOn(newValue);
        return newValue;
    }),

    ...

    @Override
    public RoxByte perform(Mos6502Alu alu, Registers registers, Memory memory, RoxByte value) {
        return instruction.perform(alu, registers, memory, value);
    }

Second: If you represent the Addressing Mode as an Enum then -similar to Operation- have one of the Enum parameters as a @FunctionalInterface (called Addressable) lambda you can say MyEnum.address(AddressedValueInstruction) which will address a value, then sent it to the given operation then address the result.

public enum AddressingMode implements Addressable {
    /** Expects no argument, operation will be performed using the Accumulator Register*/
    ACCUMULATOR("Accumulator", 1, (r, m, a, i) -> {
        final RoxByte value = r.getRegister(Registers.Register.ACCUMULATOR);
        r.setRegister(Registers.Register.ACCUMULATOR, i.perform(a, r, m, value));
    }),

    ...

    @Override
    public void address(Registers r, Memory m, Mos6502Alu alu, AddressedValueInstruction instruction) {
        address.address(r, m, alu, instruction);
    }

Third: This means you can create opcodes in the same way by creating combinations of addressing modes and operations with no specific code required, i.e. ASL_ACCUMULATOR(ACCUMULATOR, ASL) and it can be run with ASL_ACCUMULATOR.perform()

public enum OpCode implements Mos6502Instruction {
    ASL_A(0x0A, ACCUMULATOR, ASL),
    ...

    @Override
    public void perform(Mos6502Alu alu, Registers registers, Memory memory) {
        addressingMode.address(registers, memory, alu, operation::perform);
    }

This approach has the benefits of a class per opcode approach, without all the boiler plate.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.