Constant bit vectors in a concat() expression

Support Bug
1

I am having difficulty with Verilog conversion of a concatenation expression:

ONE = bool(1)
ZERO = bool(0)
EIGHT_ZEROS = 0b1010101010101010
PREAMBLE_B = 0b10011100
PREAMBLE_M = 0b10010011
PREAMBLE_W = 0b10010110

parity = bdata[0] ^ bdata[1] ^ bdata[2]  ^ bdata[3]  ^ bdata[4]  ^ bdata[5]  ^ bdata[6]  ^ bdata[7] \
       ^ bdata[8] ^ bdata[9] ^ bdata[10] ^ bdata[11] ^ bdata[12] ^ bdata[13] ^ bdata[14] ^ bdata[15]

shiftReg.next[64:] = concat(bin(PREAMBLE_W), bin(EIGHT_ZEROS), \
    ONE, bdata[0],  ONE, bdata[1],  ONE, bdata[2],  ONE, bdata[3],  \
    ONE, bdata[4],  ONE, bdata[5],  ONE, bdata[6],  ONE, bdata[7],  \
    ONE, bdata[8],  ONE, bdata[9],  ONE, bdata[10], ONE, bdata[11], \
    ONE, bdata[12], ONE, bdata[13], ONE, bdata[14], ONE, bdata[15], \
    bin(0b10), bin(0b10), bin(0b10), ONE, bin(parity))

This is converted as

parity = (((((((((((((((bdata[0] ^ bdata[1]) ^ bdata[2]) ^ bdata[3]) ^ bdata[4]) ^ bdata[5]) ^ bdata[6]) ^ bdata[7]) ^ bdata[8]) ^ bdata[9]) ^ bdata[10]) ^ bdata[11]) ^ bdata[12]) ^ bdata[13]) ^ bdata[14]) ^ bdata[15]);
spdif_tx_1_shiftReg[64-1:0] <= {bin(150), bin(43690), 1, bdata[0], 1, bdata[1], 1, bdata[2], 1, bdata[3], 1, bdata[4], 1, bdata[5], 1, bdata[6], 1, bdata[7], 1, bdata[8], 1, bdata[9], 1, bdata[10], 1, bdata[11], 1, bdata[12], 1, bdata[13], 1, bdata[14], 1, bdata[15], bin(2), bin(2), bin(2), 1, bin(parity)};

I have minimal Verilog knowledge, but bin does not appear to be a valid operator (Vivado gives a synthesis error). If I strip those away manually and leave the numeric constants “naked”, Vivado says “concatenation with unsized literal; will interpret as 32 bits”—not what we want. Is this a bug in the Verilog conversion, and/or should I be expressing my literals some other way?

BTW it’s a shame literals like “0b101” cannot be used directly in a concat(), since they [obviously] have a defined bit width already, but I imagine this is a limitation from Python?

Thanks.

2

The bin(150) is not convertible, if you want to use a literal you will need to use a binary string '10010110'. Or you need to use the bin function outside the generators:

bpre = bin(PREAMBLE_W)
bzer = bin(EIGHT_ZEROS)

@always
# ...
    shift.next = concate(bpre, bzer, ...)

Regards,
Chris

3

Thanks Chris,

I’ve been having a huge amount of trouble with this concat. Hopefully once I learn the rules it will get easier. Still trying to wrap my head around the concept that you can have an expression inline (like bin(PREAMBLE_W)) in the concat, and that cannot be converted, whereas the exact same expression can be converted if it is first assigned to a temp variable (bpre in your example), and that variable is used in the concat.

At least I thought I had this figured out. But trying what you suggest, I cannot get past some conversion errors. E.g.,

bPREAMBLE_B = '10011100'

shiftReg.next[64:] = concat(bPREAMBLE_B, ...)

(remaining concat expression omitted for clarity)

gives me

Object type is not supported in this context: bPREAMBLE_B, <class 'str'>

I thought a string, consisting of ones and zeros, was precisely the object type required?

I’m also having problems with the parity calculation. Using the code from the first post (parity = an xor of lots of bits), and then using “parity” by itself in the concat, I get

concat: inappropriate argument type: <class 'int'>

I guess I could try making “parity” a boolean signal, but I don’t want to create a register for it; it’s supposed to be a temp variable for code clarity only, so I don’t have to put a lengthy boolean expression inside each concat.

4

@haunma yes, in the myhdl generators, the convertible subset, things can get tricky.

You seem to be running into some odd corner cases, I don’t know why that isn’t working (it isn’t working for me either but I have never ran into this scenario before). The quickest work around would be:

@myhdl.block
def top(a, b):
    bPREAMBLE_B = intbv('100111001')
    @always_comb
    def beh():
        b.next = concat(bPREAMBLE_B, '1001', a)
    return beh

a = Signal(intbv(0)[8:])
b = Signal(intbv(0)[8:])
inst = top(a, b)
inst.convert()

Out[18]: <myhdl._block._Block at 0x4626550>

%less top.v

// File: top.v
// Generated by MyHDL 1.0dev
// Date: Tue Oct  4 12:24:54 2016


