major refactor of memory (removes _MemBase class entirely)

Author: timsherwood

docs/index.rst

@@ -90,10 +90,6 @@ overloaded operations such as `addition` or `bitwise or`. A bunch of other relat
     * :py:class:`~pyrtl.wire.Const` (WireVector)
     * :py:class:`~pyrtl.wire.Register` (WireVector)
 
-* Memory blocks [base class for internal use only]
-    * :py:class:`~pyrtl.memory.MemBlock` (_MemReadBase)
-    * :py:class:`~pyrtl.memory.RomBlock` (_MemReadBase)
-
 After specifying a hardware design, there are then options to simulate your design right in PyRTL,
 synthesize it down to primitive 1-bit operations, optimize it, and export it to Verilog (along with
 a testbench),.

pyrtl/analysis/estimate.py

@@ -122,13 +122,8 @@ def _bits_ports_and_isrom_from_memory(mem):
     is_rom = False
     bits = 2**mem.addrwidth * mem.bitwidth
     read_ports = len(mem.readport_nets)
-    try:
-        write_ports = len(mem.writeport_nets)
-    except AttributeError:  # dealing with ROMs
-        if not isinstance(mem, RomBlock):
-            raise PyrtlInternalError('Mem with no writeport_nets attribute'
-                                     ' but not a ROM? Thats an error')
-        write_ports = 0
+    write_ports = len(mem.writeport_nets)
+    if isinstance(mem, RomBlock):
         is_rom = True
     ports = max(read_ports, write_ports)
     return bits, ports, is_rom

pyrtl/compilesim.py

@@ -705,7 +705,7 @@ def _create_code(self, write):
         self._declare_mem_helpers(write)
         roms = {mem for mem in mems if isinstance(mem, RomBlock)}
         self._declare_roms(write, roms)
-        mems = {mem for mem in mems if isinstance(mem, MemBlock)}
+        mems = {mem for mem in mems if isinstance(mem, MemBlock) and not isinstance(mem, RomBlock)}
         self._declare_mems(write, mems)
 
         # single step function

pyrtl/core.py

@@ -617,16 +617,16 @@ def sanity_check_wirevector(self, w):
 
     def sanity_check_memblock(self, m):
         """ Check that m is a valid memblock type. """
-        from .memory import _MemReadBase
-        if not isinstance(m, _MemReadBase):
+        from .memory import MemBlock
+        if not isinstance(m, MemBlock):
             raise PyrtlError(
                 'error attempting to pass an input of type "%s" '
-                'instead of _MemReadBase' % type(m))
+                'instead of MemBlock' % type(m))
 
     def sanity_check_net(self, net):
         """ Check that net is a valid LogicNet. """
         from .wire import Input, Output, Const, Register
-        from .memory import _MemReadBase
+        from .memory import MemBlock
 
         # general sanity checks that apply to all operations
         if not isinstance(net, LogicNet):
@@ -697,7 +697,7 @@ def sanity_check_net(self, net):
                 raise PyrtlInternalError('error, mem op requires 2 op_params in tuple')
             if not isinstance(net.op_param[0], int):
                 raise PyrtlInternalError('error, mem op requires first operand as int')
-            if not isinstance(net.op_param[1], _MemReadBase):
+            if not isinstance(net.op_param[1], MemBlock):
                 raise PyrtlInternalError('error, mem op requires second operand of a memory type')
 
         # operation-specific checks on destinations

pyrtl/memory.py

@@ -87,68 +87,7 @@ def name(self, n):
         as_wires(self).name = n
 
 
