Skip to content

Commit

Permalink
Implement views, which are essentially bankdevs integrated into the
Browse files Browse the repository at this point in the history
memory map system. [O. Galibert]
  • Loading branch information
galibert committed Nov 22, 2020
1 parent 8d45590 commit 574daf4
Show file tree
Hide file tree
Showing 21 changed files with 3,328 additions and 1,601 deletions.
97 changes: 97 additions & 0 deletions docs/source/techspecs/memory.rst
Expand Up @@ -64,6 +64,20 @@ Memory regions are read-only memory zones in which ROMs are loaded.

All of these have names allowing to access them.

2.4 Views
~~~~~~~~~

Views are a way to multiplex different submaps over a memory range
with fast switching. It is to be used when multiple devices map at
the same addresses and are switched in externally. They must be
created as an object of the device and then setup either statically in
a memory map or dynamically through install_* calls.

Switchable submaps, aka variants, are named through an integer. An
internal indirection through a map ensures that any integer value can
be used.


3. Memory objects
-----------------

Expand Down Expand Up @@ -225,6 +239,37 @@ The ``memregion`` device method retrieves a memory region by name.
Beware that the lookup can be expensive, prefer finders instead.


3.4 Views - memory_view
~~~~~~~~~~~~~~~~~~~~~~~

.. code-block:: C++

class memory_view {
memory_view(device_t &device, std::string name);
memory_view_entry &operator[](int slot);

void select(int entry);
void disable();

const std::string &name() const;
}

A view allows to switch part of a memory map between multiple
possibilities, or even disable it entirely to see what was there
before. It is created as an object of the device.

.. code-block:: C++

memory_view m_view;

[device constructor] m_view(*this, "name"),
It is then setup through the address map API or dynamically. The, at
runtime, a variant, which is numbered, can be selected through the
``select`` method or the view can be disabled through the ``disabled``
method. A disabled view can be reenabled at any time.


4. Address maps API
-------------------

Expand Down Expand Up @@ -543,6 +588,40 @@ trigger the handler if a wider part of the bus is accessed. The
parameter is that trigger width (would be 16 in the 68000 case).


4.5 View setup
~~~~~~~~~~~~~~

.. code-block:: C++

map(start, end).view(m_view);
m_view[0](start1, end1).[...];

A view is setup in a address map with the view method. The only
qualifier accepted is mirror. The "disabled" version of the view will
include what was in the range prior to the view setup.

The different variants are setup by indexing the view with the variant
number and setting up an entry in the usual way. The entries within a
variant must of course stay within the range. There are no other
additional constraints. The contents of a variant, by default, are
what was there before, e.g. the contents of the disabled view, and
then setting it up allows to override part or all of it.

Variants can only be setup once the view itself has been setup with
the ``view`` method.

A view can only be put in one address map and in only one position.
If multiple views have identical or similar contents remember that
setting up a map is nothing more than a method call, and creating a
second method to setup a view is perfectly reasonable. A view is of
type **memory_view** and an indexed entry (e.g. a variant to setup) is
of type **memory_view::memory_view_entry &**.

A view can be installed in another view, but don't forget that a view
can be installed only once. A view can also being part of "what was
there before".


5. Address space dynamic mapping API
------------------------------------

Expand Down Expand Up @@ -695,3 +774,21 @@ with an optional mirror.

Install a device address with an address map in a space. The
``unitmask`` and ``cswidth`` arguments are optional.

5.9 View installation
~~~~~~~~~~~~~~~~~~~~~

.. code-block:: C++

space.install_view(addrstart, addrend, view)
space.install_view(addrstart, addrend, addrmirror, view)

view[0].install...

Installs a view in a space. This can be only done once and in only
one space, and the view must not have been setup through the address
map API before. Once the view is installed variants can be selected
through indexation to call a dynamic mapping method on it.

