build.plat, hdl.ir: coordinate missing domain creation.

Platform.prepare() was completely broken after addition of local
clock domains, and only really worked before by a series of
accidents because there was a circular dependency between creation
of missing domains, fragment preparation, and insertion of pin
subfragments.

This commit untangles the dependency by adding a separate public
method Fragment.create_missing_domains(), used in build.plat.

It also makes DomainCollector consider both used and defined domains,
such that it will work on fragments before domain propagation, since
create_missing_domains() can be called by user code before prepare().

The fragment driving missing clock domain is not flattened anymore,
because flattening does not work well combined with local domains.
This commit is contained in:
whitequark 2019-08-19 22:32:50 +00:00
parent 77012fc143
commit 13316053e3
5 changed files with 47 additions and 48 deletions

View file

@ -58,11 +58,11 @@ class Platform(ResourceManager, metaclass=ABCMeta):
raise TypeError("File contents must be str, bytes, or a file-like object")
self.extra_files[filename] = content
def build(self, fragment, name="top",
def build(self, elaboratable, name="top",
build_dir="build", do_build=True,
program_opts=None, do_program=False,
**kwargs):
plan = self.prepare(fragment, name, **kwargs)
plan = self.prepare(elaboratable, name, **kwargs)
if not do_build:
return plan
@ -90,13 +90,12 @@ class Platform(ResourceManager, metaclass=ABCMeta):
m.d.comb += ResetSignal("sync").eq(rst_i)
return m
def prepare(self, fragment, name="top", **kwargs):
def prepare(self, elaboratable, name="top", **kwargs):
assert not self._prepared
self._prepared = True
fragment = Fragment.get(fragment, self)
fragment = fragment.prepare(ports=list(self.iter_ports()),
missing_domain=self.create_missing_domain)
fragment = Fragment.get(elaboratable, self)
fragment.create_missing_domains(self.create_missing_domain)
def add_pin_fragment(pin, pin_fragment):
pin_fragment = Fragment.get(pin_fragment, self)
@ -125,6 +124,7 @@ class Platform(ResourceManager, metaclass=ABCMeta):
add_pin_fragment(pin,
self.get_diff_input_output(pin, p_port, n_port, attrs, invert))
fragment = fragment.prepare(ports=self.iter_ports(), missing_domain=lambda name: None)
return self.toolchain_prepare(fragment, name, **kwargs)
@abstractmethod

View file

@ -355,46 +355,41 @@ class Fragment:
subfrag._propagate_domains_down()
def _create_missing_domains(self, missing_domain):
def create_missing_domains(self, missing_domain):
from .xfrm import DomainCollector
collector = DomainCollector()
collector(self)
new_domains = []
for domain_name in DomainCollector()(self):
for domain_name in collector.used_domains - collector.defined_domains:
if domain_name is None:
continue
if domain_name not in self.domains:
value = missing_domain(domain_name)
if value is None:
raise DomainError("Domain '{}' is used but not defined".format(domain_name))
if type(value) is ClockDomain:
self.add_domains(value)
# And expose ports on the newly added clock domain, since it is added directly
# and there was no chance to add any logic driving it.
new_domains.append(value)
else:
new_fragment = Fragment.get(value, platform=None)
if domain_name not in new_fragment.domains:
defined = new_fragment.domains.keys()
raise DomainError(
"Fragment returned by missing domain callback does not define "
"requested domain '{}' (defines {})."
.format(domain_name, ", ".join("'{}'".format(n) for n in defined)))
new_fragment.flatten = True
self.add_subfragment(new_fragment)
self.add_domains(new_fragment.domains.values())
value = missing_domain(domain_name)
if value is None:
raise DomainError("Domain '{}' is used but not defined".format(domain_name))
if type(value) is ClockDomain:
self.add_domains(value)
# And expose ports on the newly added clock domain, since it is added directly
# and there was no chance to add any logic driving it.
new_domains.append(value)
else:
new_fragment = Fragment.get(value, platform=None)
if domain_name not in new_fragment.domains:
defined = new_fragment.domains.keys()
raise DomainError(
"Fragment returned by missing domain callback does not define "
"requested domain '{}' (defines {})."
.format(domain_name, ", ".join("'{}'".format(n) for n in defined)))
self.add_subfragment(new_fragment, "cd_{}".format(domain_name))
return new_domains
def _propagate_domains(self, missing_domain):
new_domains = self.create_missing_domains(missing_domain)
self._propagate_domains_up()
new_domains = self._create_missing_domains(missing_domain)
self._propagate_domains_down()
return new_domains
def _lower_domain_signals(self):
from .xfrm import DomainLowerer
return DomainLowerer()(self)
def _prepare_use_def_graph(self, parent, level, uses, defs, ios, top):
def add_uses(*sigs, self=self):
for sig in flatten(sigs):
@ -536,12 +531,12 @@ class Fragment:
self.add_ports(sig, dir="i")
def prepare(self, ports=None, missing_domain=lambda name: ClockDomain(name)):
from .xfrm import SampleLowerer
from .xfrm import SampleLowerer, DomainLowerer
fragment = SampleLowerer()(self)
new_domains = fragment._propagate_domains(missing_domain)
fragment._resolve_hierarchy_conflicts()
fragment = fragment._lower_domain_signals()
fragment = DomainLowerer()(fragment)
if ports is None:
fragment._propagate_ports(ports=(), all_undef_as_ports=True)
else:

View file

@ -336,12 +336,16 @@ class TransformedElaboratable(Elaboratable):
class DomainCollector(ValueVisitor, StatementVisitor):
def __init__(self):
self.domains = set()
self.used_domains = set()
self.defined_domains = set()
self._local_domains = set()
def _add_domain(self, domain_name):
if domain_name not in self._local_domains:
self.domains.add(domain_name)
def _add_used_domain(self, domain_name):
if domain_name is None:
return
if domain_name in self._local_domains:
return
self.used_domains.add(domain_name)
def on_ignore(self, value):
pass
@ -352,10 +356,10 @@ class DomainCollector(ValueVisitor, StatementVisitor):
on_Signal = on_ignore
def on_ClockSignal(self, value):
self._add_domain(value.domain)
self._add_used_domain(value.domain)
def on_ResetSignal(self, value):
self._add_domain(value.domain)
self._add_used_domain(value.domain)
on_Record = on_ignore
@ -416,10 +420,12 @@ class DomainCollector(ValueVisitor, StatementVisitor):
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.on_statements(fragment.statements)
for domain_name in fragment.drivers:
self._add_domain(domain_name)
self._add_used_domain(domain_name)
for subfragment, name in fragment.subfragments:
self.on_fragment(subfragment)
@ -427,7 +433,6 @@ class DomainCollector(ValueVisitor, StatementVisitor):
def __call__(self, fragment):
self.on_fragment(fragment)
return self.domains
class DomainRenamer(FragmentTransformer, ValueTransformer, StatementTransformer):

View file

@ -428,9 +428,8 @@ class FragmentDomainsTestCase(FHDLTestCase):
self.assertEqual(f1.domains["sync"], f2.domains["sync"])
self.assertEqual(new_domains, [])
self.assertEqual(f1.subfragments, [
(f2, None)
(f2, "cd_sync")
])
self.assertTrue(f2.flatten)
def test_propagate_create_missing_fragment_many_domains(self):
s1 = Signal()
@ -448,7 +447,7 @@ class FragmentDomainsTestCase(FHDLTestCase):
self.assertEqual(f1.domains["sync"], f2.domains["sync"])
self.assertEqual(new_domains, [])
self.assertEqual(f1.subfragments, [
(f2, None)
(f2, "cd_sync")
])
def test_propagate_create_missing_fragment_wrong(self):

View file

@ -250,7 +250,7 @@ class SimulatorUnitTestCase(FHDLTestCase):
class SimulatorIntegrationTestCase(FHDLTestCase):
@contextmanager
def assertSimulation(self, module, deadline=None):
with Simulator(module.elaborate(platform=None)) as sim:
with Simulator(module) as sim:
yield sim
if deadline is None:
sim.run()