`timescale 1ns/10ps

module top (
    a,
    b
);


input [7:0] a;
output [7:0] b;
wire [7:0] b;

assign b = {bPREAMBLE_B, 4'b1001, a};

endmodule
5

The tricky thing about concat is that it needs to know the bitwidth of each element that is being concat’d. Only intbv and binary str can be used. It seems to be a bug (limitation) that a passing a named reference to concat would fail.

A test should be submitted as a bug for this.

6

(Sometimes I wonder if I should quit my job and go into testing instead. I always seem to hit stupid corner cases without trying.)

I think what this, and my first question (about “constant - 1” constructions), have in common is that I am trying to use symbolic names for constants, and in so doing get bogged down in the Python/MyHDL type system. For example, I now realize that intbv('10010110') has a defined bit width whereas intbv(150) does not. Also, intbv(<some expression of single-bit slices>) seems not to have a bit width until you explicitly take a one-bit slice of the result. Stuff like this is really non-intuitive to me.

It would be nice if one could define macros, so you wouldn’t have to worry about some expression in literal form working, and that exact same expression, when assigned to a variable, not working, e.g. inside a concat().

You mentioned submitting a test for the last bug (possible bug? quasi-bug?) too. I’m not sure I know enough about MyHDL to do so. What would be the procedure?

(Edit: Defining my bitfield constants as intbv(some bit string) works, but in my case it is generating a reg for each one. Not knowing much about Verilog, I suppose that Vivado will optimize these out? But as I think about this some more, I think what is really needed is for the MyHDL conversion routine to support macro-style replacement of tokens. I.e., instead of figuring out some way to put a name in the concat() and have that name defined elsewhere, in Verilog, I would like to have that name replaced with the constant expression it represents, during conversion. I guess this would mean having MyHDL understand which values are constants and which are not. This would fix the “constant - 1” issue I was having earlier, also. Does that make sense?)

7

For the myhdl converter to determine if it should execute a function and use the result or convert the function is a large scope project. In this particular case I can see were you would want to use it but the correct solution is to use it outside of the generator (in what we can elaboration). That requires the bug to be fixed :slight_smile:

These should help if you want to contribute a test case for a possible bug:
http://dev.myhdl.org/guide/guide.html
http://dev.myhdl.org/guide/guide_tests.html

I haven’t look at your code snips in detail but one of the things we don’t want to get hung up on is writing Verilog constructs in MyHDL. Many things that would make sense in Verilog don’t necessarily make sense in MyHDL. But developing (or overcoming) that intuition can require some work.

8

I notice my example didn’t work either, it used bPREAMBLE literally without defining it.

9

This is how I would write the snippet (assuming I didn’t make a dumb mistake) and it has some hacks to work around the converters limitations.

@myhdl.block
def build_packet(bdata, pkt):
    nbits = len(bdata)
    
    @always_comb
    def beh():
        # kinda a work around, usually these would be outside
        # the generators and could be strings but there is a 
        # limitation in the converter
        EIGHT_ZEROS = intbv('1010101010101010')
        PREAMBLE_B = intbv('10011100')
        PREAMBLE_M = intbv('10010011')
        PREAMBLE_W = intbv('10010110')

        # generate a parity bit
        parity = bdata[0]
        for ii in range(1, nbits):
            parity = parity ^ bdata[ii]
        
        onedata = intbv(-1)[2*nbits:]
        for ii in range(nbits):
            onedata[2*ii-1] = bdata[ii] 

        head = concat(PREAMBLE_W, EIGHT_ZEROS)
        tail = concat('10', '10', '10', '1')

        # not it should be concat'n non string things
        pkt.next = concat(head, onedata, tail, bool(parity))

    return beh

always @(bdata) begin: BUILD_PACKET_BEH
    reg [8-1:0] PREAMBLE_M;
    reg [8-1:0] PREAMBLE_W;
    integer ii;
    reg [24-1:0] head;
    reg [16-1:0] EIGHT_ZEROS;
    reg [12-1:0] onedata;
    reg [7-1:0] tail;
    reg [8-1:0] PREAMBLE_B;
    integer parity;
    EIGHT_ZEROS = 16'b1010101010101010;
    PREAMBLE_B = 8'b10011100;
    PREAMBLE_M = 8'b10010011;
    PREAMBLE_W = 8'b10010110;
    parity = bdata[0];
    for (ii=1; ii<6; ii=ii+1) begin
        parity = (parity ^ bdata[ii]);
    end
    onedata = 12'hfff;
    for (ii=0; ii<6; ii=ii+1) begin
        onedata[((2 * ii) - 1)] = bdata[ii];
    end
    head = {PREAMBLE_W, EIGHT_ZEROS};
    tail = {2'b10, 2'b10, 2'b10, 1'b1};
    pkt = {head, onedata, tail, (parity != 0)};
end

To fix the issue, I think a conditional for string constants needs to go here. If it check for string_types like it checks for integer_types it should be ok (and not break anything else). That would allow named string constants to be used whereas they appear unsupported at this time. If you cast everything to an int it might work