migen icon indicating copy to clipboard operation
migen copied to clipboard

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

Open cr1901 opened this issue 9 years ago • 4 comments

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 avatar Jun 12 '15 11:06 cr1901

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

cr1901 avatar Jun 16 '15 01:06 cr1901

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

jordens avatar Jul 02 '15 05:07 jordens

@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

cr1901 avatar Sep 17 '15 13:09 cr1901

Triage: fixed in nMigen.

whitequark avatar Jul 05 '19 14:07 whitequark