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.

Sign up for GitHub

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

Jump to bottom

Memory with only a read port fails in pysim #20

Closed
adamgreig opened this issue Jan 5, 2019 · 2 comments
Closed

Memory with only a read port fails in pysim #20

adamgreig opened this issue Jan 5, 2019 · 2 comments
Labels
bug
Milestone
0.1

Comments

@adamgreig
Copy link
Contributor

A memory with a read port but no write port leads to a mysterious KeyError when simulated. Small example:

from nmigen import Module, Memory
from nmigen.back import pysim


def test_mem():
    m = Module()
    mem = Memory(width=8, depth=4, init=[0xaa, 0x55])
    m.submodules.rdport = rdport = mem.read_port()
    # Uncomment to fix this test
    # m.submodules.wrport = mem.write_port()
    with pysim.Simulator(m.lower(None)) as sim:
        def process():
            yield rdport.addr.eq(1)
            yield
            yield
            assert (yield rdport.data) == 0x55
        sim.add_clock(1e-6)
        sim.add_sync_process(process)
        sim.run()


if __name__ == "__main__":
    test_mem()

As shown, this example results in the following traceback:

Traceback 
Traceback (most recent call last):
  File "rom.py", line 22, in <module>
    test_mem()
  File "rom.py", line 10, in test_mem
    with pysim.Simulator(m.lower(None)) as sim:
  File "/home/adam/.local/lib/python3.6/site-packages/nmigen-0.1-py3.6.egg/nmigen/back/pysim.py", line 551, in __enter__
    funclet = compiler(statements)
  File "/home/adam/.local/lib/python3.6/site-packages/nmigen-0.1-py3.6.egg/nmigen/hdl/xfrm.py", line 179, in __call__
    return self.on_statement(value)
  File "/home/adam/.local/lib/python3.6/site-packages/nmigen-0.1-py3.6.egg/nmigen/hdl/xfrm.py", line 174, in on_statement
    return self.on_statements(stmt)
  File "/home/adam/.local/lib/python3.6/site-packages/nmigen-0.1-py3.6.egg/nmigen/back/pysim.py", line 329, in on_statements
    stmts = [self.on_statement(stmt) for stmt in stmts]
  File "/home/adam/.local/lib/python3.6/site-packages/nmigen-0.1-py3.6.egg/nmigen/back/pysim.py", line 329, in <listcomp>
    stmts = [self.on_statement(stmt) for stmt in stmts]
  File "/home/adam/.local/lib/python3.6/site-packages/nmigen-0.1-py3.6.egg/nmigen/hdl/xfrm.py", line 172, in on_statement
    return self.on_Switch(stmt)
  File "/home/adam/.local/lib/python3.6/site-packages/nmigen-0.1-py3.6.egg/nmigen/back/pysim.py", line 319, in on_Switch
    cases.append((make_test(mask, value), self.on_statements(stmts)))
  File "/home/adam/.local/lib/python3.6/site-packages/nmigen-0.1-py3.6.egg/nmigen/back/pysim.py", line 329, in on_statements
    stmts = [self.on_statement(stmt) for stmt in stmts]
  File "/home/adam/.local/lib/python3.6/site-packages/nmigen-0.1-py3.6.egg/nmigen/back/pysim.py", line 329, in <listcomp>
    stmts = [self.on_statement(stmt) for stmt in stmts]
  File "/home/adam/.local/lib/python3.6/site-packages/nmigen-0.1-py3.6.egg/nmigen/hdl/xfrm.py", line 165, in on_statement
    return self.on_Assign(stmt)
  File "/home/adam/.local/lib/python3.6/site-packages/nmigen-0.1-py3.6.egg/nmigen/back/pysim.py", line 295, in on_Assign
    rhs   = self.rrhs_compiler(stmt.rhs)
  File "/home/adam/.local/lib/python3.6/site-packages/nmigen-0.1-py3.6.egg/nmigen/hdl/xfrm.py", line 100, in __call__
    return self.on_value(value)
  File "/home/adam/.local/lib/python3.6/site-packages/nmigen-0.1-py3.6.egg/nmigen/hdl/xfrm.py", line 92, in on_value
    new_value = self.on_ArrayProxy(value)
  File "/home/adam/.local/lib/python3.6/site-packages/nmigen-0.1-py3.6.egg/nmigen/back/pysim.py", line 201, in on_ArrayProxy
    elems  = list(map(self, value.elems))
  File "/home/adam/.local/lib/python3.6/site-packages/nmigen-0.1-py3.6.egg/nmigen/hdl/xfrm.py", line 100, in __call__
    return self.on_value(value)
  File "/home/adam/.local/lib/python3.6/site-packages/nmigen-0.1-py3.6.egg/nmigen/hdl/xfrm.py", line 73, in on_value
    new_value = self.on_Signal(value)
  File "/home/adam/.local/lib/python3.6/site-packages/nmigen-0.1-py3.6.egg/nmigen/back/pysim.py", line 93, in on_Signal
    value_slot = self.signal_slots[value]
  File "/home/adam/.local/lib/python3.6/site-packages/nmigen-0.1-py3.6.egg/nmigen/hdl/ast.py", line 961, in __getitem__
    return self._storage[key]
KeyError: <nmigen.hdl.ast.SignalKey (sig mem(0))>

Uncommenting the write_port fixes the example which then works as expected.
@whitequark whitequark added the bug label Jan 5, 2019
@adamgreig
Copy link
Contributor Author

Adding:

for signal in self.memory._array:
    f.add_driver(signal, self.domain)

to the bottom of ReadPort.get_fragment (which WritePort.get_fragment already has) resolves the issue without raising any warnings or errors, even in designs with a read and write port. Not sure if it's really the best solution though (is it OK for a signal to have multiple drivers? wouldn't that happen anyway in a design with more than one write port?). Happy to open a PR if you think it's OK.

@adamgreig
Copy link
Contributor Author

Ugh, the above change causes occasional failure in test_memory_async_read_write, around 60% of test runs fail on the final assert after Delay(1e-6) (the combinatorial write propagation). Perhaps a Python hash randomisation thing.

@whitequark whitequark added this to the 0.1 milestone Jun 28, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
0.1
Development

No branches or pull requests
2 participants
@adamgreig @whitequark