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

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

Comments

@jfng
Copy link
Contributor

@jfng 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 whitequark commented Jul 30, 2020

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
Copy link
Member

@whitequark whitequark commented Aug 27, 2020

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 added a commit that referenced this issue Aug 27, 2020
@whitequark
Copy link
Member

@whitequark whitequark commented Aug 27, 2020

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

@whitequark whitequark commented Sep 3, 2020

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

@whitequark whitequark closed this Sep 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants