Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AssertionError domain.name not in self.domains #307

Closed
nicolas-robin opened this issue Jan 18, 2020 · 4 comments
Closed

AssertionError domain.name not in self.domains #307

nicolas-robin opened this issue Jan 18, 2020 · 4 comments

Comments

@nicolas-robin
Copy link

nicolas-robin commented Jan 18, 2020

Since a7be3b4, this code fails :

Code :

from nmigen import *
from nmigen_boards.ice40_hx8k_b_evn import ICE40HX8KBEVNPlatform

platform = ICE40HX8KBEVNPlatform()
top = Module()
top.d.sync += platform.request("led").o.eq(1)
platform.build(top)

Error :

Traceback (most recent call last):
  File "[...]/sandbox.py", line 7, in <module>
    platform.build(top)
  File "[...]/venv/lib/python3.7/site-packages/nmigen/build/plat.py", line 77, in build
    plan = self.prepare(elaboratable, name, **kwargs)
  File "[...]/venv/lib/python3.7/site-packages/nmigen/build/plat.py", line 147, in prepare
    fragment = fragment.prepare(ports=self.iter_ports(), missing_domain=lambda name: None)
  File "[...]/venv/lib/python3.7/site-packages/nmigen/hdl/ir.py", line 556, in prepare
    new_domains = fragment._propagate_domains(missing_domain)
  File "[...]/venv/lib/python3.7/site-packages/nmigen/hdl/ir.py", line 403, in _propagate_domains
    self._propagate_domains_up()
  File "[...]/venv/lib/python3.7/site-packages/nmigen/hdl/ir.py", line 359, in _propagate_domains_up
    self.add_domains(domain)
  File "[...]/venv/lib/python3.7/site-packages/nmigen/hdl/ir.py", line 145, in add_domains
    assert domain.name not in self.domains
AssertionError
@smunaut
Copy link
Contributor

smunaut commented Jan 20, 2020

Same error happens when trying to run glasgow with latest nMigen.
(seen by me, @esden @miek at least)

@smunaut
Copy link
Contributor

smunaut commented Jan 20, 2020

Not sure if this is the correct fix, but this seems to work for me :

diff --git a/nmigen/hdl/ir.py b/nmigen/hdl/ir.py
index 26dec85..2b8b74c 100644
--- a/nmigen/hdl/ir.py
+++ b/nmigen/hdl/ir.py
@@ -142,7 +142,7 @@ class Fragment:
     def add_domains(self, *domains):
         for domain in flatten(domains):
             assert isinstance(domain, ClockDomain)
-            assert domain.name not in self.domains
+            assert (domain.name not in self.domains) or (self.domains[domain.name] == domain)
             self.domains[domain.name] = domain
 
     def iter_domains(self):

@whitequark
Copy link
Contributor

Likely not the correct fix, but an acceptable workaround.

@whitequark
Copy link
Contributor

whitequark commented Feb 1, 2020

Fixed in my fork.

sbourdeauducq pushed a commit that referenced this issue Feb 2, 2020
Since commit 7257c20, platform code calls create_missing_domains()
before _propagate_domains_up() (as a part of prepare() call). Since
commit a7be3b4, without a platform, create_missing_domains() is
calle after _propagate_domains_up(); because of that, it adds
the missing domain to the fragment. When platform code then calls
prepare() again, this causes an assertion failure.

The true intent behind the platform code being written this way is
that it *overrides* a part of prepare()'s mechanism. Because it was
not changed when prepare() was modified in 7257c20, the override,
which happened to work by coincidence, stopped working. This is
now fixed by inlining the relevant parts of Fragment.prepare() into
Platform.prepare().

This is not a great solution, but given the amount of breakage this
causes (no platform-using code works), it is acceptable for now.

Fixes #307.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants