From 8113e27292af16a28c00cbee7a712730f06aa301 Mon Sep 17 00:00:00 2001 From: ajrhacker Date: Thu, 12 Oct 2017 14:39:51 -0400 Subject: device_state_interface: Polymorphism and std::function for entries (nw) (#2690) * device_state_interface: Polymorphism and std::function for entries (nw) - Create a templated subclass of device_state_entry to provide separate read/write interfaces for registers of varying widths. The efficiency impact of this should be minimal, given that this eliminates the need to make each byte width a subcase for reads and writes. - Create similarly templated "pseudo-register" versions of device_state_entry that provides custom read/write interfaces through std::function. The intent of this is to eventually replace the dummy register + state_export interface hitherto necessary to provide debugger access to bankswitched or computed state registers. - State registers can now be made read-only, and this is automatically done now when state_add is called with a std::function read handler but no write handler. This property is honored by MAME debug expressions. * Add override keyword (nw) * Remove explicit instantiations that were causing linking errors in tools build (nw) --- src/devices/cpu/cop400/cop400.cpp | 47 ++++++++---------- src/devices/cpu/cop400/cop400.h | 9 ++-- src/emu/debug/debugcon.cpp | 8 +-- src/emu/debug/debugcon.h | 2 +- src/emu/debug/debugcpu.cpp | 2 +- src/emu/distate.cpp | 100 +++++++++++++++----------------------- src/emu/distate.h | 96 ++++++++++++++++++++++++++++++++---- 7 files changed, 156 insertions(+), 108 deletions(-) diff --git a/src/devices/cpu/cop400/cop400.cpp b/src/devices/cpu/cop400/cop400.cpp index 30b1680150a..696e504835d 100644 --- a/src/devices/cpu/cop400/cop400.cpp +++ b/src/devices/cpu/cop400/cop400.cpp @@ -1117,9 +1117,10 @@ void cop400_cpu_device::device_start() // setup debugger state display offs_t pc_mask = m_program->addrmask(); + using namespace std::placeholders; state_add(STATE_GENPC, "GENPC", m_pc).mask(pc_mask).noshow(); state_add(STATE_GENPCBASE, "CURPC", m_prevpc).mask(pc_mask).noshow(); - state_add(STATE_GENFLAGS, "GENFLAGS", m_flags).mask(0x7).callimport().callexport().noshow().formatstr("%3s"); + state_add(STATE_GENFLAGS, "GENFLAGS", std::bind(&cop400_cpu_device::get_flags, this), std::bind(&cop400_cpu_device::set_flags, this, _1)).mask(0x7).noshow().formatstr("%3s"); state_add(COP400_PC, "PC", m_pc).mask(pc_mask); state_add(COP400_SA, "SA", m_sa).mask(pc_mask); state_add(COP400_SB, "SB", m_sb).mask(pc_mask); @@ -1127,7 +1128,7 @@ void cop400_cpu_device::device_start() state_add(COP400_SC, "SC", m_sc).mask(pc_mask); state_add(COP400_B, "B", m_b); state_add(COP400_A, "A", m_a).mask(0xf); - state_add(COP400_M, "M", m_temp_m).mask(0xf).callimport().callexport(); + state_add(COP400_M, "M", std::bind(&cop400_cpu_device::get_m, this), std::bind(&cop400_cpu_device::set_m, this, _1)).mask(0xf); state_add(COP400_G, "G", m_g).mask(0xf); state_add(COP400_Q, "Q", m_q); state_add(COP400_SIO, "SIO", m_sio).mask(0xf).formatstr("%4s"); @@ -1145,8 +1146,6 @@ void cop400_cpu_device::device_start() m_sb = 0; m_sc = 0; m_sio = 0; - m_flags = 0; - m_temp_m = 0; m_il = 0; m_in[0] = m_in[1] = m_in[2] = m_in[3] = 0; m_si = 0; @@ -1309,36 +1308,28 @@ void cop400_cpu_device::execute_run() GENERAL CONTEXT ACCESS ***************************************************************************/ -void cop400_cpu_device::state_import(const device_state_entry &entry) +uint8_t cop400_cpu_device::get_flags() const { - switch (entry.index()) - { - case STATE_GENFLAGS: - m_skt_latch = BIT(m_flags, 2); - m_c = BIT(m_flags, 1); - m_skl = BIT(m_flags, 0); - break; + return (m_skt_latch ? 0x04 : 0x00) | (m_c ? 0x02 : 0x00) | (m_skl ? 0x01 : 0x00); +} - case COP400_M: - auto dis = machine().disable_side_effect(); - RAM_W(B, m_temp_m); - break; - } +void cop400_cpu_device::set_flags(uint8_t flags) +{ + m_skt_latch = BIT(flags, 2); + m_c = BIT(flags, 1); + m_skl = BIT(flags, 0); } -void cop400_cpu_device::state_export(const device_state_entry &entry) +uint8_t cop400_cpu_device::get_m() const { - switch (entry.index()) - { - case STATE_GENFLAGS: - m_flags = (m_skt_latch ? 0x04 : 0x00) | (m_c ? 0x02 : 0x00) | (m_skl ? 0x01 : 0x00); - break; + auto dis = machine().disable_side_effect(); + return RAM_R(B); +} - case COP400_M: - auto dis = machine().disable_side_effect(); - m_temp_m = RAM_R(B); - break; - } +void cop400_cpu_device::set_m(uint8_t m) +{ + auto dis = machine().disable_side_effect(); + RAM_W(B, m); } void cop400_cpu_device::state_string_export(const device_state_entry &entry, std::string &str) const diff --git a/src/devices/cpu/cop400/cop400.h b/src/devices/cpu/cop400/cop400.h index 13461ffe583..3a047d10c8a 100644 --- a/src/devices/cpu/cop400/cop400.h +++ b/src/devices/cpu/cop400/cop400.h @@ -162,8 +162,6 @@ protected: virtual space_config_vector memory_space_config() const override; // device_state_interface overrides - virtual void state_import(const device_state_entry &entry) override; - virtual void state_export(const device_state_entry &entry) override; virtual void state_string_export(const device_state_entry &entry, std::string &str) const override; // device_disasm_interface overrides @@ -226,8 +224,6 @@ protected: uint16_t m_sa, m_sb, m_sc; /* subroutine save registers */ uint8_t m_sio; /* 4-bit shift register and counter */ int m_skl; /* 1-bit latch for SK output */ - uint8_t m_flags; // used for debugger state only - uint8_t m_temp_m; // 4-bit RAM at B (for debugger state only) /* counter */ uint8_t m_t; /* 8-bit timer */ @@ -289,6 +285,11 @@ protected: void skip(); void sk_update(); + uint8_t get_flags() const; + void set_flags(uint8_t flags); + uint8_t get_m() const; + void set_m(uint8_t m); + void illegal(uint8_t operand); void asc(uint8_t operand); void add(uint8_t operand); diff --git a/src/emu/debug/debugcon.cpp b/src/emu/debug/debugcon.cpp index 8b0cd786bff..16f15fbbc60 100644 --- a/src/emu/debug/debugcon.cpp +++ b/src/emu/debug/debugcon.cpp @@ -344,7 +344,7 @@ CMDERR debugger_console::execute_command(const std::string &command, bool echo) if (!echo) printf(">%s\n", command.c_str()); printf(" %*s^\n", CMDERR_ERROR_OFFSET(result), ""); - printf("%s\n", cmderr_to_string(result)); + printf("%s\n", cmderr_to_string(result).c_str()); } /* update all views */ @@ -404,8 +404,9 @@ void debugger_console::register_command(const char *command, u32 flags, int ref, for a given command error -------------------------------------------------*/ -const char *debugger_console::cmderr_to_string(CMDERR error) +std::string debugger_console::cmderr_to_string(CMDERR error) { + int offset = CMDERR_ERROR_OFFSET(error); switch (CMDERR_ERROR_CLASS(error)) { case CMDERR_UNKNOWN_COMMAND: return "unknown command"; @@ -414,7 +415,8 @@ const char *debugger_console::cmderr_to_string(CMDERR error) case CMDERR_UNBALANCED_QUOTES: return "unbalanced quotes"; case CMDERR_NOT_ENOUGH_PARAMS: return "not enough parameters for command"; case CMDERR_TOO_MANY_PARAMS: return "too many parameters for command"; - case CMDERR_EXPRESSION_ERROR: return "error in assignment expression"; + case CMDERR_EXPRESSION_ERROR: return string_format("error in assignment expression: %s", + expression_error(static_cast(offset)).code_string()); default: return "unknown error"; } } diff --git a/src/emu/debug/debugcon.h b/src/emu/debug/debugcon.h index 54bc3bf6772..7cc1231d4c9 100644 --- a/src/emu/debug/debugcon.h +++ b/src/emu/debug/debugcon.h @@ -104,7 +104,7 @@ public: vprintf_wrap(wrapcol, util::make_format_argument_pack(std::forward(fmt), std::forward(args)...)); } - static const char * cmderr_to_string(CMDERR error); + static std::string cmderr_to_string(CMDERR error); private: void exit(); diff --git a/src/emu/debug/debugcpu.cpp b/src/emu/debug/debugcpu.cpp index 46ad9e04683..5b41d0f8241 100644 --- a/src/emu/debug/debugcpu.cpp +++ b/src/emu/debug/debugcpu.cpp @@ -1560,7 +1560,7 @@ device_debug::device_debug(device_t &device) for (const auto &entry : m_state->state_entries()) { strmakelower(tempstr.assign(entry->symbol())); - m_symtable.add(tempstr.c_str(), (void *)(uintptr_t)entry->index(), get_state, set_state, entry->format_string()); + m_symtable.add(tempstr.c_str(), (void *)(uintptr_t)entry->index(), get_state, entry->writeable() ? set_state : nullptr, entry->format_string()); } } diff --git a/src/emu/distate.cpp b/src/emu/distate.cpp index 2a1ed9b9ea9..b40537ec501 100644 --- a/src/emu/distate.cpp +++ b/src/emu/distate.cpp @@ -49,30 +49,18 @@ const u64 device_state_entry::k_decimal_divisor[] = // device_state_entry - constructor //------------------------------------------------- -device_state_entry::device_state_entry(int index, const char *symbol, void *dataptr, u8 size, device_state_interface *dev) +device_state_entry::device_state_entry(int index, const char *symbol, u8 size, u64 sizemask, device_state_interface *dev) : m_device_state(dev), m_index(index), - m_dataptr(dataptr), - m_datamask(0), + m_datamask(sizemask), m_datasize(size), m_flags(0), m_symbol(symbol), m_default_format(true), - m_sizemask(0) + m_sizemask(sizemask) { - // convert the size to a mask assert(size == 1 || size == 2 || size == 4 || size == 8); - if (size == 1) - m_sizemask = 0xff; - else if (size == 2) - m_sizemask = 0xffff; - else if (size == 4) - m_sizemask = 0xffffffff; - else - m_sizemask = ~u64(0); - - // default the data mask to the same - m_datamask = m_sizemask; + format_from_mask(); // override well-known symbols @@ -87,10 +75,9 @@ device_state_entry::device_state_entry(int index, const char *symbol, void *data device_state_entry::device_state_entry(int index, device_state_interface *dev) : m_device_state(dev), m_index(index), - m_dataptr(nullptr), m_datamask(0), m_datasize(0), - m_flags(DSF_DIVIDER), + m_flags(DSF_DIVIDER | DSF_READONLY), m_symbol(), m_default_format(true), m_sizemask(0) @@ -135,22 +122,23 @@ void device_state_entry::format_from_mask() //------------------------------------------------- -// value - return the current value as a u64 +// entry_baseptr - return a pointer to where the +// data lives (if applicable) //------------------------------------------------- -u64 device_state_entry::value() const +void *device_state_entry::entry_baseptr() const { - // pick up the value - u64 result; - switch (m_datasize) - { - default: - case 1: result = *static_cast(m_dataptr); break; - case 2: result = *static_cast(m_dataptr); break; - case 4: result = *static_cast(m_dataptr); break; - case 8: result = *static_cast(m_dataptr); break; - } - return result & m_datamask; + return nullptr; +} + + +//------------------------------------------------- +// entry_value - return the current value as a u64 +//------------------------------------------------- + +u64 device_state_entry::entry_value() const +{ + return 0; } @@ -349,6 +337,8 @@ std::string device_state_entry::format(const char *string, bool maxout) const void device_state_entry::set_value(u64 value) const { + assert((m_flags & DSF_READONLY) == 0); + // apply the mask value &= m_datamask; @@ -357,14 +347,16 @@ void device_state_entry::set_value(u64 value) const value |= ~m_datamask; // store the value - switch (m_datasize) - { - default: - case 1: *static_cast(m_dataptr) = value; break; - case 2: *static_cast(m_dataptr) = value; break; - case 4: *static_cast(m_dataptr) = value; break; - case 8: *static_cast(m_dataptr) = value; break; - } + entry_set_value(value); +} + + +//------------------------------------------------- +// entry_set_value - set the value from a u64 +//------------------------------------------------- + +void device_state_entry::entry_set_value(u64 value) const +{ } @@ -509,38 +501,22 @@ void device_state_interface::set_state_string(int index, const char *string) //------------------------------------------------- -// state_add - return the value of the given -// pieces of indexed state as a u64 +// state_add - add a new piece of indexed state //------------------------------------------------- -device_state_entry &device_state_interface::state_add(int index, const char *symbol, void *data, u8 size) +device_state_entry &device_state_interface::state_add(std::unique_ptr &&entry) { - // assert validity of incoming parameters - assert(size == 1 || size == 2 || size == 4 || size == 8); - assert(symbol != nullptr); - // append to the end of the list - m_state_list.push_back(std::make_unique(index, symbol, data, size, this)); + m_state_list.push_back(std::move(entry)); + device_state_entry &new_entry = *m_state_list.back(); // set the fast entry if applicable - if (index >= FAST_STATE_MIN && index <= FAST_STATE_MAX) - m_fast_state[index - FAST_STATE_MIN] = m_state_list.back().get(); + if (new_entry.index() >= FAST_STATE_MIN && new_entry.index() <= FAST_STATE_MAX && !new_entry.divider()) + m_fast_state[new_entry.index() - FAST_STATE_MIN] = &new_entry; - return *m_state_list.back().get(); + return new_entry; } -//------------------------------------------------- -// state_add_divider - add a simple divider -// entry -//------------------------------------------------- - -device_state_entry &device_state_interface::state_add_divider(int index) -{ - // append to the end of the list - m_state_list.push_back(std::make_unique(index, this)); - - return *m_state_list.back().get(); -} //------------------------------------------------- // state_import - called after new state is diff --git a/src/emu/distate.h b/src/emu/distate.h index 22db6cc22af..743b2f8d670 100644 --- a/src/emu/distate.h +++ b/src/emu/distate.h @@ -46,7 +46,7 @@ class device_state_entry friend class device_state_interface; public: // construction/destruction - device_state_entry(int index, const char *symbol, void *dataptr, u8 size, device_state_interface *dev); + device_state_entry(int index, const char *symbol, u8 size, u64 sizemask, device_state_interface *dev); device_state_entry(int index, device_state_interface *dev); public: @@ -57,12 +57,15 @@ public: device_state_entry &callimport() { m_flags |= DSF_IMPORT; return *this; } device_state_entry &callexport() { m_flags |= DSF_EXPORT; return *this; } device_state_entry &noshow() { m_flags |= DSF_NOSHOW; return *this; } + device_state_entry &readonly() { m_flags |= DSF_READONLY; return *this; } // query information int index() const { return m_index; } - void *dataptr() const { return m_dataptr; } + void *dataptr() const { return entry_baseptr(); } + u64 datamask() const { return m_datamask; } const char *symbol() const { return m_symbol.c_str(); } bool visible() const { return ((m_flags & DSF_NOSHOW) == 0); } + bool writeable() const { return ((m_flags & DSF_READONLY) == 0); } bool divider() const { return m_flags & DSF_DIVIDER; } device_state_interface *parent_state() const {return m_device_state;} const std::string &format_string() const { return m_format; } @@ -75,6 +78,7 @@ protected: static constexpr u8 DSF_EXPORT = 0x08; // call the export function prior to fetching the data static constexpr u8 DSF_CUSTOM_STRING = 0x10; // set if the format has a custom string static constexpr u8 DSF_DIVIDER = 0x20; // set if this is a divider entry + static constexpr u8 DSF_READONLY = 0x40; // set if this entry does not permit writes // helpers bool needs_custom_string() const { return ((m_flags & DSF_CUSTOM_STRING) != 0); } @@ -82,7 +86,7 @@ protected: // return the current value -- only for our friends who handle export bool needs_export() const { return ((m_flags & DSF_EXPORT) != 0); } - u64 value() const; + u64 value() const { return entry_value() & m_datamask; } std::string format(const char *string, bool maxout = false) const; // set the current value -- only for our friends who handle import @@ -90,13 +94,17 @@ protected: void set_value(u64 value) const; void set_value(const char *string) const; + // overrides + virtual void *entry_baseptr() const; + virtual u64 entry_value() const; + virtual void entry_set_value(u64 value) const; + // statics static const u64 k_decimal_divisor[20]; // divisors for outputting decimal values // public state description device_state_interface *m_device_state; // link to parent device state u32 m_index; // index by which this item is referred - void * m_dataptr; // pointer to where the data lives u64 m_datamask; // mask that applies to the data u8 m_datasize; // size of the data u8 m_flags; // flags for this data @@ -107,6 +115,59 @@ protected: }; +// ======================> device_state_register + +// class template representing a state register of a specific width +template +class device_state_register : public device_state_entry +{ +public: + // construction/destruction + device_state_register(int index, const char *symbol, ItemType &data, device_state_interface *dev) + : device_state_entry(index, symbol, sizeof(ItemType), std::numeric_limits::max(), dev), + m_data(data) + { + } + +protected: + // device_state_entry overrides + virtual void *entry_baseptr() const override { return &m_data; } + virtual u64 entry_value() const override { return m_data; } + virtual void entry_set_value(u64 value) const override { m_data = value; } + +private: + ItemType & m_data; // reference to where the data lives +}; + + +// ======================> device_state_register + +// class template representing a state register of a specific width +template +class device_pseudo_state_register : public device_state_entry +{ +public: + typedef typename std::function getter_func; + typedef typename std::function setter_func; + + // construction/destruction + device_pseudo_state_register(int index, const char *symbol, getter_func &&getter, setter_func &&setter, device_state_interface *dev) + : device_state_entry(index, symbol, sizeof(ItemType), std::numeric_limits::max(), dev), + m_getter(std::move(getter)), + m_setter(std::move(setter)) + { + } + +protected: + // device_state_entry overrides + virtual u64 entry_value() const override { return m_getter(); } + virtual void entry_set_value(u64 value) const override { m_setter(value); } + +private: + getter_func m_getter; // function to retrieve the data + setter_func m_setter; // function to store the data +}; + // ======================> device_state_interface @@ -143,15 +204,32 @@ public: public: // protected eventually - // add a new state item - template device_state_entry &state_add(int index, const char *symbol, _ItemType &data) + // add a new state register item + template device_state_entry &state_add(int index, const char *symbol, ItemType &data) + { + assert(symbol != nullptr); + return state_add(std::make_unique>(index, symbol, data, this)); + } + + // add a new state pseudo-register item (template argument must be explicit) + template device_state_entry &state_add(int index, const char *symbol, + typename device_pseudo_state_register::getter_func &&getter, + typename device_pseudo_state_register::setter_func &&setter) { - return state_add(index, symbol, &data, sizeof(data)); + assert(symbol != nullptr); + return state_add(std::make_unique>(index, symbol, std::move(getter), std::move(setter), this)); } - device_state_entry &state_add(int index, const char *symbol, void *data, u8 size); + template device_state_entry &state_add(int index, const char *symbol, + typename device_pseudo_state_register::getter_func &&getter) + { + assert(symbol != nullptr); + return state_add(std::make_unique>(index, symbol, std::move(getter), [](ItemType){}, this)).readonly(); + } + + device_state_entry &state_add(std::unique_ptr &&entry); // add a new divider entry - device_state_entry &state_add_divider(int index); + device_state_entry &state_add_divider(int index) { return state_add(std::make_unique(index, this)); } protected: // derived class overrides -- cgit v1.2.3