From 7dd93bea57b563241f9d72695ddc29275503e3dd Mon Sep 17 00:00:00 2001 From: Catherine Date: Tue, 9 Apr 2024 15:21:02 +0000 Subject: [PATCH] Document RFC 62. This includes a few minor code changes: - Removing redundant `lib.memory.Memory.Init = hdl.MemoryData.Init` re-export; - Renaming `FrozenError` to `FrozenMemory` and moving it to `.hdl`; - Marking `ReadPort` and `WritePort` as `@final`. --- amaranth/hdl/__init__.py | 4 +-- amaranth/hdl/_mem.py | 62 ++++++++++++++++++++++++++++++++++------ amaranth/lib/memory.py | 43 ++++++++++++---------------- docs/stdlib/memory.rst | 32 +++++++++++++++++---- tests/test_lib_memory.py | 8 +++--- 5 files changed, 105 insertions(+), 44 deletions(-) diff --git a/amaranth/hdl/__init__.py b/amaranth/hdl/__init__.py index 43852da..7227775 100644 --- a/amaranth/hdl/__init__.py +++ b/amaranth/hdl/__init__.py @@ -8,7 +8,7 @@ from ._dsl import Module from ._cd import DomainError, ClockDomain from ._ir import UnusedElaboratable, Elaboratable, DriverConflict, Fragment from ._ir import Instance, IOBufferInstance -from ._mem import MemoryData, MemoryInstance, Memory, ReadPort, WritePort, DummyPort +from ._mem import FrozenMemory, MemoryData, MemoryInstance, Memory, ReadPort, WritePort, DummyPort from ._rec import Record from ._xfrm import DomainRenamer, ResetInserter, EnableInserter @@ -29,7 +29,7 @@ __all__ = [ "UnusedElaboratable", "Elaboratable", "DriverConflict", "Fragment", "Instance", "IOBufferInstance", # _mem - "MemoryData", "MemoryInstance", "Memory", "ReadPort", "WritePort", "DummyPort", + "FrozenMemory", "MemoryData", "MemoryInstance", "Memory", "ReadPort", "WritePort", "DummyPort", # _rec "Record", # _xfrm diff --git a/amaranth/hdl/_mem.py b/amaranth/hdl/_mem.py index da11270..14cc60a 100644 --- a/amaranth/hdl/_mem.py +++ b/amaranth/hdl/_mem.py @@ -9,21 +9,50 @@ from ..utils import ceil_log2 from .._utils import deprecated, final -__all__ = ["MemoryData", "Memory", "ReadPort", "WritePort", "DummyPort"] +__all__ = ["FrozenMemory", "MemoryData", "Memory", "ReadPort", "WritePort", "DummyPort"] @final -class FrozenError(Exception): - """This exception is raised when ports are added to a :class:`Memory` or its - :attr:`~Memory.init` attribute is changed after it has been elaborated once. - """ +class FrozenMemory(Exception): + """This exception is raised when a memory array is being modified after elaboration.""" @final class MemoryData: + """Abstract description of a memory array. + + A :class:`MemoryData` object describes the geometry (shape and depth) and the initial contents + of a memory array, without specifying the way in which it is accessed. It is conceptually + similar to an array of :class:`Signal`\\ s. + + The :py:`init` parameter and assignment to the :py:`init` attribute have the same effect, with + :class:`MemoryData.Init` converting elements of the iterable to match :py:`shape` and using + a default value for rows that are not explicitly initialized. + + Changing the initial contents of a :class:`MemoryData` is only possible until it is used to + elaborate a memory; afterwards, attempting to do so will raise :exc:`FrozenMemory`. + + .. warning:: + + Uninitialized memories (including ASIC memories and some FPGA memories) are + `not yet supported `_, and + the :py:`init` parameter must be always provided, if only as :py:`init=[]`. + + Parameters + ---------- + shape : :ref:`shape-like ` object + Shape of each memory row. + depth : :class:`int` + Number of memory rows. + init : iterable of initial values + Initial values for memory rows. + """ + @final class Init(MutableSequence): - """Memory initialization data. + """Init(...) + + Memory initialization data. This is a special container used only for initial contents of memories. It is similar to :class:`list`, but does not support inserting or deleting elements; its length is always @@ -72,7 +101,7 @@ class MemoryData: def __setitem__(self, index, value): if self._frozen: - raise FrozenError("Cannot set 'init' on a memory that has already been elaborated") + raise FrozenMemory("Cannot set 'init' on a memory that has already been elaborated") if isinstance(index, slice): indices = range(*index.indices(len(self._elems))) @@ -117,7 +146,7 @@ class MemoryData: def _lhs_signals(self): # This value cannot ever appear in a design. raise NotImplementedError # :nocov: - + _rhs_signals = _lhs_signals def __repr__(self): @@ -152,13 +181,28 @@ class MemoryData: @init.setter def init(self, init): if self._frozen: - raise FrozenError("Cannot set 'init' on a memory that has already been elaborated") + raise FrozenMemory("Cannot set 'init' on a memory that has already been elaborated") self._init = MemoryData.Init(init, shape=self._shape, depth=self._depth) def __repr__(self): return f"(memory-data {self.name})" def __getitem__(self, index): + """Retrieve a memory row for simulation. + + A :class:`MemoryData` object can be indexed with an :class:`int` to construct a special + value that can be used to read and write the selected memory row in a simulation testbench, + without having to create a memory port. + + .. important:: + + Even in a simulation, the value returned by this function cannot be used in a module; + it can only be used with :py:`sim.get()` and :py:`sim.set()`. + + Returns + ------- + :class:`~amaranth.hdl.Value`, :ref:`assignable ` + """ index = operator.index(index) if index not in range(self.depth): raise IndexError(f"Index {index} is out of bounds (memory has {self.depth} rows)") diff --git a/amaranth/lib/memory.py b/amaranth/lib/memory.py index e52aa56..5028df0 100644 --- a/amaranth/lib/memory.py +++ b/amaranth/lib/memory.py @@ -3,53 +3,46 @@ from collections import OrderedDict from collections.abc import MutableSequence from ..hdl import MemoryData, MemoryInstance, Shape, ShapeCastable, Const -from ..hdl._mem import FrozenError +from ..hdl._mem import FrozenMemory from ..utils import ceil_log2 from .._utils import final from .. import tracer from . import wiring, data -__all__ = ["Memory", "ReadPort", "WritePort"] +__all__ = ["FrozenMemory", "Memory", "ReadPort", "WritePort"] class Memory(wiring.Component): """Addressable array of rows. This :ref:`component ` is used to construct a memory array by first specifying its - dimensions and initial contents using the :py:`shape`, :py:`depth`, and :py:`init` parameters, - and then adding memory ports using the :meth:`read_port` and :meth:`write_port` methods. - Because it is mutable, it should be created and used locally within - the :ref:`elaborate ` method. It is an error to add ports to or change - initial contents of a memory after it has been elaborated. + dimensions and initial contents using the :class:`~amaranth.hdl.MemoryData` object and + the :py:`data` parameter (or by providing :py:`shape`, :py:`depth`, and :py:`init` parameters + directly instead) and then adding memory ports using the :meth:`read_port` and + :meth:`write_port` methods. Because it is mutable, it should be created and used locally within + the :ref:`elaborate ` method. - The :py:`init` parameter and assignment to the :py:`init` attribute have the same effect, with - :class:`Memory.Init` converting elements of the iterable to match :py:`shape` and using - a default value for rows that are not explicitly initialized. + Adding ports or changing initial contents of a :class:`Memory` is only possible until it is + elaborated; afterwards, attempting to do so will raise :class:`~amaranth.hdl.FrozenMemory`. - .. warning:: - - Uninitialized memories (including ASIC memories and some FPGA memories) are - `not yet supported `_, and - the :py:`init` parameter must be always provided, if only as :py:`init=[]`. + Platform overrides + ------------------ + Define the :py:`get_memory()` platform method to override the implementation of + :class:`Memory`, e.g. to instantiate library cells directly. Parameters ---------- + data : :class:`~amaranth.hdl.MemoryData` + Representation of memory geometry and contents. shape : :ref:`shape-like ` object Shape of each memory row. depth : :class:`int` Number of memory rows. init : iterable of initial values Initial values for memory rows. - - Platform overrides - ------------------ - Define the :py:`get_memory()` platform method to override the implementation of - :class:`Memory`, e.g. to instantiate library cells directly. """ - Init = MemoryData.Init - def __init__(self, data=None, *, shape=None, depth=None, init=None, attrs=None, src_loc_at=0): if data is None: if shape is None: @@ -128,7 +121,7 @@ class Memory(wiring.Component): :class:`ReadPort` """ if self._frozen: - raise FrozenError("Cannot add a memory port to a memory that has already been elaborated") + raise FrozenMemory("Cannot add a memory port to a memory that has already been elaborated") signature = ReadPort.Signature(shape=self.shape, addr_width=ceil_log2(self.depth)) return ReadPort(signature, memory=self, domain=domain, transparent_for=transparent_for, src_loc_at=1 + src_loc_at) @@ -153,7 +146,7 @@ class Memory(wiring.Component): :class:`WritePort` """ if self._frozen: - raise FrozenError("Cannot add a memory port to a memory that has already been elaborated") + raise FrozenMemory("Cannot add a memory port to a memory that has already been elaborated") signature = WritePort.Signature( shape=self.shape, addr_width=ceil_log2(self.depth), granularity=granularity) return WritePort(signature, memory=self, domain=domain, @@ -195,6 +188,7 @@ class Memory(wiring.Component): return instance +@final class ReadPort: """A read memory port. @@ -318,6 +312,7 @@ class ReadPort: return self._transparent_for +@final class WritePort: """A write memory port. diff --git a/docs/stdlib/memory.rst b/docs/stdlib/memory.rst index f5f6ad9..ad2dc42 100644 --- a/docs/stdlib/memory.rst +++ b/docs/stdlib/memory.rst @@ -170,17 +170,39 @@ However, the memory read port is also configured to be *transparent* relative to } -Memories -======== +Simulation +========== + +.. todo:: + + This section will be written once the simulator itself is documented. + + +Memory description +================== + +.. autoexception:: amaranth.hdl.FrozenMemory + +.. autoclass:: amaranth.hdl.MemoryData + + +Memory component +================ .. attributes are not documented because they can be easily used to break soundness and we don't document them for signals either; they are rarely necessary for interoperability -.. autoclass:: Memory(*, depth, shape, init, src_loc_at=0) - :no-members: +.. + the following two directives document a pair of overloads of :class:`Memory`; this is a little + weird and not really how rst/sphinx are supposed to work but it results in a comprehensible + generated document. be careful to not break this! - .. autoclass:: amaranth.lib.memory::Memory.Init(...) +.. class:: Memory(data, *, src_loc_at=0) + +.. autoclass:: Memory(*, shape, depth, init, src_loc_at=0) + :noindex: + :no-members: .. automethod:: read_port diff --git a/tests/test_lib_memory.py b/tests/test_lib_memory.py index 69946c9..b2b7bde 100644 --- a/tests/test_lib_memory.py +++ b/tests/test_lib_memory.py @@ -439,15 +439,15 @@ class MemoryTestCase(FHDLTestCase): m = memory.Memory(shape=unsigned(8), depth=4, init=[]) m.write_port() m.elaborate(None) - with self.assertRaisesRegex(memory.FrozenError, + with self.assertRaisesRegex(memory.FrozenMemory, r"^Cannot add a memory port to a memory that has already been elaborated$"): m.write_port() - with self.assertRaisesRegex(memory.FrozenError, + with self.assertRaisesRegex(memory.FrozenMemory, r"^Cannot add a memory port to a memory that has already been elaborated$"): m.read_port() - with self.assertRaisesRegex(memory.FrozenError, + with self.assertRaisesRegex(memory.FrozenMemory, r"^Cannot set 'init' on a memory that has already been elaborated$"): m.init = [1, 2, 3, 4] - with self.assertRaisesRegex(memory.FrozenError, + with self.assertRaisesRegex(memory.FrozenMemory, r"^Cannot set 'init' on a memory that has already been elaborated$"): m.init[0] = 1