Partial Zilog Z80 emulator written in C++

Question

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.

• user33306: I would suggest that in order to get better reviews, you include a sample file that your program can emulate.
• S.S. Anne: @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.
• user33306: That's fine. It's mainly so reviewers can step through your code.
• S.S. Anne: @tinstaafl Done.

Answer 1

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.

• S.S. Anne: Uhh... What does point 6 mean? I think you should see the beginner tag. I'm also slightly confused by what constexpr int is.
• S.S. Anne: Ah, I should've mentioned that I wanted this to be portable.
• Toby Speight: To get away from combine()/uncombine(), it might be best to give reg some member functions to read/write individual registers in the pair.
• 1201ProgramAlarm: @TobySpeight After marinating on this overnight, I had similar thoughts and updated my answer to use member functions instead of punning with a union.
• 1201ProgramAlarm: @JL2210 If you've made changes to the code that you want reviewed, they should be posted as a follow on question.