A view can be installed into a variant of another view without issues
with the only usual constraint of single installation.
2 changes: 2 additions & 0 deletions scripts/src/emu.lua
Expand Up @@ -117,6 +117,8 @@ files {
MAME_DIR .. "src/emu/emucore.h",
MAME_DIR .. "src/emu/emumem.cpp",
MAME_DIR .. "src/emu/emumem.h",
MAME_DIR .. "src/emu/emumem_aspace.cpp",
MAME_DIR .. "src/emu/emumem_mview.cpp",
MAME_DIR .. "src/emu/emumem_mud.cpp",
MAME_DIR .. "src/emu/emumem_mud.h",
MAME_DIR .. "src/emu/emumem_hea.h",
Expand Down
77 changes: 38 additions & 39 deletions src/emu/addrmap.cpp
Expand Up @@ -19,39 +19,6 @@

#define DETECT_OVERLAPPING_MEMORY (0)

/*-------------------------------------------------
core_i64_hex_format - i64 format printf helper
why isn't fatalerror going through the same
channels as logerror exactly?
-------------------------------------------------*/

static char *core_i64_hex_format(u64 value, u8 mindigits)
{
static char buffer[16][64];
// TODO: this can overflow - e.g. when a lot of unmapped writes are logged
static int index;
char *bufbase = &buffer[index++ % 16][0];
char *bufptr = bufbase;
s8 curdigit;

for (curdigit = 15; curdigit >= 0; curdigit--)
{
int nibble = (value >> (curdigit * 4)) & 0xf;
if (nibble != 0 || curdigit < mindigits)
{
mindigits = curdigit;
*bufptr++ = "0123456789ABCDEF"[nibble];
}
}
if (bufptr == bufbase)
*bufptr++ = '0';
*bufptr = 0;

return bufbase;
}



//**************************************************************************
// ADDRESS MAP ENTRY
//**************************************************************************
Expand Down Expand Up @@ -201,6 +168,15 @@ address_map_entry &address_map_entry::m(device_t *device, address_map_constructo
return *this;
}

void address_map_entry::view(memory_view &mv)
{
m_read.m_type = AMH_VIEW;
m_read.m_tag = nullptr;
m_write.m_type = AMH_VIEW;
m_write.m_tag = nullptr;
m_view = &mv;
mv.initialize_from_address_map(m_addrstart, m_addrend, m_map.get_config());
}

//-------------------------------------------------
// r/w/rw - handler setters for
Expand Down Expand Up @@ -787,17 +763,18 @@ bool address_map_entry::unitmask_is_appropriate(u8 width, u64 unitmask, const ch
address_map::address_map(device_t &device, int spacenum)
: m_spacenum(spacenum),
m_device(&device),
m_view(nullptr),
m_unmapval(0),
m_globalmask(0)
m_globalmask(0)
{
// get our memory interface
const device_memory_interface *memintf;
if (!m_device->interface(memintf))
throw emu_fatalerror("No memory interface defined for device '%s'\n", m_device->tag());

// and then the configuration for the current address space
const address_space_config *spaceconfig = memintf->space_config(spacenum);
if (spaceconfig == nullptr)
m_config = memintf->space_config(spacenum);
if (m_config == nullptr)
throw emu_fatalerror("No memory address space configuration found for device '%s', space %d\n", m_device->tag(), spacenum);

// append the map provided by the owner
Expand All @@ -809,8 +786,8 @@ address_map::address_map(device_t &device, int spacenum)
}

// construct the internal device map (last so it takes priority)
if (!spaceconfig->m_internal_map.isnull())
spaceconfig->m_internal_map(*this);
if (!m_config->m_internal_map.isnull())
m_config->m_internal_map(*this);
}


Expand All @@ -822,6 +799,7 @@ address_map::address_map(device_t &device, int spacenum)
address_map::address_map(device_t &device, address_map_entry *entry)
: m_spacenum(AS_PROGRAM),
m_device(&device),
m_view(nullptr),
m_unmapval(0),
m_globalmask(0)
{
Expand All @@ -839,13 +817,29 @@ address_map::address_map(device_t &device, address_map_entry *entry)
address_map::address_map(const address_space &space, offs_t start, offs_t end, u64 unitmask, int cswidth, device_t &device, address_map_constructor submap_delegate)
: m_spacenum(space.spacenum()),
m_device(&device),
m_view(nullptr),
m_unmapval(space.unmap()),
m_globalmask(space.addrmask())
{
(*this)(start, end).m(DEVICE_SELF, submap_delegate).umask64(unitmask).cswidth(cswidth);
}


//----------------------------------------------------------
// address_map - constructor memory view mapping case
//----------------------------------------------------------

address_map::address_map(memory_view &view)
: m_spacenum(-1),
m_device(&view.m_device),
m_view(&view),
m_config(view.m_config),
m_unmapval(0),
m_globalmask(0)
{
}


//-------------------------------------------------
// ~address_map - destructor
//-------------------------------------------------
Expand Down Expand Up @@ -981,7 +975,7 @@ void address_map::import_submaps(running_machine &machine, device_t &owner, int
subentry_ratio ++;
subentry_ratio = data_width / subentry_ratio;
if (ratio * subentry_ratio > data_width / 8)
fatalerror("import_submap: In range %x-%x mask %x mirror %x select %x of device %s, the import unitmask of %s combined with an entry unitmask of %s does not fit in %d bits.\n", subentry->m_addrstart, subentry->m_addrend, subentry->m_addrmask, subentry->m_addrmirror, subentry->m_addrselect, entry->m_read.m_tag, core_i64_hex_format(entry->m_mask, data_width/4), core_i64_hex_format(subentry->m_mask, data_width/4), data_width);
fatalerror("import_submap: In range %x-%x mask %x mirror %x select %x of device %s, the import unitmask of %0*x combined with an entry unitmask of %0*x does not fit in %d bits.\n", subentry->m_addrstart, subentry->m_addrend, subentry->m_addrmask, subentry->m_addrmirror, subentry->m_addrselect, entry->m_read.m_tag, data_width/4, entry->m_mask, data_width/4, subentry->m_mask, data_width);

// Regenerate the unitmask
u64 newmask = 0;
Expand Down Expand Up @@ -1394,3 +1388,8 @@ void address_map::map_validity_check(validity_checker &valid, int spacenum) cons
valid.validate_tag(entry.m_share);
}
}

const address_space_config &address_map::get_config() const
{
return *m_config;
}
10 changes: 9 additions & 1 deletion src/emu/addrmap.h
Expand Up @@ -40,7 +40,8 @@ enum map_handler_type
AMH_DEVICE_DELEGATE_SMO,
AMH_PORT,
AMH_BANK,
AMH_DEVICE_SUBMAP
AMH_DEVICE_SUBMAP,
AMH_VIEW
};


