10
\$\begingroup\$

Followed-up here.

I've written a partial z80 emulator in C++. It has a decent part of the instructions that can be reused implemented but I've had some issues with duplicated code.

I'd also like to note that this is my first C++ project; before that I've primarily dealt with C and Assembly.

emulate.cpp:

#include <stdexcept>
#include "z80emu.hpp"
#include "opcodes.h"

#ifndef NDEBUG
# include <iostream>
using std::cout;
using std::endl;
#endif

namespace z80emu {
    // return value: number of instructions executed
    uint16_t z80::emulate()
    {
        reg *rp[] =
        {
            &regs.bc,
            &regs.de,
            &regs.hl,
            &regs.sp
        };
        reg *rp2[] =
        {
            &regs.bc,
            &regs.de,
            &regs.hl,
            &regs.af
        };
        uint16_t inst = 0;

        for(;;)
        {
            switch(mem[regs.pc])
            {
                case NOP:
                    break;
                case LD_BC_IMM:
                case LD_DE_IMM:
                case LD_HL_IMM:
                case LD_SP_IMM:
                    ld16imm(mem[regs.pc] >> 4, rp);
                    break;
                case LD_DBC_A:
                case LD_DDE_A:
                    deref16_u8(mem[regs.pc] >> 4, rp) = regs.af.high;
                    break;
                case INC_BC:
                case INC_DE:
                case INC_HL:
                case INC_SP:
                    inc16(mem[regs.pc] >> 4, rp);
                    break;
                case DEC_BC:
                case DEC_DE:
                case DEC_HL:
                case DEC_SP:
                    dec16(mem[regs.pc] >> 4, rp);
                    break;
                case INC_B:
                case INC_C:
                case INC_D:
                case INC_E:
                case INC_H:
                case INC_L:
                    inc8(mem[regs.pc], rp);
                    break;
                case DEC_B:
                case DEC_C:
                case DEC_D:
                case DEC_E:
                case DEC_H:
                case DEC_L:
                    dec8(mem[regs.pc], rp);
                    break;
                case LD_B_IMM:
                case LD_C_IMM:
                case LD_D_IMM:
                case LD_E_IMM:
                case LD_H_IMM:
                case LD_L_IMM:
                    ld8imm(mem[regs.pc], rp);
                    break;
                case RRCA:
                    rrc8(mem[regs.pc], rp);
                    break;
                case EX_AF_AF:
                    regs.af.exchange();
                    break;
                case ADD_HL_BC:
                case ADD_HL_DE:
                case ADD_HL_HL:
                case ADD_HL_SP:
                    regs.hl.combined += rp[mem[regs.pc] >> 4]->combined;
                    regs.hl.uncombine();
                    break;
                case LD_A_DBC:
                case LD_A_DDE:
                    regs.af.high = deref16_u8(mem[regs.pc] >> 4, rp);
                    regs.af.combine();
                    break;
                default:
#ifndef NDEBUG
                    cout << std::hex << std::showbase
                         << "af: " << regs.af.combined << endl
                         << "af': " << regs.af.exx << endl
                         << "bc: " << regs.bc.combined << endl
                         << "bc': " << regs.bc.exx << endl
                         << "de: " << regs.de.combined << endl
                         << "de': " << regs.de.exx << endl
                         << "hl: " << regs.hl.combined << endl
                         << "hl': " << regs.hl.exx << endl
                         << "sp: " << regs.sp.combined << endl
                         << "a: " << +regs.af.high << endl
                         << "f: " << +regs.af.low << endl
                         << "b: " << +regs.bc.high << endl
                         << "c: " << +regs.bc.low << endl
                         << "d: " << +regs.de.high << endl
                         << "e: " << +regs.de.low << endl
                         << "h: " << +regs.hl.high << endl
                         << "l: " << +regs.hl.low << endl;
#endif
                    throw std::logic_error("Unimplemented opcode!");
            }
            regs.pc++;
            inst++;
        }
        return inst;
    }
}

main.cpp:

#include <cerrno>
#include <cstdlib>
#include <cstring>
#include <fstream>
#include <iostream>
#include <exception>
#include "z80emu.hpp"

