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

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

Open
scted opened this issue Dec 6, 2020 · 8 comments
Open

Comments

@scted
Copy link

scted commented Dec 6, 2020

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

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

scted commented Dec 7, 2020

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

scted commented Dec 7, 2020

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

@sbourdeauducq
Copy link
Member

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

scted commented Dec 8, 2020

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

@sbourdeauducq
Copy link
Member

( {} )[] 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
Copy link
Author

scted commented Dec 8, 2020

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

scted commented Dec 8, 2020

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

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

No branches or pull requests

2 participants