hdl.ir: flatten hierarchy based on memory accesses, too.

nmigen/hdl/ir.py

@@ -105,51 +105,81 @@ class Fragment:
                 break
         assert found
 
-    def _resolve_driver_conflicts(self, hierarchy=("top",), mode="warn"):
+    def _resolve_hierarchy_conflicts(self, hierarchy=("top",), mode="warn"):
         assert mode in ("silent", "warn", "error")
 
         driver_subfrags = SignalDict()
+        memory_subfrags = OrderedDict()
+        def add_subfrag(registry, entity, entry):
+            if entity not in registry:
+                registry[entity] = set()
+            registry[entity].add(entry)
 
         # For each signal driven by this fragment and/or its subfragments, determine which
         # subfragments also drive it.
         for domain, signal in self.iter_drivers():
-            if signal not in driver_subfrags:
+            add_subfrag(driver_subfrags, signal, (None, hierarchy))
-                driver_subfrags[signal] = set()
-            driver_subfrags[signal].add((None, hierarchy))
 
         for i, (subfrag, name) in enumerate(self.subfragments):
-            # Never flatten instances.
             if isinstance(subfrag, Instance):
+                # For memories (which are subfragments, but semantically a part of superfragment),
+                # record that this fragment is driving it.
+                if subfrag.type in ("$memrd", "$memwr"):
+                    memory = subfrag.parameters["MEMID"]
+                    add_subfrag(memory_subfrags, memory, (None, hierarchy))
+
+                # Never flatten instances.
                 continue
 
             # First, recurse into subfragments and let them detect driver conflicts as well.
             if name is None:
                 name = "<unnamed #{}>".format(i)
             subfrag_hierarchy = hierarchy + (name,)
-            subfrag_drivers = subfrag._resolve_driver_conflicts(subfrag_hierarchy, mode)
+            subfrag_drivers, subfrag_memories = \
+                subfrag._resolve_hierarchy_conflicts(subfrag_hierarchy, mode)
 
-            # Second, classify subfragments by domains they define.
+            # Second, classify subfragments by signals they drive and memories they use.
             for signal in subfrag_drivers:
-                if signal not in driver_subfrags:
+                add_subfrag(driver_subfrags, signal, (subfrag, subfrag_hierarchy))
-                    driver_subfrags[signal] = set()
+            for memory in subfrag_memories:
-                driver_subfrags[signal].add((subfrag, subfrag_hierarchy))
+                add_subfrag(memory_subfrags, memory, (subfrag, subfrag_hierarchy))
 
         # Find out the set of subfragments that needs to be flattened into this fragment
         # to resolve driver-driver conflicts.
         flatten_subfrags = set()
-        for signal, subfrags in driver_subfrags.items():
+        def flatten_subfrags_if_needed(subfrags):
-            if len(subfrags) > 1:
+            if len(subfrags) == 1:
-                flatten_subfrags.update((f, h) for f, h in subfrags if f is not None)
+                return []
+            flatten_subfrags.update((f, h) for f, h in subfrags if f is not None)
+            return list(sorted(".".join(h) for f, h in subfrags))
 
-                # While we're at it, show a message.
+        for signal, subfrags in driver_subfrags.items():
-                subfrag_names = ", ".join(sorted(".".join(h) for f, h in subfrags))
+            subfrag_names = flatten_subfrags_if_needed(subfrags)
-                message = ("Signal '{}' is driven from multiple fragments: {}"
+            if not subfrag_names:
-                           .format(signal, subfrag_names))
+                continue
-                if mode == "error":
+
-                    raise DriverConflict(message)
+            # While we're at it, show a message.
-                elif mode == "warn":
+            message = ("Signal '{}' is driven from multiple fragments: {}"
-                    message += "; hierarchy will be flattened"
+                       .format(signal, ", ".join(subfrag_names)))
-                    warnings.warn_explicit(message, DriverConflict, *signal.src_loc)
+            if mode == "error":
+                raise DriverConflict(message)
+            elif mode == "warn":
+                message += "; hierarchy will be flattened"
+                warnings.warn_explicit(message, DriverConflict, *signal.src_loc)
+
+        for memory, subfrags in memory_subfrags.items():
+            subfrag_names = flatten_subfrags_if_needed(subfrags)
+            if not subfrag_names:
+                continue
+
+            # While we're at it, show a message.
+            message = ("Memory '{}' is accessed from multiple fragments: {}"
+                       .format(memory.name, ", ".join(subfrag_names)))
+            if mode == "error":
+                raise DriverConflict(message)
+            elif mode == "warn":
+                message += "; hierarchy will be flattened"
+                warnings.warn_explicit(message, DriverConflict, *memory.src_loc)
 
         # Flatten hierarchy.
         for subfrag, subfrag_hierarchy in sorted(flatten_subfrags, key=lambda x: x[1]):
@@ -163,10 +193,11 @@ class Fragment:
         # has another conflict.
         if any(flatten_subfrags):
             # Try flattening again.
-            return self._resolve_driver_conflicts(hierarchy, mode)
+            return self._resolve_hierarchy_conflicts(hierarchy, mode)
 
         # Nothing was flattened, we're done!