void usage(const char *prog_name);

int main(int argc, const char **argv)
{
    z80emu::z80 z80;
    std::ifstream infile;
    uint16_t file_size;

    if((unsigned)argc - 2 > 0)
    {
         usage(argv[0]);
         return EXIT_FAILURE;
    }

    infile.open(argv[1], std::ifstream::in | std::ifstream::binary);
    if(!infile.good())
    {
        std::cerr << "Opening " << argv[1] << " failed: "
                  << std::strerror(errno) << std::endl;
    }

    file_size = infile.seekg(0, infile.end).tellg();
    infile.seekg(0, infile.beg);

    infile.read((char *)z80.mem, file_size);

    try {
        z80.emulate();
    } catch(std::exception &e) {
        std::cerr << "Emulation failed: " << e.what() << std::endl;
        return EXIT_FAILURE;
    }

    return 0;
}

void usage(const char *progname)
{
    std::cout << "  Usage: " << progname << " z80-prog" << std::endl;
}

opcodes.h:

#ifndef Z80EMU_OPCODES_H
#define Z80EMU_OPCODES_H 1

#define NOP     0x00
#define LD_BC_IMM   0x01
#define LD_DBC_A    0x02
#define INC_BC      0x03
#define INC_B       0x04
#define DEC_B       0x05
#define LD_B_IMM    0x06
#define RLCA        0x07
#define EX_AF_AF    0x08
#define ADD_HL_BC   0x09
#define LD_A_DBC    0x0a
#define DEC_BC      0x0b
#define INC_C       0x0c
#define DEC_C       0x0d
#define LD_C_IMM    0x0e
#define RRCA        0x0f
#define DJNZ_IMM    0x10
#define LD_DE_IMM   0x11
#define LD_DDE_A    0x12
#define INC_DE      0x13
#define INC_D       0x14
#define DEC_D       0x15
#define LD_D_IMM    0x16
#define RLA     0x17
#define JR_IMM      0x18
#define ADD_HL_DE   0x19
#define LD_A_DDE    0x1a
#define DEC_DE      0x1b
#define INC_E       0x1c
#define DEC_E       0x1d
#define LD_E_IMM    0x1e
#define RRA     0x1f
#define JR_NZ_IMM   0x20
#define LD_HL_IMM   0x21
#define LD_DIMM_HL  0x22
#define INC_HL      0x23
#define INC_H       0x24
#define DEC_H       0x25
#define LD_H_IMM    0x26
#define DAA     0x27
#define JR_Z_IMM    0x28
#define ADD_HL_HL   0x29
#define LD_HL_DIMM  0x2a
#define DEC_HL      0x2b
#define INC_L       0x2c
#define DEC_L       0x2d
#define LD_L_IMM    0x2e
#define CPL     0x2f
#define JR_NC_IMM   0x30
#define LD_SP_IMM   0x31
#define LD_DIMM_A   0x32
#define INC_SP      0x33
#define INC_DHL     0x34
#define DEC_DHL     0x35
#define LD_DHL_IMM  0x36
#define SCF     0x37
#define JR_C_IMM    0x38
#define ADD_HL_SP   0x39
#define LD_A_DIMM   0x3a
#define DEC_SP      0x3b
#define INC_A       0x3c
#define DEC_A       0x3d
#define LD_A_IMM    0x3e
#define CCF     0x3f

#endif

tools.cpp:

#include "z80emu.hpp"

namespace z80emu
{
    // return reference to 8-bit register or memory location
    uint8_t &z80::ref8(uint8_t idx, bool low, reg **tab)
    {
        // idx is the index for the 16-bit register (usually op >> 4)

        // if low is true, return the low part of the variable,
        // otherwise return the high part (usually !!(op & 8))

        switch(idx & 3)
        {
            case 3:
                return low ? regs.af.high : mem[regs.hl.combined];
            default:
                return low ? tab[idx]->low : tab[idx]->high;
        }
    }

    // return reference to a byte in memory
    // specified by a 16-bit pointer
    uint8_t &z80::deref16_u8(uint8_t idx, reg **tab)
    {
        return mem[tab[idx]->combined];
    }

