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

ResetSynchronizer clockdomain in submodule is not renamed properly with multiple submodule instances #304

Closed
povauboin opened this issue Jan 13, 2020 · 3 comments
Labels
Milestone

Comments

@povauboin
Copy link

Traceback (most recent call last):
  File "test.py", line 37, in <module>
    print(verilog.convert(top, ports=[top.clk, top.rst]))
  File "/mnt/data/prog/fpga/nmigen/nmigen/back/verilog.py", line 76, in convert
    rtlil_text = rtlil.convert(*args, **kwargs)
  File "/mnt/data/prog/fpga/nmigen/nmigen/back/rtlil.py", line 1010, in convert
    fragment = ir.Fragment.get(elaboratable, platform).prepare(**kwargs)
  File "/mnt/data/prog/fpga/nmigen/nmigen/hdl/ir.py", line 548, in prepare
    fragment = DomainLowerer()(fragment)
  File "/mnt/data/prog/fpga/nmigen/nmigen/hdl/xfrm.py", line 315, in __call__
    return self.on_fragment(value)
  File "/mnt/data/prog/fpga/nmigen/nmigen/hdl/xfrm.py", line 539, in on_fragment
    new_fragment = super().on_fragment(fragment)
  File "/mnt/data/prog/fpga/nmigen/nmigen/hdl/xfrm.py", line 307, in on_fragment
    self.map_subfragments(fragment, new_fragment)
  File "/mnt/data/prog/fpga/nmigen/nmigen/hdl/xfrm.py", line 270, in map_subfragments
    new_fragment.add_subfragment(self(subfragment), name)
  File "/mnt/data/prog/fpga/nmigen/nmigen/hdl/xfrm.py", line 315, in __call__
    return self.on_fragment(value)
  File "/mnt/data/prog/fpga/nmigen/nmigen/hdl/xfrm.py", line 539, in on_fragment
    new_fragment = super().on_fragment(fragment)
  File "/mnt/data/prog/fpga/nmigen/nmigen/hdl/xfrm.py", line 307, in on_fragment
    self.map_subfragments(fragment, new_fragment)
  File "/mnt/data/prog/fpga/nmigen/nmigen/hdl/xfrm.py", line 270, in map_subfragments
    new_fragment.add_subfragment(self(subfragment), name)
  File "/mnt/data/prog/fpga/nmigen/nmigen/hdl/xfrm.py", line 315, in __call__
    return self.on_fragment(value)
  File "/mnt/data/prog/fpga/nmigen/nmigen/hdl/xfrm.py", line 539, in on_fragment
    new_fragment = super().on_fragment(fragment)
  File "/mnt/data/prog/fpga/nmigen/nmigen/hdl/xfrm.py", line 310, in on_fragment
    self.map_drivers(fragment, new_fragment)
  File "/mnt/data/prog/fpga/nmigen/nmigen/hdl/xfrm.py", line 506, in map_drivers
    new_fragment.add_driver(self.on_value(signal), domain)
  File "/mnt/data/prog/fpga/nmigen/nmigen/hdl/xfrm.py", line 106, in on_value
    new_value = self.on_ResetSignal(value)
  File "/mnt/data/prog/fpga/nmigen/nmigen/hdl/xfrm.py", line 516, in on_ResetSignal
    domain = self._resolve(value.domain, value)
  File "/mnt/data/prog/fpga/nmigen/nmigen/hdl/xfrm.py", line 500, in _resolve
    raise DomainError("Signal {!r} refers to nonexistent domain '{}'"
nmigen.hdl.cd.DomainError: Signal (rst sub) refers to nonexistent domain 'sub'

MCVE:

from nmigen import *
from nmigen.back import verilog
from nmigen.lib.cdc import ResetSynchronizer

class MySubModule(Elaboratable):
    def __init__(self, clk, rst):
        self.clk = clk
        self.rst = rst

    def elaborate(self, platform):
        m = Module()

        m.domains.cd_sub = cd_sub = ClockDomain()
        m.d.comb += cd_sub.clk.eq(self.clk)
        m.submodules.reset_sync = ResetSynchronizer(self.rst, domain="sub")

        return m

class MyTopModule(Elaboratable):
    def __init__(self):
        self.clk = Signal()
        self.rst = Signal()
        self.counter = Signal(8)

    def elaborate(self, platform):
        m = Module()

        m.submodules.s1 = MySubModule(self.clk, self.rst)
        m.submodules.s2 = MySubModule(self.clk, self.rst)

        # m.d.s1_sub += self.counter.eq(self.counter + 1)

        return m

if __name__ == "__main__":
    top = MyTopModule()
    print(verilog.convert(top, ports=[top.clk, top.rst]))
@whitequark whitequark added the bug label Jan 13, 2020
@whitequark whitequark added this to the 0.2 milestone Jan 16, 2020
@povauboin
Copy link
Author

Thanks @whitequark . Can you clarify how to use the renamed clock domains ?

Doing this:

m.d.s1_sub += self.counter.eq(self.counter + 1)

yields the error:

Traceback (most recent call last):
  File "test.py", line 40, in <module>
    print(verilog.convert(top, ports=[top.clk, top.rst]))
  File "/mnt/data/prog/fpga/nmigen/nmigen/back/verilog.py", line 76, in convert
    rtlil_text = rtlil.convert(*args, **kwargs)
  File "/mnt/data/prog/fpga/nmigen/nmigen/back/rtlil.py", line 1010, in convert
    fragment = ir.Fragment.get(elaboratable, platform).prepare(**kwargs)
  File "/mnt/data/prog/fpga/nmigen/nmigen/hdl/ir.py", line 544, in prepare
    new_domains = fragment._propagate_domains(missing_domain)
  File "/mnt/data/prog/fpga/nmigen/nmigen/hdl/ir.py", line 394, in _propagate_domains
    self._propagate_domains_up()
  File "/mnt/data/prog/fpga/nmigen/nmigen/hdl/ir.py", line 350, in _propagate_domains_up
    self.add_domains(domain)
  File "/mnt/data/prog/fpga/nmigen/nmigen/hdl/ir.py", line 145, in add_domains
    assert domain.name not in self.domains
AssertionError

But if the submodule is created with a DomainRenamer like this, it works:

m.submodules.s1 = DomainRenamer({"sub": "s1_sub"})(
    MySubModule(self.clk, self.rst)                
)                                                  

@whitequark
Copy link
Contributor

Can you clarify how to use the renamed clock domains ?

That's actually a different bug, one that is much harder to fix.

@whitequark
Copy link
Contributor

@povauboin Your example works now.

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

No branches or pull requests

2 participants