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

Cat(..)[slice] on LHS needs a proxy #20

Open
cr1901 opened this issue Jun 12, 2015 · 4 comments
Open

Cat(..)[slice] on LHS needs a proxy #20

cr1901 opened this issue Jun 12, 2015 · 4 comments

Comments

@cr1901
Copy link
Contributor

cr1901 commented Jun 12, 2015

When attempting to use Record.raw_bits() to create a bus to manipulate in further Migen expressions, Migen will generate the concatenation "in place" in the generated Verilog code for each use of the bus instead of assigning to a temporary bus.

E.g. Assuming a record with fields a, b, c, d of varying widths and used as inouts, Migen will generate:

 {a, b, c, d}[0] = my_wire;

for each use of the Signal created from Record.raw_bits(), as opposed to:

my_bus = {a,b,c,d}; 
assign my_bus[0] = my_wire;

While the generated Verilog code in the former may be syntactically valid, some vendors may reject such assignments anyway- Xilinx's compiler (when use_new_parser=YES, as it should be for Migen) is known to choke on these assignments.

See the following gist for the code I was trying to synthesize when I ran into this issue: https://gist.github.com/cr1901/c994d2d307a35f1344ac

@cr1901
Copy link
Contributor Author

cr1901 commented Jun 16, 2015

A workaround to using Record.raw_bits() is to iterate over a record's subsignals using Record.iter_flat(). Using the above gist as an example, replace the line
self.specials += [self.iobufs[i].get_tristate(self.pins[i])]
with the following lines in a separate loop after for i in range():

for subsig, _ in pads.iter_flat():
    siglen = flen(subsig)
    for i in range(siglen):
        self.specials += [self.iobufs[start + i].get_tristate(subsig[i])]
    start += siglen
    if(start >= 30): #iter_flat() is generator... only need 30 out of 32 signals.
        break

@jordens
Copy link
Member

jordens commented Jul 2, 2015

I suspect that the solution here is to reaffirm that migen does not guarantee syntactictally valid verilog.

@cr1901
Copy link
Contributor Author

cr1901 commented Sep 17, 2015

@jordens You may be right. FWIW, I have found another edge case where the same invalid Verilog idiom will be generated when dealing with tristates: https://gist.github.com/cr1901/3a666c635f956fa6a819

@jordens jordens changed the title Migen Verilog Compilation Error when using Record.raw_bits() Cat(..)[slice] on LHS needs a proxy Jun 21, 2017
@whitequark
Copy link
Contributor

Triage: fixed in nMigen.

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

No branches or pull requests

3 participants