diff --git a/amaranth/hdl/_ir.py b/amaranth/hdl/_ir.py index cd2e3dec6..5037afb9a 100644 --- a/amaranth/hdl/_ir.py +++ b/amaranth/hdl/_ir.py @@ -67,6 +67,7 @@ def __init__(self, *, src_loc=None): self.generated = OrderedDict() self.src_loc = src_loc self.origins = None + self.domains_propagated_up = {} def add_driver(self, signal, domain="comb"): assert isinstance(domain, str) @@ -179,22 +180,37 @@ def _propagate_domains_up(self, hierarchy=("top",)): self.subfragments[i] = (DomainRenamer(domain_name_map)(subfrag), name, src_loc) # Finally, collect the (now unique) subfragment domains, and merge them into our domains. - for subfrag, name, src_loc in self.subfragments: + for i, (subfrag, name, src_loc) in enumerate(self.subfragments): + hier_name = name + if hier_name is None: + hier_name = f"" for domain_name, domain in subfrag.domains.items(): if domain.local: continue self.add_domains(domain) + if domain_name in subfrag.domains_propagated_up: + _used_in, defined_in = subfrag.domains_propagated_up[domain_name] + else: + defined_in = (*hierarchy, hier_name) + self.domains_propagated_up[domain_name] = (hierarchy, defined_in) - def _propagate_domains_down(self): + def _propagate_domains_down(self, hierarchy=("top",)): # For each domain defined in this fragment, ensure it also exists in all subfragments. - for subfrag, name, src_loc in self.subfragments: + for i, (subfrag, name, src_loc) in enumerate(self.subfragments): + hier_name = name + if hier_name is None: + hier_name = f"" + for domain in self.iter_domains(): if domain in subfrag.domains: assert self.domains[domain] is subfrag.domains[domain] else: subfrag.add_domains(self.domains[domain]) + if domain in self.domains_propagated_up: + _used_in, defined_in = self.domains_propagated_up[domain] + subfrag.domains_propagated_up[domain] = (hierarchy + (hier_name,)), defined_in - subfrag._propagate_domains_down() + subfrag._propagate_domains_down(hierarchy + (hier_name,)) def _create_missing_domains(self, missing_domain, *, platform=None): from ._xfrm import DomainCollector diff --git a/amaranth/hdl/_xfrm.py b/amaranth/hdl/_xfrm.py index 90c23b7d8..f96ec5161 100644 --- a/amaranth/hdl/_xfrm.py +++ b/amaranth/hdl/_xfrm.py @@ -1,3 +1,4 @@ +import warnings from abc import ABCMeta, abstractmethod from collections import OrderedDict from collections.abc import Iterable @@ -539,11 +540,26 @@ def map_memory_ports(self, fragment, new_fragment): class DomainLowerer(FragmentTransformer, ValueTransformer, StatementTransformer): def __init__(self, domains=None): self.domains = domains + self.domains_propagated_up = {} + + def _warn_on_propagation_up(self, domain, src_loc): + if domain in self.domains_propagated_up: + used_in, defined_in = self.domains_propagated_up[domain] + common_prefix = [] + for u, d in zip(used_in, defined_in): + if u == d: + common_prefix.append(u) + warnings.warn_explicit(f"Domain '{domain}' is used in '{'.'.join(used_in)}', but " + f"defined in '{'.'.join(defined_in)}', which will not be " + f"supported in Amaranth 0.6; define the domain in " + f"'{'.'.join(common_prefix)}' or one of its parents", + DeprecationWarning, filename=src_loc[0], lineno=src_loc[1]) def _resolve(self, domain, context): if domain not in self.domains: raise DomainError("Signal {!r} refers to nonexistent domain '{}'" .format(context, domain)) + self._warn_on_propagation_up(domain, context.src_loc) return self.domains[domain] def map_drivers(self, fragment, new_fragment): @@ -569,6 +585,14 @@ def on_ResetSignal(self, value): def on_fragment(self, fragment): self.domains = fragment.domains + self.domains_propagated_up = fragment.domains_propagated_up + for domain, statements in fragment.statements.items(): + self._warn_on_propagation_up(domain, statements[0].src_loc) + if isinstance(fragment, MemoryInstance): + for port in fragment._read_ports: + self._warn_on_propagation_up(port._domain, fragment.src_loc) + for port in fragment._write_ports: + self._warn_on_propagation_up(port._domain, fragment.src_loc) return super().on_fragment(fragment) diff --git a/docs/changes.rst b/docs/changes.rst index 2675c3885..0c482340b 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -38,6 +38,7 @@ Apply the following changes to code written against Amaranth 0.4 to migrate it t * Replace imports of ``amaranth.asserts.{Assert, Assume, Cover}`` with imports from ``amaranth.hdl`` * Remove any usage of ``name=`` with assertions, possibly replacing them with custom messages * Ensure all elaboratables are subclasses of :class:`Elaboratable` +* Ensure clock domains aren't used outside the module that defines them, or its submodules; move clock domain definitions upwards in the hierarchy as necessary Implemented RFCs @@ -53,6 +54,7 @@ Implemented RFCs .. _RFC 51: https://amaranth-lang.org/rfcs/0051-const-from-bits.html .. _RFC 53: https://amaranth-lang.org/rfcs/0053-ioport.html .. _RFC 55: https://amaranth-lang.org/rfcs/0055-lib-io.html +.. _RFC 59: https://amaranth-lang.org/rfcs/0059-no-domain-upwards-propagation.html .. _RFC 62: https://amaranth-lang.org/rfcs/0062-memory-data.html * `RFC 17`_: Remove ``log2_int`` @@ -64,6 +66,7 @@ Implemented RFCs * `RFC 50`_: ``Print`` statement and string formatting * `RFC 51`_: Add ``ShapeCastable.from_bits`` and ``amaranth.lib.data.Const`` * `RFC 53`_: Low-level I/O primitives +* `RFC 59`_: Get rid of upwards propagation of clock domains Language changes @@ -88,6 +91,7 @@ Language changes * Changed: :class:`Instance` IO ports now accept only IO values, not plain values. (`RFC 53`_) * Deprecated: :func:`amaranth.utils.log2_int`. (`RFC 17`_) * Deprecated: :class:`amaranth.hdl.Memory`. (`RFC 45`_) +* Deprecated: upwards propagation of clock domains. (`RFC 59`_) * Removed: (deprecated in 0.4) :meth:`Const.normalize`. (`RFC 5`_) * Removed: (deprecated in 0.4) :class:`Repl`. (`RFC 10`_) * Removed: (deprecated in 0.4) :class:`ast.Sample`, :class:`ast.Past`, :class:`ast.Stable`, :class:`ast.Rose`, :class:`ast.Fell`. diff --git a/tests/test_hdl_ir.py b/tests/test_hdl_ir.py index a1de10bfc..07464e33b 100644 --- a/tests/test_hdl_ir.py +++ b/tests/test_hdl_ir.py @@ -668,6 +668,20 @@ def test_propagate_create_missing_fragment_wrong(self): r"domain 'sync' \(defines 'foo'\)\.$")): f1._propagate_domains(missing_domain=lambda name: f2) + def test_propagate_up_use(self): + a = Signal() + m = Module() + m1 = Module() + m2 = Module() + m.submodules.m1 = m1 + m.submodules.m2 = m2 + m1.d.test += a.eq(1) + m2.domains.test = ClockDomain() + with self.assertWarnsRegex(DeprecationWarning, + r"^Domain 'test' is used in 'top.m1', but defined in 'top.m2', which will not be " + r"supported in Amaranth 0.6; define the domain in 'top' or one of its parents$"): + Fragment.get(m, None).prepare() + class FragmentHierarchyConflictTestCase(FHDLTestCase): def test_no_conflict_local_domains(self):