-        return SignalSet(driver_subfrags.keys())
+        return (SignalSet(driver_subfrags.keys()),
+                set(memory_subfrags.keys()))
 
     def _propagate_domains_up(self, hierarchy=("top",)):
         from .xfrm import DomainRenamer
@@ -309,7 +340,7 @@ class Fragment:
 
         fragment = FragmentTransformer()(self)
         fragment._propagate_domains(ensure_sync_exists)
-        fragment._resolve_driver_conflicts()
+        fragment._resolve_hierarchy_conflicts()
         fragment = fragment._insert_domain_resets()
         fragment = fragment._lower_domain_signals()
         fragment._propagate_ports(ports)

nmigen/test/test_hdl_ir.py

@@ -3,6 +3,7 @@ from collections import OrderedDict
 from ..hdl.ast import *
 from ..hdl.cd import *
 from ..hdl.ir import *
+from ..hdl.mem import *
 from .tools import *
 
 
@@ -321,7 +322,7 @@ class FragmentDomainsTestCase(FHDLTestCase):
         self.assertEqual(f1.domains["sync"], f2.domains["sync"])
 
 
-class FragmentDriverConflictTestCase(FHDLTestCase):
+class FragmentHierarchyConflictTestCase(FHDLTestCase):
     def setUp_self_sub(self):
         self.s1 = Signal()
         self.c1 = Signal()
@@ -350,7 +351,7 @@ class FragmentDriverConflictTestCase(FHDLTestCase):
     def test_conflict_self_sub(self):
         self.setUp_self_sub()
 
-        self.f1._resolve_driver_conflicts(mode="silent")
+        self.f1._resolve_hierarchy_conflicts(mode="silent")
         self.assertEqual(self.f1.subfragments, [
             (self.f1a, "f1a"),
             (self.f1b, "f1b"),
@@ -372,7 +373,7 @@ class FragmentDriverConflictTestCase(FHDLTestCase):
 
         with self.assertRaises(DriverConflict,
                 msg="Signal '(sig s1)' is driven from multiple fragments: top, top.<unnamed #1>"):
-            self.f1._resolve_driver_conflicts(mode="error")
+            self.f1._resolve_hierarchy_conflicts(mode="error")
 
     def test_conflict_self_sub_warning(self):
         self.setUp_self_sub()
@@ -380,7 +381,7 @@ class FragmentDriverConflictTestCase(FHDLTestCase):
         with self.assertWarns(DriverConflict,
                 msg="Signal '(sig s1)' is driven from multiple fragments: top, top.<unnamed #1>; "
                     "hierarchy will be flattened"):
-            self.f1._resolve_driver_conflicts(mode="warn")
+            self.f1._resolve_hierarchy_conflicts(mode="warn")
 
     def setUp_sub_sub(self):
         self.s1 = Signal()
@@ -402,7 +403,7 @@ class FragmentDriverConflictTestCase(FHDLTestCase):
     def test_conflict_sub_sub(self):
         self.setUp_sub_sub()
 
-        self.f1._resolve_driver_conflicts(mode="silent")
+        self.f1._resolve_hierarchy_conflicts(mode="silent")
         self.assertEqual(self.f1.subfragments, [])
         self.assertRepr(self.f1.statements, """
         (
@@ -431,7 +432,7 @@ class FragmentDriverConflictTestCase(FHDLTestCase):
     def test_conflict_self_subsub(self):
         self.setUp_self_subsub()
 
-        self.f1._resolve_driver_conflicts(mode="silent")
+        self.f1._resolve_hierarchy_conflicts(mode="silent")
         self.assertEqual(self.f1.subfragments, [])
         self.assertRepr(self.f1.statements, """
         (
@@ -440,6 +441,43 @@ class FragmentDriverConflictTestCase(FHDLTestCase):
         )
         """)
 
+    def setUp_memory(self):
+        self.m = Memory(width=8, depth=4)
+        self.fr = self.m.read_port().get_fragment(platform=None)
+        self.fw = self.m.write_port().get_fragment(platform=None)
+        self.f1 = Fragment()
+        self.f2 = Fragment()
+        self.f2.add_subfragment(self.fr)
+        self.f1.add_subfragment(self.f2)
+        self.f3 = Fragment()
+        self.f3.add_subfragment(self.fw)
+        self.f1.add_subfragment(self.f3)
+
+    def test_conflict_memory(self):
+        self.setUp_memory()
+
+        self.f1._resolve_hierarchy_conflicts(mode="silent")
+        self.assertEqual(self.f1.subfragments, [
+            (self.fr, None),
+            (self.fw, None),
+        ])
+
+    def test_conflict_memory_error(self):
+        self.setUp_memory()
+
+        with self.assertRaises(DriverConflict,
+                msg="Memory 'm' is accessed from multiple fragments: top.<unnamed #0>, "
+                    "top.<unnamed #1>"):
+            self.f1._resolve_hierarchy_conflicts(mode="error")
+
+    def test_conflict_memory_warning(self):
+        self.setUp_memory()
+
+        with self.assertWarns(DriverConflict,
+                msg="Memory 'm' is accessed from multiple fragments: top.<unnamed #0>, "
+                    "top.<unnamed #1>; hierarchy will be flattened"):
+            self.f1._resolve_hierarchy_conflicts(mode="warn")
+
 
 class InstanceTestCase(FHDLTestCase):
     def setUp_cpu(self):