7
\$\begingroup\$

I want to make sure I won't have to go back and redo large chunks of code. I have each opcode as a value in an enum that implements Runnable. Is there a more efficient way that I should do this or am I on the right track to getting something that runs a testsuite ROM accurately?

package com.codeblox.nes.cpu;

public class CPU {

    private byte x, y, ac, pcl, pch;
    private short pc;
    private boolean debugEnabled = false, isGood = true;
    private static byte [] mainMem = new byte [0x10000];
    public Opcode opcode;

    CPU(boolean debugEnabled){
        opcode =Opcode.nop;
        pc = 0;
        this.debugEnabled = debugEnabled;

    }

    public enum Opcode implements Runnable{


        adc(){public void run(){System.out.println("adc");}},
        and(){public void run(){System.out.println("and");}},
        asl(){public void run(){System.out.println("asl");}},
        bcc(){public void run(){System.out.println("bcc");}},
        bcs(){public void run(){System.out.println("bcs");}},
        beq(){public void run(){System.out.println("beq");}},
        bit(){public void run(){System.out.println("bit");}},
        bmi(){public void run(){System.out.println("bmi");}},
        bne(){public void run(){System.out.println("bne");}},
        bpl(){public void run(){System.out.println("bpl");}},
        brk(){public void run(){System.out.println("brk");}},
        bvc(){public void run(){System.out.println("bvc");}},
        bvs(){public void run(){System.out.println("bvs");}},
        clc(){public void run(){System.out.println("clc");}},
        cld(){public void run(){System.out.println("cld");}},
        cli(){public void run(){System.out.println("cli");}},
        clv(){public void run(){System.out.println("clv");}},
        cmp(){public void run(){System.out.println("cmp");}},
        cpx(){public void run(){System.out.println("cpx");}},
        cpy(){public void run(){System.out.println("cpy");}},
        dec(){public void run(){System.out.println("dec");}},
        dex(){public void run(){System.out.println("dex");}},
        dey(){public void run(){System.out.println("dey");}},
        eor(){public void run(){System.out.println("eor");}},
        inc(){public void run(){System.out.println("inc");}},
        inx(){public void run(){System.out.println("inx");}},
        iny(){public void run(){System.out.println("iny");}},
        jmp(){public void run(){System.out.println("jmp");}},
        jsr(){public void run(){System.out.println("jsr");}},
        lda(){public void run(){System.out.println("lda");}},
        ldx(){public void run(){System.out.println("ldx");}},
        ldy(){public void run(){System.out.println("ldy");}},
        lsr(){public void run(){System.out.println("lsr");}},
        nop(){public void run(){System.out.println("nop");}},
        ora(){public void run(){System.out.println("ora");}},
        pha(){public void run(){System.out.println("pha");}},
        php(){public void run(){System.out.println("php");}},
        pla(){public void run(){System.out.println("pla");}},
        plp(){public void run(){System.out.println("plp");}},
        rol(){public void run(){System.out.println("rol");}},
        ror(){public void run(){System.out.println("ror");}},
        rti(){public void run(){System.out.println("rti");}},
        rts(){public void run(){System.out.println("rts");}},
        sbc(){public void run(){System.out.println("sbc");}},
        sec(){public void run(){System.out.println("sec");}},
        sed(){public void run(){System.out.println("sed");}},
        sei(){public void run(){System.out.println("sei");}},
        sta(){public void run(){System.out.println("sta");}},
        stx(){public void run(){System.out.println("stx");}},
        sty(){public void run(){System.out.println("sty");}},
        tax(){public void run(){System.out.println("tax");}},
        tay(){public void run(){System.out.println("tay");}},
        tsx(){public void run(){System.out.println("tsx");}},
        txa(){public void run(){System.out.println("txa");}},
        txs(){public void run(){System.out.println("txs");}},
        tya(){public void run(){System.out.println("tya");}},
        ;

        public String mnemonic = "";
        public String addressMode;
        public byte code;
        public byte data;

        Opcode(){

            this.mnemonic = new String();

        }

        public void print(){

            System.out.printf("Opcode: %02X %s %s\n", 
                              this.code, 
                              this.mnemonic.toUpperCase(),
                              this.addressMode);

        }

        public String getMode00(byte opcode){

            switch(opcode){

                case 0x00: return "Immediate";
                case 0x04: return "ZeroPaged";
                case 0x0C: return "Absolute";
                case 0x14: return "IndexedZeroPagedX";
                case 0x1C: return "IndexedAbsoluteX";
                default: return "Type 0 undefined";

            }

        }

        public String getMode01(byte opcode){

            switch(opcode){

                case 0x00: return "InirectIndexedZeroPagedX";
                case 0x04: return "ZeroPaged";
                case 0x08: return "Immediate";
                case 0x0C: return "Absolute";
                case 0x10: return "IndrectedZeroPagedY";
                case 0x14: return "IndexedZeroPagedX";
                case 0x18: return "IndexedAbsoluteY";
                case 0x1C: return "IndexedAbsoluteX";
                default: return "Type 1 Undefined";         

            }

        }

        public String getMode02(byte opcode){ 

            switch(opcode){

                case 0x00: return "Immediate";
                case 0x04: return "ZeroPaged";
                case 0x08: return "Accumulator";
                case 0x0C: return "Absolute";
                case 0x14: return "IndexedZeroPagedX";
                case 0x1C: return "IndexedAbsoluteX";
                default: return "Type 2 Undefined";

            }

        }

        public String getMode03(byte opcode){ return "";}