    // load 16-bit register with immediate
    void z80::ld16imm(uint8_t idx, reg **tab)
    {
        // Do these individually because
        // of endianness and memory wrapping
        tab[idx]->low = mem[++regs.pc];
        tab[idx]->high = mem[++regs.pc];
        tab[idx]->combine();
    }

    // load 8-bit register with immediate
    void z80::ld8imm(uint8_t op, reg **tab)
    {
        uint8_t idx = op >> 4;
        ref8(idx, !!(op & 8), tab) = mem[++regs.pc];
        if((idx & 3) != 3) tab[idx]->combine();
        else if(op & 8) regs.af.combine();
    }

    // increment 16-bit register
    void z80::inc16(uint8_t idx, reg **tab)
    {
        tab[idx]->combined++;
        tab[idx]->uncombine();
    }

    // decrement 16-bit register
    void z80::dec16(uint8_t idx, reg **tab)
    {
        tab[idx]->combined--;
        tab[idx]->uncombine();
    }

    // add number to 8-bit register
    void z80::add8(uint8_t op, reg **tab, uint8_t incr)
    {
        uint8_t idx = op >> 4;
        ref8(idx, !!(op & 8), tab) += incr;
        if((idx & 3) != 3) tab[idx]->combine();
        else if(op & 8) regs.af.combine();
    }

    // increment 8-bit register
    void z80::inc8(uint8_t op, reg **tab)
    {
        add8(op, tab, 1);
    }

    // decrement 8-bit register
    void z80::dec8(uint8_t op, reg **tab)
    {
        add8(op, tab, -1);
    }

    void z80::rrc8(uint8_t op, reg **tab)
    {
        uint8_t idx = (op & 0x7) >> 1;
        uint8_t &ref = ref8(idx, op & 1, tab);
        ref = ref >> 1 | (ref & 1) << 7;
        if((idx & 3) != 3) tab[idx]->combine();
        else if((op & 0x7) == 0x7) regs.af.combine();
    }
}

z80emu.hpp:

#ifndef Z80EMU_HPP
#define Z80EMU_HPP 1

#if __cplusplus >= 201103L
# include <cstdint>
# include <utility>
using std::uint16_t;
using std::uint8_t;
#else
# include <stdint.h>
# include <algorithm>
#endif

#include <cstring>
#include <vector>

// TODO: Decide whether to use struct or class

namespace z80emu
{
    enum cc
    {
        NZ = 0,
        Z  = 1,
        NC = 2,
        C  = 3,
        PO = 4,
        PE = 5,
        P  = 6,
        M  = 7
    };

    struct reg
    {
        uint8_t high, low;
        uint16_t combined, exx;

        void combine()
        {
            combined = high << 8 | low;
        }
        void uncombine()
        {
            high = combined >> 8;
            low = combined;
        }
        void exchange()
        {
            std::swap(combined, exx);
            uncombine();
        }
    };

#if __cplusplus >= 201103L
    static_assert(sizeof(reg) == 6, "sizeof(reg) != 6");
#endif

    struct registers
    {
        reg af;
        reg bc;
        reg de;
        reg hl;
        reg ix;
        reg iy;
        reg sp;
        reg wz;
        uint16_t pc;
    };

    struct z80
    {
        uint8_t *mem;
        registers regs;

        uint16_t emulate();
        uint8_t &ref8(uint8_t op, bool low, reg **tab);
        uint8_t &deref16_u8(uint8_t op, reg **tab);
        void ld16imm(uint8_t op, reg **tab);
        void ld8imm(uint8_t op, reg **tab);
        void inc16(uint8_t op, reg **tab);
        void dec16(uint8_t op, reg **tab);
        void add8(uint8_t op, reg **tab, uint8_t incr);
        void inc8(uint8_t op, reg **tab);
        void dec8(uint8_t op, reg **tab);
        void rrc8(uint8_t op, reg **tab);

        z80()
        {
            mem = new uint8_t[0xffff];
        }

        ~z80()
        {
            delete[] mem;
        }
    };
}

#endif

Shell script to create example program (filename test) that will cause the program built with the modified version of emulate.cpp to print the registers on exit:

#!/bin/sh