Expand Down Expand Up @@ -184,6 +185,8 @@ class address_map_entry
address_map_entry &m(const char *tag, address_map_constructor func);
address_map_entry &m(device_t *device, address_map_constructor func);

// view initialization
void view(memory_view &mv);

// implicit base -> delegate converter
template <typename T, typename Ret, typename... Params>
Expand Down Expand Up @@ -404,6 +407,7 @@ class address_map_entry

device_t *m_submap_device;
address_map_constructor m_submap_delegate;
memory_view *m_view;

// information used during processing
void * m_memory; // pointer to memory backing this entry
Expand Down Expand Up @@ -477,6 +481,7 @@ class address_map
public:
// construction/destruction
address_map(device_t &device, int spacenum);
address_map(memory_view &view);
address_map(device_t &device, address_map_entry *entry);
address_map(const address_space &space, offs_t start, offs_t end, u64 unitmask, int cswidth, device_t &device, address_map_constructor submap_delegate);
~address_map();
Expand All @@ -492,12 +497,15 @@ class address_map
// public data
int m_spacenum; // space number of the map
device_t * m_device; // associated device
memory_view * m_view; // view, when in one
const address_space_config * m_config; // space configuration
u8 m_unmapval; // unmapped memory value
offs_t m_globalmask; // global mask
simple_list<address_map_entry> m_entrylist; // list of entries

void import_submaps(running_machine &machine, device_t &owner, int data_width, endianness_t endian, int addr_shift);
void map_validity_check(validity_checker &valid, int spacenum) const;
const address_space_config &get_config() const;
};

#endif // MAME_EMU_ADDRMAP_H
11 changes: 8 additions & 3 deletions src/emu/debug/debugcmd.cpp
Expand Up @@ -3767,10 +3767,15 @@ void debugger_commands::execute_memdump(int ref, const std::vector<std::string>
for (memory_entry &entry : entries[mode])
{
if (octal)
fprintf(file, "%0*o - %0*o", nc, entry.start, nc, entry.end);
fprintf(file, "%0*o - %0*o:", nc, entry.start, nc, entry.end);
else
fprintf(file, "%0*x - %0*x", nc, entry.start, nc, entry.end);
fprintf(file, ": %s\n", entry.entry->name().c_str());
fprintf(file, "%0*x - %0*x:", nc, entry.start, nc, entry.end);
for(const auto &c : entry.context)
if(c.disabled)
fprintf(file, " %s[off]", c.view->name().c_str());
else
fprintf(file, " %s[%d]", c.view->name().c_str(), c.slot);
fprintf(file, " %s\n", entry.entry->name().c_str());
}
fprintf(file, "\n");
}
Expand Down
1 change: 1 addition & 0 deletions src/emu/emufwd.h
Expand Up @@ -149,6 +149,7 @@ class memory_bank;
class memory_manager;
class memory_region;
class memory_share;
class memory_view;

// declared in emuopts.h
class emu_options;
Expand Down

2 comments on commit 574daf4

@cuavas
Copy link
Member

@cuavas cuavas commented on 574daf4 Nov 22, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This blows up allocating a very large temporary on the stack. The issue is this code in GNU libstdc++ (from include/c++/9.3.0/bits/stl_uninitialized.h):

  template<>
    struct __uninitialized_default_n_1<true>
    {
      template<typename _ForwardIterator, typename _Size>
        static _ForwardIterator
        __uninit_default_n(_ForwardIterator __first, _Size __n)
        {
          typedef typename iterator_traits<_ForwardIterator>::value_type
            _ValueType;

          return std::fill_n(__first, __n, _ValueType());
        }
    };

Note that it stacks a temporary _ValueType for the call to std::fill_n. It’s not calling the default constructor in-place, it’s filling with multiple copies of one default-constructed object.

This gets called when default-appending to a std::vector under certain circumstances. When the handler_entry_read_dsipatch constructor calls m_dispatch_array.resize(1) this ends up being called with a very large _ValueType and exceeds the maximum stack frame size.

@cuavas
Copy link
Member

@cuavas cuavas commented on 574daf4 Nov 22, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at it a bit more, it only uses this form if the value is trivially constructible, which is true for a std::array of pointers. It might be possible to work around.

Please sign in to comment.