diff --git a/amaranth/hdl/_cd.py b/amaranth/hdl/_cd.py index 51a26e71d..b4a8b2b34 100644 --- a/amaranth/hdl/_cd.py +++ b/amaranth/hdl/_cd.py @@ -1,3 +1,5 @@ +import warnings + from .. import tracer from ._ast import Signal @@ -27,9 +29,6 @@ class ClockDomain: If ``True``, the domain uses an asynchronous reset, and registers within this domain are initialized to their reset state when reset level changes. Otherwise, registers are initialized to reset state at the next clock cycle when reset is asserted. - local : bool - If ``True``, the domain will propagate only downwards in the design hierarchy. Otherwise, - the domain will propagate everywhere. Attributes ---------- @@ -48,7 +47,7 @@ def _name_for(domain_name, signal_name): return f"{domain_name}_{signal_name}" def __init__(self, name=None, *, clk_edge="pos", reset_less=False, async_reset=False, - local=False): + local=None): if name is None: try: name = tracer.get_var_name() @@ -75,7 +74,17 @@ def __init__(self, name=None, *, clk_edge="pos", reset_less=False, async_reset=F self.async_reset = async_reset - self.local = local + # TODO(amaranth-0.7): remove + if local is not None: + warnings.warn(f"`local` is deprecated and has no effect", + DeprecationWarning, stacklevel=2) + + @property + def local(self): + # TODO(amaranth-0.7): remove + warnings.warn(f"`local` is deprecated and has no effect", + DeprecationWarning, stacklevel=2) + return True def rename(self, new_name): self.name = new_name diff --git a/amaranth/hdl/_ir.py b/amaranth/hdl/_ir.py index f24cd2e41..c750c5403 100644 --- a/amaranth/hdl/_ir.py +++ b/amaranth/hdl/_ir.py @@ -82,7 +82,6 @@ def __init__(self, *, src_loc=None): self.generated = OrderedDict() self.src_loc = src_loc self.origins = None - self.domains_propagated_up = {} self.domain_renames = {} def add_domains(self, *domains): @@ -127,67 +126,6 @@ def find_generated(self, *path): def elaborate(self, platform): return self - def _propagate_domains_up(self, hierarchy=("top",)): - from ._xfrm import DomainRenamer - - domain_subfrags = defaultdict(set) - - # For each domain defined by a subfragment, determine which subfragments define it. - for i, (subfrag, name, src_loc) in enumerate(self.subfragments): - # First, recurse into subfragments and let them propagate domains up as well. - hier_name = name - if hier_name is None: - hier_name = f"" - subfrag._propagate_domains_up(hierarchy + (hier_name,)) - - # Second, classify subfragments by domains they define. - for domain_name, domain in subfrag.domains.items(): - if domain.local: - continue - domain_subfrags[domain_name].add((subfrag, name, src_loc, i)) - - # For each domain defined by more than one subfragment, rename the domain in each - # of the subfragments such that they no longer conflict. - for domain_name, subfrags in domain_subfrags.items(): - if len(subfrags) == 1: - continue - - names = [n for f, n, s, i in subfrags] - if not all(names): - names = sorted(f"" if n is None else f"'{n}'" - for f, n, s, i in subfrags) - raise _cd.DomainError( - "Domain '{}' is defined by subfragments {} of fragment '{}'; it is necessary " - "to either rename subfragment domains explicitly, or give names to subfragments" - .format(domain_name, ", ".join(names), ".".join(hierarchy))) - - if len(names) != len(set(names)): - names = sorted(f"#{i}" for f, n, s, i in subfrags) - raise _cd.DomainError( - "Domain '{}' is defined by subfragments {} of fragment '{}', some of which " - "have identical names; it is necessary to either rename subfragment domains " - "explicitly, or give distinct names to subfragments" - .format(domain_name, ", ".join(names), ".".join(hierarchy))) - - for subfrag, name, src_loc, i in subfrags: - domain_name_map = {domain_name: f"{name}_{domain_name}"} - 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 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, hierarchy=("top",)): # For each domain defined in this fragment, ensure it also exists in all subfragments. for i, (subfrag, name, src_loc) in enumerate(self.subfragments): @@ -196,13 +134,8 @@ def _propagate_domains_down(self, hierarchy=("top",)): hier_name = f"" for domain in self.iter_domains(): - if domain in subfrag.domains: - assert self.domains[domain] is subfrag.domains[domain] - else: + if domain not in subfrag.domains: 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(hierarchy + (hier_name,)) @@ -237,7 +170,6 @@ def _create_missing_domains(self, missing_domain, *, platform=None): return new_domains def _propagate_domains(self, missing_domain, *, platform=None): - self._propagate_domains_up() self._propagate_domains_down() new_domains = self._create_missing_domains(missing_domain, platform=platform) self._propagate_domains_down() diff --git a/amaranth/hdl/_xfrm.py b/amaranth/hdl/_xfrm.py index 92d070594..5d4d95c5e 100644 --- a/amaranth/hdl/_xfrm.py +++ b/amaranth/hdl/_xfrm.py @@ -464,10 +464,7 @@ def on_fragment(self, fragment): old_local_domains, self._local_domains = self._local_domains, set(self._local_domains) for domain_name, domain in fragment.domains.items(): - if domain.local: - self._local_domains.add(domain_name) - else: - self.defined_domains.add(domain_name) + self._local_domains.add(domain_name) for domain_name, statements in fragment.statements.items(): self._add_used_domain(domain_name) @@ -549,26 +546,11 @@ def on_fragment(self, 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 replace_value_src_loc(self, value, new_value): @@ -590,14 +572,6 @@ 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/amaranth/lib/cdc.py b/amaranth/lib/cdc.py index 1afcc7698..776c1bcc8 100644 --- a/amaranth/lib/cdc.py +++ b/amaranth/lib/cdc.py @@ -173,7 +173,7 @@ def elaborate(self, platform): .format(type(platform).__name__)) m = Module() - m.domains += ClockDomain("async_ff", async_reset=True, local=True) + m.domains += ClockDomain("async_ff", async_reset=True) flops = [Signal(1, name=f"stage{index}", init=1) for index in range(self._stages)] for i, o in zip((0, *flops), flops): diff --git a/docs/changes.rst b/docs/changes.rst index 22ca652be..5731eef83 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -29,11 +29,13 @@ Language changes .. currentmodule:: amaranth.hdl * Changed: overriding :meth:`ValueCastable.from_bits` is now mandatory. (`RFC 51`_) +* Deprecated: the :py:`local=` argument to :class:`ClockDomain`. (`RFC 59`_) * Removed: (deprecated in 0.4) :class:`Record`. * Removed: (deprecated in 0.5) :class:`Memory` (`RFC 45`_) * Removed: (deprecated in 0.5) public submodules of :mod:`amaranth.hdl`. * Removed: (deprecated in 0.5) :meth:`Value.implies`. -* Removed: (deprecated in 0.5) :meth:`Const.width`, :meth:`Const.signed`, :meth:`Signal.width`, :meth:`Signal.signed` +* Removed: (deprecated in 0.5) :meth:`Const.width`, :meth:`Const.signed`, :meth:`Signal.width`, :meth:`Signal.signed`. +* Removed: (deprecated in 0.5) upwards propagation of clock domains. (`RFC 59`_) Standard library changes diff --git a/tests/test_hdl_cd.py b/tests/test_hdl_cd.py index 5a9fa736b..b2625501e 100644 --- a/tests/test_hdl_cd.py +++ b/tests/test_hdl_cd.py @@ -9,7 +9,6 @@ def test_name(self): self.assertEqual(sync.name, "sync") self.assertEqual(sync.clk.name, "clk") self.assertEqual(sync.rst.name, "rst") - self.assertEqual(sync.local, False) pix = ClockDomain() self.assertEqual(pix.name, "pix") self.assertEqual(pix.clk.name, "pix_clk") @@ -21,8 +20,6 @@ def test_name(self): with self.assertRaisesRegex(ValueError, r"^Clock domain name must be specified explicitly$"): ClockDomain() - cd_reset = ClockDomain(local=True) - self.assertEqual(cd_reset.local, True) def test_edge(self): sync = ClockDomain() @@ -77,3 +74,11 @@ def test_wrong_name_comb(self): with self.assertRaisesRegex(ValueError, r"^Domain 'comb' may not be clocked$"): comb = ClockDomain() + + def test_local(self): + with self.assertWarnsRegex(DeprecationWarning, + r"^`local` is deprecated and has no effect$"): + sync = ClockDomain(local=False) + with self.assertWarnsRegex(DeprecationWarning, + r"^`local` is deprecated and has no effect$"): + self.assertTrue(sync.local) \ No newline at end of file diff --git a/tests/test_hdl_ir.py b/tests/test_hdl_ir.py index 5b6a682cd..060e67ef4 100644 --- a/tests/test_hdl_ir.py +++ b/tests/test_hdl_ir.py @@ -456,99 +456,6 @@ def test_port_not_iterable(self): class FragmentDomainsTestCase(FHDLTestCase): - def test_propagate_up(self): - cd = ClockDomain() - - f1 = Fragment() - f2 = Fragment() - f1.add_subfragment(f2) - f2.add_domains(cd) - - f1._propagate_domains_up() - self.assertEqual(f1.domains, {"cd": cd}) - - def test_propagate_up_local(self): - cd = ClockDomain(local=True) - - f1 = Fragment() - f2 = Fragment() - f1.add_subfragment(f2) - f2.add_domains(cd) - - f1._propagate_domains_up() - self.assertEqual(f1.domains, {}) - - def test_domain_conflict(self): - cda = ClockDomain("sync") - cdb = ClockDomain("sync") - - fa = Fragment() - fa.add_domains(cda) - fb = Fragment() - fb.add_domains(cdb) - f = Fragment() - f.add_subfragment(fa, "a") - f.add_subfragment(fb, "b") - - f._propagate_domains_up() - self.assertEqual(f.domains, {"a_sync": cda, "b_sync": cdb}) - (fa, _, _), (fb, _, _) = f.subfragments - self.assertEqual(fa.domains, {"a_sync": cda}) - self.assertEqual(fb.domains, {"b_sync": cdb}) - - def test_domain_conflict_anon(self): - cda = ClockDomain("sync") - cdb = ClockDomain("sync") - - fa = Fragment() - fa.add_domains(cda) - fb = Fragment() - fb.add_domains(cdb) - f = Fragment() - f.add_subfragment(fa, "a") - f.add_subfragment(fb) - - with self.assertRaisesRegex(DomainError, - (r"^Domain 'sync' is defined by subfragments 'a', of fragment " - r"'top'; it is necessary to either rename subfragment domains explicitly, " - r"or give names to subfragments$")): - f._propagate_domains_up() - - def test_domain_conflict_name(self): - cda = ClockDomain("sync") - cdb = ClockDomain("sync") - - fa = Fragment() - fa.add_domains(cda) - fb = Fragment() - fb.add_domains(cdb) - f = Fragment() - f.add_subfragment(fa, "x") - f.add_subfragment(fb, "x") - - with self.assertRaisesRegex(DomainError, - (r"^Domain 'sync' is defined by subfragments #0, #1 of fragment 'top', some " - r"of which have identical names; it is necessary to either rename subfragment " - r"domains explicitly, or give distinct names to subfragments$")): - f._propagate_domains_up() - - def test_domain_conflict_rename_drivers(self): - cda = ClockDomain("sync") - cdb = ClockDomain("sync") - - fa = Fragment() - fa.add_domains(cda) - fb = Fragment() - fb.add_domains(cdb) - fb.add_statements("comb", ResetSignal("sync").eq(1)) - f = Fragment() - f.add_subfragment(fa, "a") - f.add_subfragment(fb, "b") - - f._propagate_domains_up() - fb_new, _, _ = f.subfragments[1] - self.assertRepr(fb_new.statements["comb"], "((eq (rst b_sync) (const 1'd1)))") - def test_domain_conflict_rename_drivers_before_creating_missing(self): cda = ClockDomain("sync") cdb = ClockDomain("sync") @@ -673,29 +580,15 @@ 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): f1 = Fragment() - cd1 = ClockDomain("d", local=True) + cd1 = ClockDomain("d") f1.add_domains(cd1) f1.add_statements("comb", ClockSignal("d").eq(1)) f2 = Fragment() - cd2 = ClockDomain("d", local=True) + cd2 = ClockDomain("d") f2.add_domains(cd2) f2.add_statements("comb", ClockSignal("d").eq(1)) f3 = Fragment()