printf '\001\275\274\021\337\336\041\316\315\061\255\336\377' > test

I'd like to note that later on in the implementation (when I get 15/16ths of it done), rp2 will be used for push and pop so the af register (accumulator and flags) can be saved.

And, for the instructions I do have implemented, everything seems to be working fine (as in, the register values seem to be correct when I use gdb and std::cout).

What I'm mainly interested in:

  • Are there any "more C++" things (that work from c++98 to c++2a) that I can do? Have I started using the language's featured adequately?

  • Can I reduce code duplication? Most of the functions in tools.cpp have to call uncombine or combine after altering the registers. Is there a way around this? What about with the rrc8 function?

  • Are there any C++ "best practices" that I'm missing here?

  • If there are any other miscellaneous things that could be improved, please let me know, although I would appreciate it if you could focus on the other points more.

\$\endgroup\$
4
  • 1
    \$\begingroup\$ I would suggest that in order to get better reviews, you include a sample file that your program can emulate. \$\endgroup\$
    – user33306
    Commented Nov 26, 2019 at 0:03
  • \$\begingroup\$ @tinstaafl I can do that, but it won't be able to do much other than modify the registers. I'll go ahead and add a version of emulate.cpp that prints the registers, too. \$\endgroup\$
    – S.S. Anne
    Commented Nov 26, 2019 at 0:05
  • \$\begingroup\$ That's fine. It's mainly so reviewers can step through your code. \$\endgroup\$
    – user33306
    Commented Nov 26, 2019 at 0:08
  • \$\begingroup\$ @tinstaafl Done. \$\endgroup\$
    – S.S. Anne
    Commented Nov 26, 2019 at 0:15

1 Answer 1

5
\$\begingroup\$

A few quick observations:

  1. Don't use #define for all those compile time constants (opcodes). They should be part of an opcode enum or declared as constexpr int.

  2. You're only allocating 65,535 bytes for mem, when you should be allocating 65,536. An attempt to access mem[0xFFFF] will result in Undefined Behavior because it is past the end of the allocated space.

  3. Having a default case label to catch unimplemented opcodes is a Good Thing.

  4. if((unsigned)argc - 2 > 0) is somewhat obscure. What's wrong with if (argc != 2)?

  5. I'm not sure what you're trying to do with your reg struct, but you could eliminate the duplicate data storage to add byte and word accessor methods. Both GCC and Clang will optimize (-O3) these to single instruction byte accesses. MSVC does the same for getters, but setters are not fully optimized (with /O2 and variations I tried).

    struct reg {
        uint16_t combined;
        uint8_t high() const
        {
            return combined >> 8;
        }
        uint8_t low() const
        {
            return combined & 255;
        }
        void seth(uint8_t v)
        {
            combined = (combined & 255) | (v << 8);
        }
        void setl(uint8_t v)
        {
            combined = (combined & ~255) | v;
        }
    };
    

    combined could be made private and accessors added to it if desired.

  6. While it is unlikely a z80 object will be copied, you should =delete the copy and move constructors and assignment operators. See this question on Stack Overflow.

\$\endgroup\$
7
  • \$\begingroup\$ Uhh... What does point 6 mean? I think you should see the beginner tag. I'm also slightly confused by what constexpr int is. \$\endgroup\$
    – S.S. Anne
    Commented Nov 26, 2019 at 0:48
  • \$\begingroup\$ Ah, I should've mentioned that I wanted this to be portable. \$\endgroup\$
    – S.S. Anne
    Commented Nov 26, 2019 at 1:02
  • 1
    \$\begingroup\$ To get away from combine()/uncombine(), it might be best to give reg some member functions to read/write individual registers in the pair. \$\endgroup\$ Commented Nov 26, 2019 at 10:22
  • 1
    \$\begingroup\$ @TobySpeight After marinating on this overnight, I had similar thoughts and updated my answer to use member functions instead of punning with a union. \$\endgroup\$ Commented Nov 26, 2019 at 16:12
  • 1
    \$\begingroup\$ @JL2210 If you've made changes to the code that you want reviewed, they should be posted as a follow on question. \$\endgroup\$ Commented Dec 4, 2019 at 20:32

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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