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

Jump to bottom

cxxsim: synchronous assignment inside If statement is not executed #455

Closed
jfng opened this issue Jul 30, 2020 · 4 comments
Closed

cxxsim: synchronous assignment inside If statement is not executed #455

jfng opened this issue Jul 30, 2020 · 4 comments
Labels
bug simulator:cxxsim
Milestone
0.3

Comments

@jfng
Copy link
Member

jfng commented Jul 30, 2020

(with the cxxsim branch checked out at 060ad25)

Repro:

from nmigen import *
from nmigen.sim.cxxsim import *

m = Module()
i = Signal()
o = Signal()
x = Signal()

m.d.comb += x.eq(i)
with m.If(x):
    m.d.sync += o.eq(1)


def process():
    yield i.eq(1)
    yield Delay()
    yield
    yield Delay()
    assert (yield o) == 1

sim = Simulator(m)
sim.add_sync_process(process)
sim.add_clock(1e-6)
with sim.write_vcd("repro.vcd"):
    sim.run()

Output:

Traceback (most recent call last):
  File "repro.py", line 26, in <module>
    sim.run()
  File "/home/jf/src/nmigen/nmigen/sim/_core.py", line 202, in run
    while self.advance():
  File "/home/jf/src/nmigen/nmigen/sim/_core.py", line 191, in advance
    self._real_step()
  File "/home/jf/src/nmigen/nmigen/sim/cxxsim.py", line 124, in _real_step
    process.run()
  File "/home/jf/src/nmigen/nmigen/sim/_pycoro.py", line 121, in run
    self.coroutine.throw(exn)
  File "/home/jf/src/nmigen/nmigen/sim/_pycoro.py", line 62, in run
    command = self.coroutine.send(response)
  File "/home/jf/src/nmigen/nmigen/sim/_core.py", line 102, in wrapper
    yield from process()
  File "repro.py", line 21, in process
    assert (yield o) == 1
AssertionError
@whitequark
Copy link
Member

whitequark commented Jul 30, 2020 •
edited

Better repro (no Delay(), so clearly just an issue in synchronous logic):

def process():
    yield i.eq(1)
    yield
    yield
    assert (yield o) == 1

@whitequark whitequark added this to the 0.3 milestone Aug 26, 2020
@whitequark
Copy link
Member

Okay, I figured out what happens here. There are two bugs. The first was finishing delta cycles too early, fixed in 7ca1477. The second one is that there's actually not enough information to trigger the process that's waiting on Tick because clk is single-buffered. This will require some rethinking of the cxxrtl interface.

@whitequark whitequark mentioned this issue Aug 27, 2020
Closed
whitequark added a commit that referenced this issue Aug 27, 2020
@whitequark
wip (#439, #455)
32338f9
@whitequark
Copy link
Member

I've pushed a low-performance workaround to the cxxsim branch. With this workaround I am able to run the Minerva "hello" example.

@whitequark
Copy link
Member

After the last refactor (pushed to cxxsim branch) I'm confident the root cause of this issue is no longer present.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug simulator:cxxsim
Milestone
0.3
Development

No branches or pull requests
2 participants
@whitequark @jfng