m-labs / migen

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

Migen - Cat() simulation not matching verilog when Cat_object is sliced #228

Open scted opened 3 years ago

scted commented 3 years ago

Cat() not behaving as i expect when i use slices ... Cat_object[slice] ...

Have tried a number of permutations ... assigning to a slice ... Cat_object[n].eq(rhs) or from a slice ... lhs.eq(Cat_object[n])

The verilog (good and bad) has been tested in HW with both Vivado and Symbiflow ... it is not the toolchain that is a problem.

Possible that I do not understand how Cat is supposed to be used but the fact remains the simulation results differ from the produced verilog.

Code I used to test here: https://github.com/scted/Cat-demo

Snippet of one permutation (see TestRHSCat) here:

from migen import *
from migen.fhdl import verilog

class TestRHSCat(Module):
    def __init__(self):
        self.ins = ins = Signal(2)
        self.outs = outs = Signal(2)

        self.s0 = s0 = Signal()                                 
        self.s1 = s1 = Signal()

        #Cat_object of interest
        self.cat_bus = cat_bus = Cat(s0, s1)

        self.comb += cat_bus.eq(ins)

        #using using slices of cat_bus
        self.comb += outs[0].eq(cat_bus[0])
        self.comb += outs[1].eq(cat_bus[1])

Output verilog ends up with only MSB being connected to an input. For some reason, 4 wires ... slice_proxy0[1:0] and slice_proxy1[1:0] are declared):

/* Machine-generated using Migen */
module top(

);

reg [1:0] ins = 2'd0;
reg [1:0] outs;
wire s0;
wire s1;
wire [1:0] slice_proxy0;
wire [1:0] slice_proxy1;

// synthesis translate_off
reg dummy_s;
initial dummy_s <= 1'd0;
// synthesis translate_on

assign {s1, s0} = ins;

// synthesis translate_off
reg dummy_d;
// synthesis translate_on
always @(*) begin
    outs <= 2'd0;
    outs[0] <= slice_proxy0[0];
    outs[1] <= slice_proxy1[1];
// synthesis translate_off
    dummy_d <= dummy_s;
// synthesis translate_on
end
assign slice_proxy0 = {s1, s0};
assign slice_proxy1 = {s1, s0};

endmodule
sbourdeauducq commented 3 years ago

Does Verilog allow {s1, s0}[a:b]? If yes, this should be an easy fix. If not, we should probably just write a VHDL backend anyway, there are just too many of these issues with Verilog.

scted commented 3 years ago

Sounds like i'm not doing anything wrong in that I'm using 'Cat' as it is intended to be used.

Doesn't look hopeful for {x, y}[a:b]

Note: lines 20 and 21 were my edits

I haven't used verilog so don't really know if there are workarounds ... i might look more into the code for Cat and verilog.convert to see if i have any ideas.

I don't think Yosys will be supporting VHDL anytime soon so I doubt switching to VHDL will be adopted by the Symbiflow community.

image

scted commented 3 years ago

Love Migen BTW ... definitely delivers on the promise of making HDLs Pythonic ... hope this can get sorted out

sbourdeauducq commented 3 years ago

Doesn't look hopeful for {x, y}[a:b]

With parentheses maybe? ({x, y})[a:b] Otherwise a workaround is to assign the Cat to an intermediate signal and then slice it.

scted commented 3 years ago

( {} )[] not working either ... i'll try the other idea

sbourdeauducq commented 3 years ago

( {} )[] not working either ...

Sigh, Verilog is such a pain.

I don't think Yosys will be supporting VHDL anytime soon so I doubt switching to VHDL will be adopted by the Symbiflow community.

Is this any good? https://github.com/ghdl/ghdl-yosys-plugin

scted commented 3 years ago

i don't know about ghdl ... will look at it

the workaround worked ... assigning to an intermediate signal and slicing that

do we leave this open?

scted commented 3 years ago

you can use Cat().l as the intermediate if you're careful ... 'l' doesn't make sense as an identifier if you are accessing both sides (l and r) of the wire ...

Cat().l is not flat so this workaround not universal

>>> c = Cat(Signal(), Signal())
>>> s = Signal()
>>> cs = Cat(c, s)
>>> len(cs) == len(cs.l)
False