-class _MemReadBase(object):
-    """ This is the base class for the memories and ROM blocks and
-    it implements the read and initialization operations needed for
-    both of them.
-    """
-
-    # FIXME: right now read port is built unconditionally (no read enable)
-
-    def __init__(self, bitwidth, addrwidth, name, max_read_ports, asynchronous, block):
-        self.max_read_ports = max_read_ports
-        self.read_ports = 0
-        self.block = working_block(block)
-        name = next_tempvar_name(name)
-
-        if bitwidth <= 0:
-            raise PyrtlError('bitwidth must be >= 1')
-        if addrwidth <= 0:
-            raise PyrtlError('addrwidth must be >= 1')
-
-        self.bitwidth = bitwidth
-        self.name = name
-        self.addrwidth = addrwidth
-        self.readport_nets = []
-        self.id = _memIndex.next_index()
-        self.asynchronous = asynchronous
-        self.block._add_memblock(self)
-
-    def __getitem__(self, item):
-        """ Builds circuitry to retrieve an item from the memory """
-        item = as_wires(item, bitwidth=self.addrwidth, truncating=False)
-        if len(item) > self.addrwidth:
-            raise PyrtlError('memory index bitwidth > addrwidth')
-        return _MemIndexed(mem=self, index=item)
-
-    def _readaccess(self, addr):
-        # FIXME: add conditional read ports
-        return self._build_read_port(addr)
-
-    def _build_read_port(self, addr):
-        if self.max_read_ports is not None:
-            self.read_ports += 1
-            if self.read_ports > self.max_read_ports:
-                raise PyrtlError('maximum number of read ports (%d) exceeded' % self.max_read_ports)
-        data = WireVector(bitwidth=self.bitwidth)
-        readport_net = LogicNet(
-            op='m',
-            op_param=(self.id, self),
-            args=(addr,),
-            dests=(data,))
-        working_block().add_net(readport_net)
-        self.readport_nets.append(readport_net)
-        return data
-
-    def __setitem__(self, key, value):
-        """ Not legal on a object that only allows for reads"""
-        raise PyrtlError("error, invalid call __setitem__ made on _MemReadBase")
-
-    def _make_copy(self, block):
-        pass
-
-
-class MemBlock(_MemReadBase):
+class MemBlock(object):
     """ MemBlock is the object for specifying block memories.  It can be
     indexed like an array for both reading and writing.  Writes under a conditional
     are automatically converted to enabled writes.   For example, consider the following
@@ -205,19 +144,61 @@ def __init__(self, bitwidth, addrwidth, name='', max_read_ports=2, max_write_por
         that memories with high numbers of ports may not be possible to map to physical memories
         such as block rams or existing memory hardware macros.
         """
-        super(MemBlock, self).__init__(bitwidth, addrwidth, name, max_read_ports,
-                                       asynchronous, block)
+        self.max_read_ports = max_read_ports
+        self.num_read_ports = 0
+        self.block = working_block(block)
+        name = next_tempvar_name(name)
+
+        if bitwidth <= 0:
+            raise PyrtlError('bitwidth must be >= 1')
+        if addrwidth <= 0:
+            raise PyrtlError('addrwidth must be >= 1')
+
+        self.bitwidth = bitwidth
+        self.name = name
+        self.addrwidth = addrwidth
+        self.readport_nets = []
+        self.id = _memIndex.next_index()
+        self.asynchronous = asynchronous
+        self.block._add_memblock(self)
+
         self.max_write_ports = max_write_ports
-        self.write_ports = 0
+        self.num_write_ports = 0
         self.writeport_nets = []
 
+    def __getitem__(self, item):
+        """ Builds circuitry to retrieve an item from the memory """
+        item = as_wires(item, bitwidth=self.addrwidth, truncating=False)
+        if len(item) > self.addrwidth:
+            raise PyrtlError('memory index bitwidth > addrwidth')
+        return _MemIndexed(mem=self, index=item)
+
     def __setitem__(self, item, assignment):
         """ Builds circuitry to set an item in the memory """
         if isinstance(assignment, _MemAssignment):
             self._assignment(item, assignment.rhs, is_conditional=assignment.is_conditional)
         else:
             raise PyrtlError('error, assigment to memories should use "<<=" not "=" operator')
 
+    def _readaccess(self, addr):
+        # FIXME: add conditional read ports
+        return self._build_read_port(addr)
+
+    def _build_read_port(self, addr):
+        if self.max_read_ports is not None:
+            self.num_read_ports += 1
+            if self.num_read_ports > self.max_read_ports:
+                raise PyrtlError('maximum number of read ports (%d) exceeded' % self.max_read_ports)
+        data = WireVector(bitwidth=self.bitwidth)
+        readport_net = LogicNet(
+            op='m',
+            op_param=(self.id, self),
+            args=(addr,),
+            dests=(data,))
+        working_block().add_net(readport_net)
+        self.readport_nets.append(readport_net)
+        return data
+
     def _assignment(self, item, val, is_conditional):
         from .conditional import _build
 