        public void decode(){

            switch(this.code & 0x03){

                case 0x00: this.addressMode = getMode00((byte)(this.code & 0x1C)); break;
                case 0x01: this.addressMode = getMode01((byte)(this.code & 0x1C)); break;
                case 0x02: this.addressMode = getMode02((byte)(this.code & 0x1C)); break;
                case 0x03: this.addressMode = getMode03((byte)(this.code & 0x1C)); break;
                default: break;


            }


        }

    }


    public void init(){

        pc = 0;

    }

    public void start(){

        while(isGood){


            opcode.code = readMem(pc++);
            CPU.Opcode.valueOf(opcode.mnemonic).run();

        }

        if(!isGood){

            System.err.println("isGood == false");

        }

    }

    public byte readMem(short ptr){

        return mainMem[ptr];

    }

    public byte readMem(short ptr, byte addressMode){

        return mainMem[ptr];

    }

    public void exec(){

        opcode.decode();

        switch(opcode.code & 0xFF){

            case 0x69: case 0x65: case 0x75: 
            case 0x6D: case 0x7D: case 0x79: 
            case 0x61: case 0x71: opcode.mnemonic = "adc"; break;

            case 0x29: case 0x25: case 0x35: 
            case 0x2D: case 0x3D: case 0x39:
            case 0x21: case 0x31: opcode.mnemonic = "and"; break;

            case 0x0A: case 0x06: case 0x16: 
            case 0x0E: case 0x1E: opcode.mnemonic = "asl"; break;



            default: opcode.mnemonic = null;

        }

        //Opcodes.valueOf(this.mnemonic).run();

    }

    public void testOpcodes(){

        opcode.code = 0;

        while((opcode.code & 0xFF) < 0xFF){

            //System.out.printf("PC = 0x%04X \n", PC);
            exec();
            if(opcode.mnemonic != null)
                opcode.print();
                //Opcode.valueOf(opcode.mnemonic).run();

            opcode.code++;

        }


    }
    public static void main(String[] args) {
        // TODO Auto-generated method stub

        CPU cpu = new CPU(true);
        cpu.init();
        cpu.testOpcodes();

    }

}
\$\endgroup\$
1
\$\begingroup\$

I would change to this:

public class CPU {

   public interface OpCode {
       OpCode getFor(int opCodeByte);
       void execute(Cpu cpu, int[] memory);
   }

    // byte is signed in Java which is problematic....
    private int x, y, ac, pcl, pch;
    private short pc;
    private boolean debugEnabled = false, isGood = true;
    private static int [] mainMem = new int [0x10000]; 
    public Opcode opcode;

    CPU(boolean debugEnabled){
        opcode =Opcode.nop;
        pc = 0;
        this.debugEnabled = debugEnabled;

    }

    /**
      * Opcodes in order of their byte value
      */
    public enum Opcode implements OpCode {
      OC_00_NOOP{
       @Override
       void execute(Cpu cpu, Memory memory){
          cpu.pc= ++cpu.pc%memory.length; // avoid IOOBE
          cpu.opcode= getFor(memory[cpu.pc]);
       }
      },
      OC_01_WHAT_EVER{ /*...*/ },
      // ...
      OC_FF_WHAT_EVER{ /*...*/ };

      OpCode getFor(int opCodeByte){
        if(opCodeByte<values().length)
            return values()[opCodeByte];
        throw IllegalArgumentException(opCodeByte+ " is not a valid op code");
      }
    }
\$\endgroup\$
  • \$\begingroup\$ I wonder about the wisdom of parameter lists that say int byte since both are type names. It might compile, but is it a good idea? \$\endgroup\$ – Edward Nov 21 '16 at 14:51
  • \$\begingroup\$ @Edward "since both are type names" sorry, I'm just a human and make mistakes... ;o) Thanks for pointing at it. \$\endgroup\$ – Timothy Truckle Nov 21 '16 at 14:57
1
\$\begingroup\$

I have some ideas that may help you improve your code.

Consider a different organization

I would not likely do things this way. While what you have is probably going to work, it is not organized very well in that each mnemonic has bits and pieces of information spread out over many different places. Instead, I'd be inclined to create an Operation class rather than just an Opcode enum. The reason for this is that each opcode has a number of things that are very similar. Each has both a mnemonic and one or more corresponding hex representations. Each one also has attributes such as the length of the instruction and the number of CPU cycles it takes. I'd suggest instead that a more data-oriented approach could be used.

Reconsider public functions

The current code includes a number of functions such as getMode00 and getMode01 which take an opcode as an argument and return a string. I would suggest that these functions are not very useful and should be refactored. First, there's not indication to the user about which getModexx function would be appropriate to call (they're really intended to be private and only used within decode() if I'm not mistaken). Second, having a string representing the addressing mode is of secondary value behind actually identifying the mode. For this case, I'd suggest that each opcode could have its own mode function (naturally, they could share them as appropriate) and that the return should be something that's actually useful for decoding the operation rather than just a string.

Consider the other effects of executing opcodes

Changing register contents (which are currently represented) are only one of the effect of executing an opcode. Other effects are that the CPU's flags are possibly changed and that time has passed. You may or may not be interested in modeling the cycle-time, but you probably are interested in modeling the flags if the code is to be at all useful. It's not to soon to consider how those things should be represented.

Think of the user

Printing an error message that reads "isGood == false" is unlikely to be of much use to an actual user of the code. Consider spelling out something more descriptive, and ideally, something that suggests an action the user might take to correct the problem.

\$\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.