memory: split the names of MemoryMap resources into multiple parts.

Author: jfng

amaranth_soc/csr/bus.py

@@ -569,7 +569,7 @@ def _check_memory_map(self, memory_map):
                     reg.signature.members["element"].flow == Out and
                     reg.signature.members["element"].is_signature and
                     isinstance(reg.signature.members["element"].signature, Element.Signature)):
-                raise AttributeError(f"Signature of CSR register '{reg_name}' must have a "
+                raise AttributeError(f"Signature of CSR register {reg_name} must have a "
                                      f"csr.Element.Signature member named 'element' and oriented "
                                      f"as wiring.Out")
 

amaranth_soc/csr/event.py

@@ -66,8 +66,8 @@ def __init__(self, event_map, *, trigger="level", data_width, alignment=0, name=
         reg_size   = (event_map.size + data_width - 1) // data_width
         addr_width = 1 + max(ceil_log2(reg_size), alignment)
         memory_map = MemoryMap(addr_width=addr_width, data_width=data_width, alignment=alignment)
-        memory_map.add_resource(self._enable,  size=reg_size, name="enable")
-        memory_map.add_resource(self._pending, size=reg_size, name="pending")
+        memory_map.add_resource(self._enable,  size=reg_size, name=("enable",))
+        memory_map.add_resource(self._pending, size=reg_size, name=("pending",))
 
         self._mux = Multiplexer(memory_map)
 

amaranth_soc/csr/reg.py

@@ -884,7 +884,7 @@ def traverse_path(path, obj):
 
             reg_size = (register.element.width + data_width - 1) // data_width
             reg_name = "__".join(path)
-            memory_map.add_resource(register, name=reg_name, size=reg_size, addr=reg_addr,
+            memory_map.add_resource(register, name=(reg_name,), size=reg_size, addr=reg_addr,
                                     alignment=reg_alignment)
 
         self._map = register_map

amaranth_soc/memory.py

@@ -1,7 +1,6 @@
 import bisect
 
 from amaranth.lib import wiring
-from amaranth.utils import bits_for
 
 
 __all__ = ["ResourceInfo", "MemoryMap"]
@@ -48,6 +47,60 @@ def items(self):
             yield key, self._values[key]
 
 
+class _Namespace:
+    """Namespace.
+
+    A set of path-like names that identify locations in a hierarchical structure.
+    """
+    def __init__(self):
+        self._assignments = dict()
+
+    def is_available(self, *names, reasons=None):
+        for name in names:
+            assert name and isinstance(name, tuple)
+            assert all(part and isinstance(part, str) for part in name)
+
+        conflicts = False
+        for name_idx, name in enumerate(names):
+            # Also check for conflicts between the queried names.
+            reserved_names = sorted(self._assignments.keys() | set(names[name_idx + 1:]))
+
+            for reserved_name in reserved_names:
+                # A conflict happens in the following cases:
+                #  - `name` is equal to `reserved_name`
+                #  - `name` is a prefix of `reserved_name`
+                #  - `reserved_name` is a prefix of `name`.
+                for part_idx, part in enumerate(name):
+                    if part != reserved_name[part_idx]:
+                        # Available. `name` and `reserved_name` are not equal and are not prefixes
+                        # of each other.
+                        break
+                    if part_idx == min(len(name), len(reserved_name)) - 1:
+                        # `name` and `reserved_name` are equal so far, but we have reached the end
+                        # of at least one of them. In either case, they are in conflict.
+                        conflicts = True
+                        # Conflicts between the queried names should never happen.
+                        assert reserved_name in self._assignments
+                        if reasons is not None:
+                            reasons.append(f"{name} conflicts with local name {reserved_name} "
+                                           f"assigned to {self._assignments[reserved_name]!r}")
+                        break
+
+        return not conflicts
+
+    def assign(self, name, obj):
+        assert self.is_available(name)
+        self._assignments[name] = obj
+
+    def extend(self, other):
+        assert isinstance(other, _Namespace)
+        assert self.is_available(*other.names())
+        self._assignments.update(other._assignments)
+
+    def names(self):
+        yield from self._assignments.keys()
+
+
 class ResourceInfo:
     """Resource metadata.
 
@@ -57,7 +110,7 @@ class ResourceInfo:
     ----------
     resource : :class:`wiring.Component`
         A resource located in the memory map. See :meth:`MemoryMap.add_resource` for details.
-    path : iter(str)
+    path : :class:`tuple` of (:class:`str` or (:class:`tuple` of :class:`str`))
         Path of the resource. It is composed of the names of each window sitting between
         the resource and the memory map from which this :class:`ResourceInfo` was obtained.
         See :meth:`MemoryMap.add_window` for details.
@@ -71,9 +124,12 @@ class ResourceInfo:
         is located behind a window that uses sparse addressing.
     """
     def __init__(self, resource, path, start, end, width):
-        if not path or not all(isinstance(part, str) and part for part in path):
-            raise TypeError("Path must be a non-empty sequence of non-empty strings, not {!r}"
-                            .format(path))
+        flattened_path = []
+        for name in path:
+            flattened_path += name if name and isinstance(name, tuple) else [name]
+        if not (path and isinstance(path, tuple) and
+                all(name and isinstance(name, str) for name in flattened_path)):
+            raise TypeError(f"Path must be a non-empty tuple of non-empty strings, not {path!r}")
         if not isinstance(start, int) or start < 0:
             raise TypeError("Start address must be a non-negative integer, not {!r}"
                             .format(start))
@@ -162,7 +218,7 @@ def __init__(self, *, addr_width, data_width, alignment=0, name=None):
         self._ranges     = _RangeMap()
         self._resources  = dict()
         self._windows    = dict()
-        self._namespace  = dict()
+        self._namespace  = _Namespace()
 
         self._next_addr  = 0
         self._frozen     = False
@@ -266,8 +322,8 @@ def add_resource(self, resource, *, name, size, addr=None, alignment=None):
         ---------
         resource : :class:`wiring.Component`
             The resource to be added.
-        name : str
-            Name of the resource. It must not collide with the name of other resources or windows
+        name : :class:`tuple` of (:class:`str`)
+            Name of the resource. It must not conflict with the name of other resources or windows
             present in this memory map.
         addr : int
             Address of the resource. Optional. If ``None``, the implicit next address will be used.
@@ -295,7 +351,8 @@ def add_resource(self, resource, *, name, size, addr=None, alignment=None):
         :exc:`ValueError`
             If the resource has already been added to this memory map.
         :exc:`ValueError`
-            If the name of the resource is already present in the namespace of this memory map.
+            If the resource name conflicts with the name of other resources or windows present in
+            this memory map.
         """
         if self._frozen:
             raise ValueError("Memory map has been frozen. Cannot add resource {!r}"
@@ -309,10 +366,17 @@ def add_resource(self, resource, *, name, size, addr=None, alignment=None):
             raise ValueError("Resource {!r} is already added at address range {:#x}..{:#x}"
                              .format(resource, addr_range.start, addr_range.stop))
 
-        if not isinstance(name, str) or not name:
-            raise TypeError("Name must be a non-empty string, not {!r}".format(name))
-        if name in self._namespace:
-            raise ValueError("Name {} is already used by {!r}".format(name, self._namespace[name]))
+        if not (name and isinstance(name, tuple) and
+                all(part and isinstance(part, str) for part in name)):
+            raise TypeError(f"Resource name must be a non-empty tuple of non-empty strings, not "
+                            f"{name!r}")
+
+        reasons = []
+        if not self._namespace.is_available(name, reasons=reasons):
+            reasons_as_string = "".join(f"\n- {reason}" for reason in reasons)
+            raise ValueError(f"Resource {resource!r} cannot be added to the local namespace:" +
+                             reasons_as_string)
+        del reasons
 
         if alignment is not None:
             if not isinstance(alignment, int) or alignment < 0:
@@ -325,7 +389,7 @@ def add_resource(self, resource, *, name, size, addr=None, alignment=None):
         addr_range = self._compute_addr_range(addr, size, alignment=alignment)
         self._ranges.insert(addr_range, resource)
         self._resources[id(resource)] = resource, name, addr_range
-        self._namespace[name] = resource
+        self._namespace.assign(name, resource)
         self._next_addr = addr_range.stop
         return addr_range.start, addr_range.stop
 
@@ -382,17 +446,24 @@ def add_window(self, window, *, addr=None, sparse=None):
 
         Exceptions
         ----------
-        Raises :exn:`ValueError` if one of the following occurs:
-
-        - this memory map is frozen;
-        - the requested address and size, after alignment, would overlap with any resources or
-        windows that have already been added, or would be out of bounds;
-        - the added memory map has a wider datapath than this memory map;
-        - dense address translation is used and the datapath width of this memory map is not an
-        integer multiple of the datapath of the added memory map;
-        - the name of the added memory map is already present in the namespace of this memory map;
-        - the added memory map has no name, and the name of one of its subordinate resources or
-        windows is already present in the namespace of this memory map;
+        :exc:`ValueError`
+            If the memory map is frozen.
+        :exc:`TypeError`
+            If the added memory map is not a :class:`MemoryMap`.
+        :exc:`ValueError`
+            If the requested address and size, after alignment, would overlap with any resources or
+            windows that have already been added, or would be out of bounds.
+        :exc:`ValueError`
+            If the added memory map has a wider datapath than this memory map.
+        :exc:`ValueError`
+            If dense address translation is used and the datapath width of this memory map is not
+            an integer multiple of the datapath of the added memory map.
+        :exc:`ValueError`
+            If the name of the added memory map conflicts with the name of other resources or
+            windows present in this memory map.
+        :exc:`ValueError`
+            If the added memory map has no name, and the name of one of its resources or windows
+            conflicts with the name of others present in this memory map.
         """
         if not isinstance(window, MemoryMap):
             raise TypeError("Window must be a MemoryMap, not {!r}"
@@ -423,17 +494,13 @@ def add_window(self, window, *, addr=None, sparse=None):
                                  "data width {}"
                                  .format(self.data_width, window.data_width))
 
-        if window.name is None:
-            name_conflicts = sorted(self._namespace.keys() & window._namespace.keys())
-            if name_conflicts:
-                name_conflict_descrs = ["{} is used by {!r}".format(name, self._namespace[name])
-                                        for name in name_conflicts]
-                raise ValueError("The following names are already used: {}"
-                                 .format("; ".join(name_conflict_descrs)))
-        else:
-            if window.name in self._namespace:
-                raise ValueError("Name {} is already used by {!r}"
-                                 .format(window.name, self._namespace[window.name]))
+        queries = window._namespace.names() if window.name is None else ((window.name,),)
+        reasons = []
+        if not self._namespace.is_available(*queries, reasons=reasons):
+            reasons_as_string = "".join(f"\n- {reason}" for reason in reasons)
+            raise ValueError(f"Window {window!r} cannot be added to the local namespace:" +
+                             reasons_as_string)
+        del queries, reasons
 
         if not sparse:
             ratio = self.data_width // window.data_width
@@ -452,9 +519,9 @@ def add_window(self, window, *, addr=None, sparse=None):
         self._ranges.insert(addr_range, window)
         self._windows[id(window)] = window, addr_range
         if window.name is None:
-            self._namespace.update(window._namespace)
+            self._namespace.extend(window._namespace)
         else:
-            self._namespace[window.name] = window
+            self._namespace.assign((window.name,), window)
         self._next_addr = addr_range.stop
         return addr_range.start, addr_range.stop, addr_range.step
 
@@ -587,3 +654,6 @@ def decode_address(self, address):
             return assignment.decode_address((address - addr_range.start) // addr_range.step)
         else:
             assert False # :nocov:
+
+    def __repr__(self):
+        return f"MemoryMap(name={self.name!r})"

tests/test_csr_bus.py

@@ -187,8 +187,8 @@ def test_memory_map(self):
         reg_8_rw = _MockRegister(8, "rw")
 
         memory_map = MemoryMap(addr_width=2, data_width=4)
-        memory_map.add_resource(reg_4_rw, name="reg_4_rw", size=1)
-        memory_map.add_resource(reg_8_rw, name="reg_8_rw", size=2)
+        memory_map.add_resource(reg_4_rw, name=("reg_4_rw",), size=1)
+        memory_map.add_resource(reg_8_rw, name=("reg_8_rw",), size=2)
         memory_map.freeze()
 
         dut = csr.Multiplexer(memory_map)
@@ -207,30 +207,31 @@ class _Reg(wiring.Component):
             pass
         # wrong name
         map_0 = MemoryMap(addr_width=1, data_width=8)
-        map_0.add_resource(_Reg({"foo": Out(csr.Element.Signature(8, "rw"))}), name="a", size=1)
+        map_0.add_resource(_Reg({"foo": Out(csr.Element.Signature(8, "rw"))}), name=("a",), size=1)
         with self.assertRaisesRegex(AttributeError,
-                r"Signature of CSR register 'a' must have a csr\.Element\.Signature member "
+                r"Signature of CSR register \('a',\) must have a csr\.Element\.Signature member "
                 r"named 'element' and oriented as wiring\.Out"):
             csr.Multiplexer(map_0)
         # wrong direction
         map_1 = MemoryMap(addr_width=1, data_width=8)
-        map_1.add_resource(_Reg({"element": In(csr.Element.Signature(8, "rw"))}), name="a", size=1)
+        map_1.add_resource(_Reg({"element": In(csr.Element.Signature(8, "rw"))}), name=("a",),
+                           size=1)
         with self.assertRaisesRegex(AttributeError,
-                r"Signature of CSR register 'a' must have a csr\.Element\.Signature member "
+                r"Signature of CSR register \('a',\) must have a csr\.Element\.Signature member "
                 r"named 'element' and oriented as wiring\.Out"):
             csr.Multiplexer(map_1)
         # wrong member type
         map_2 = MemoryMap(addr_width=1, data_width=8)
-        map_2.add_resource(_Reg({"element": Out(unsigned(8))}), name="a", size=1)
+        map_2.add_resource(_Reg({"element": Out(unsigned(8))}), name=("a",), size=1)
         with self.assertRaisesRegex(AttributeError,
-                r"Signature of CSR register 'a' must have a csr\.Element\.Signature member "
+                r"Signature of CSR register \('a',\) must have a csr\.Element\.Signature member "
                 r"named 'element' and oriented as wiring\.Out"):
             csr.Multiplexer(map_2)
         # wrong member signature
         map_3 = MemoryMap(addr_width=1, data_width=8)
-        map_3.add_resource(_Reg({"element": Out(wiring.Signature({}))}), name="a", size=1)
+        map_3.add_resource(_Reg({"element": Out(wiring.Signature({}))}), name=("a",), size=1)
         with self.assertRaisesRegex(AttributeError,
-                r"Signature of CSR register 'a' must have a csr\.Element\.Signature member "
+                r"Signature of CSR register \('a',\) must have a csr\.Element\.Signature member "
                 r"named 'element' and oriented as wiring\.Out"):
             csr.Multiplexer(map_3)
 
@@ -249,9 +250,9 @@ def test_sim(self):
                 reg_16_rw = _MockRegister(16, "rw")
 
                 memory_map = MemoryMap(addr_width=16, data_width=8)
-                memory_map.add_resource(reg_4_r,   name="reg_4_r",   size=1)
-                memory_map.add_resource(reg_8_w,   name="reg_8_w",   size=1)
-                memory_map.add_resource(reg_16_rw, name="reg_16_rw", size=2)
+                memory_map.add_resource(reg_4_r,   name=("reg_4_r",),   size=1)
+                memory_map.add_resource(reg_8_w,   name=("reg_8_w",),   size=1)
+                memory_map.add_resource(reg_16_rw, name=("reg_16_rw",), size=2)
 
                 dut = csr.Multiplexer(memory_map, shadow_overlaps=shadow_overlaps)
                 bus = dut.bus
@@ -336,7 +337,7 @@ def test_sim(self):
             with self.subTest(shadow_overlaps=shadow_overlaps):
                 reg_20_rw = _MockRegister(20, "rw")
                 memory_map = MemoryMap(addr_width=16, data_width=8, alignment=2)
-                memory_map.add_resource(reg_20_rw, name="reg_20_rw", size=3)
+                memory_map.add_resource(reg_20_rw, name=("reg_20_rw",), size=3)
 
                 dut = csr.Multiplexer(memory_map, shadow_overlaps=shadow_overlaps)
                 bus = dut.bus
@@ -411,10 +412,10 @@ def test_sim(self):
         reg_2 = _MockRegister(8, "rw")
 
         memory_map_1 = MemoryMap(addr_width=10, data_width=8)
-        memory_map_1.add_resource(reg_1, name="reg_1", size=1)
+        memory_map_1.add_resource(reg_1, name=("reg_1",), size=1)
 
         memory_map_2 = MemoryMap(addr_width=10, data_width=8)
-        memory_map_2.add_resource(reg_2, name="reg_2", size=1, addr=2)
+        memory_map_2.add_resource(reg_2, name=("reg_2",), size=1, addr=2)
 
         mux_1 = csr.Multiplexer(memory_map_1)
         mux_2 = csr.Multiplexer(memory_map_2)