@@ -246,8 +227,8 @@ def _assignment(self, item, val, is_conditional):
     def _build(self, addr, data, enable):
         """ Builds a write port. """
         if self.max_write_ports is not None:
-            self.write_ports += 1
-            if self.write_ports > self.max_write_ports:
+            self.num_write_ports += 1
+            if self.num_write_ports > self.max_write_ports:
                 raise PyrtlError('maximum number of write ports (%d) exceeded' %
                                  self.max_write_ports)
         writeport_net = LogicNet(
@@ -269,7 +250,7 @@ def _make_copy(self, block=None):
                         block=block)
 
 
-class RomBlock(_MemReadBase):
+class RomBlock(MemBlock):
     """ PyRTL Read Only Memory.
 
     RomBlocks are the read only memory block for PyRTL.  They support the same read interface
@@ -302,8 +283,9 @@ def __init__(self, bitwidth, addrwidth, romdata, name='', max_read_ports=2,
         :param block: The block to add to, defaults to the working block
         """
 
-        super(RomBlock, self).__init__(bitwidth, addrwidth, name, max_read_ports,
-                                       asynchronous, block)
+        super(RomBlock, self).__init__(bitwidth=bitwidth, addrwidth=addrwidth, name=name,
+                                       max_read_ports=max_read_ports, max_write_ports=0,
+                                       asynchronous=asynchronous, block=block)
         self.data = romdata
         self.build_new_roms = build_new_roms
         self.current_copy = self
@@ -317,6 +299,9 @@ def __getitem__(self, item):
             # If you really know what you are doing, use a Const WireVector instead.
         return super(RomBlock, self).__getitem__(item)
 
+    def __setitem__(self, item, assignment):
+        raise PyrtlError('no writing to a read-only memory')
+
     def _get_read_data(self, address):
         import types
         try:
@@ -361,7 +346,7 @@ def _get_read_data(self, address):
 
     def _build_read_port(self, addr):
         if self.build_new_roms and \
-                (self.current_copy.read_ports >= self.current_copy.max_read_ports):
+                (self.current_copy.num_read_ports >= self.current_copy.max_read_ports):
             self.current_copy = self._make_copy()
         return super(RomBlock, self.current_copy)._build_read_port(addr)
 

tests/test_memblock.py

@@ -129,7 +129,7 @@ def test_read_memindexed_ilshift(self):
             })
             self.assertEqual(sim.inspect(y), 5 - i)
             self.assertEqual(sim.inspect(z), 5 - i)
-        self.assertEqual(self.mem.read_ports, 1)
+        self.assertEqual(self.mem.num_read_ports, 1)
 
     def test_write_memindexed_ilshift(self):
         self.mem1 = pyrtl.MemBlock(8, 8)
@@ -151,8 +151,8 @@ def test_write_memindexed_ilshift(self):
                 inp: 5 - i
             })
             self.assertEqual(sim.inspect(out), 0 if i == 0 else 5 - (i - 1))
-        self.assertEqual(self.mem1.read_ports, 1)  # 2 b/c of the output read
-        self.assertEqual(self.mem2.write_ports, 1)
+        self.assertEqual(self.mem1.num_read_ports, 1)  # 2 b/c of the output read
+        self.assertEqual(self.mem2.num_write_ports, 1)
 
     def test_read_memindexed_ior(self):
         self.mem = pyrtl.MemBlock(8, 8)
@@ -189,7 +189,7 @@ def test_read_memindexed_ior(self):
             self.assertEqual(sim.inspect(y), y_exp)
             self.assertEqual(sim.inspect(z), z_exp)
             self.assertEqual(sim.inspect(w), w_exp)
-        self.assertEqual(self.mem.read_ports, 1)
+        self.assertEqual(self.mem.num_read_ports, 1)
 
     def test_write_memindexed_ior(self):
         self.mem1 = pyrtl.MemBlock(8, 8)
@@ -225,8 +225,8 @@ def test_write_memindexed_ior(self):
             else:
                 out_exp = 2
             self.assertEqual(sim.inspect(out), out_exp)
-        self.assertEqual(self.mem1.read_ports, 1)
-        self.assertEqual(self.mem2.write_ports, 1)
+        self.assertEqual(self.mem1.num_read_ports, 1)
+        self.assertEqual(self.mem2.num_write_ports, 1)
 
 
 class RTLRomBlockWiring(unittest.TestCase):