m-labs / migen

A Python toolbox for building complex digital hardware
https://m-labs.hk/migen
Other
1.23k stars 211 forks source link

mixing comb and sync assignments of a signal's bits doesn't work #50

Closed jordens closed 5 years ago

jordens commented 8 years ago
from migen import *

class TB(Module):
    def __init__(self, n):
        self.n = n
        self.r = Signal(n, reset=0b10)
        self.comb += self.r[0].eq(self.r[-1])
        self.sync += self.r[1:].eq(self.r)
        self.out = []

    def run(self):
        for i in range(2*self.n):
            self.out.append((yield self.r))
            yield

n = 3
tb = TB(n)
run_simulation(tb, tb.run(), vcd_name="t.vcd")
assert tb.out == [2, 4, 1, 2, 4, 1], tb.out
# AssertionError: [2, 2, 2, 2, 2, 2]

(originally #36)

whitequark commented 5 years ago

@jordens I don't think this can be made to work in general. What is the semantics of something like:

        self.comb += self.r[0].eq(self.r[-1])
        self.sync += self.r.part(x,1).eq(self.r)
jordens commented 5 years ago

What's part()?

whitequark commented 5 years ago

Indexed part select. r[x+:1] in Verilog.

When used on left-hand side, r[x+:w] it's equivalent to r[x+w-1:x], if such a construct was semantically valid.

whitequark commented 5 years ago

@jordens Unless you have a good argument for why this should be possible, I think that because of https://github.com/m-labs/migen/issues/50#issuecomment-457874029 it must be disallowed.

jordens commented 5 years ago

Is it an error? Is mixing clock domains allowed within a Record?

whitequark commented 5 years ago

Is it an error?

nMigen will not allow you to drive a signal from any two different domains, including comb and any clocked domain.

Is mixing clock domains allowed within a Record?

It is allowed, yes.

jordens commented 5 years ago

Ok. Sounds good.