Verilog Tips and Traps...

Cluso99

Silly things slip by when you don't fully understand everything because often you look in the wrong places.

This caught me out for a few days

wire [[s][COLOR=#ff0000]1[/COLOR][/s][COLOR=#0000cd]3[/COLOR]:0][31:0] usr_r    = {  s,            // [s][COLOR=#ff0000]usr0[/COLOR][/s] [COLOR=#0000cd]usr3[/COLOR]
                                d,            // [s][COLOR=#ff0000]usr1[/COLOR][/s] [COLOR=#0000cd]usr2[/COLOR]
                                23'b0, p,     // [s][COLOR=#ff0000]usr2[/COLOR][/s] [COLOR=#0000cd]usr1[/COLOR]
                                26'b0, i };   // [s][COLOR=#ff0000]usr3[/COLOR][/s] [COLOR=#0000cd]usr0[/COLOR]

assign r            = i[5]                    ? add_r
                    : i[4]                    ? log_r
                    : i[3]                    ? rot_r
                    : i[2]                    ? usr_r[i[1:0]]
                    : run || ~&p[8:4]         ? bus_q
                                              : 32'b0;    // write 0's to last 16 registers during load;

What I am doing is creating a result in cog_alu.v for the 4 different USR (MUL/MULS/ENC/ONES) unused instructions.
The assign line :i[2] ? usr_r[i[1:0]] selects one of 4 sets of 32 bit results, depending on the lowest 2 bits of the opcode.

The bug...
wire [3:0][31:0] usr_r = { s, // usr0
The wire [3:0] is for 4 sets/range (ie it's not wire [1:0] as a binary of 4)

Postedit: Discovered another problem...
wire[3:0]....
reverses the index ie it is 3 downto 0

ozpropdev

Thanks Ray,
Ah yes, it always seems to be the little things that bite!
The joys of coding

rjo__

Since this thread is destined to become a sticky

Identification of programmed FPGA images:

http://forums.parallax.com/showthread.php/157175-Identification-of-programmed-FPGA-images

Documentation of sources:


http://forums.parallax.com/showthread.php/157071-Any-brainiacs-willing-to-better-comment-P1V-code-for-us-mere-mortals

KeithE

Here's another way to write that (unchecked code here) - note I tend to put in extra begins/ends because it saves some errors when you add code later and forget to add them in. The following should be plain old verilog with usr_r and r defined as type reg.

always @(*) begin
    case (i[1:0])
        2'd0: usr_r = {26'b0, i};
        2'd1: usr_r = {23'b0, p};
        2'd2: usr_r = d;
        2'd3: usr_r = s;
    endcase
end

always @(*) begin
    if (i[5]) begin
        r = add_r;
    end
    else if (i[4]) begin
        r = log_r;
    end
    else if (i[3]] begin
        r = rot_r;
    end
    else if (i[2]) begin
        r = usr_r;
    end
    else if (run || ~&p[8:4]) begin
        r = bus_q;
    end
    else begin
        r = 32'b0;
    end
end

Cluso99

Note the second fix to my top post

jac_goudsmit

Cluso99 wrote: »

wire [[s][COLOR=#ff0000]1[/COLOR][/s][COLOR=#0000cd]3[/COLOR]:0][31:0] usr_r    = {  s,            // [s][COLOR=#ff0000]usr0[/COLOR][/s] [COLOR=#0000cd]usr3[/COLOR]
                                d,            // [s][COLOR=#ff0000]usr1[/COLOR][/s] [COLOR=#0000cd]usr2[/COLOR]
                                23'b0, p,     // [s][COLOR=#ff0000]usr2[/COLOR][/s] [COLOR=#0000cd]usr1[/COLOR]
                                26'b0, i };   // [s][COLOR=#ff0000]usr3[/COLOR][/s] [COLOR=#0000cd]usr0[/COLOR]

As a clarification, I would like to add:

Creating a wire array (or reg array or input array or whatever) using the [n:m] notation creates a bus that's (1+ n - m) or (1 + m - n) bits wide (depending on whether n is larger than m). If you create a two-dimensional array (e.g. wire [3:0][31:0] usr_r) you basically create a bus of buses. If you would change the wire into a reg, it would become a memory.

The piece of code quoted above uses a concatenation operation (values in braces, separated by commas) to define the bits. The sizes of the items in the braces can vary. So the above code generates a long list of 4*32=128 bits from the values of s, d, p and i, that could be represented as:

ssssssssssssssssssssssssssssssssdddddddddddddddddddddddddddddddd00000000000000000000000ppppppppp00000000000000000000000000iiiiii

Note that it's important that in this case s and d are 32 bits, p must be 9 bits (because apparently it's padded with 23 bits) and i must be 6 bits (because it's padded with 23 bits). If you use the wrong bit lengths in the concatenation, the compiler will misalign your data (and possibly pad extra zero-bits) and probably without warning you. And if the resulting array (in this case: wire) isn't big enough (1:0 instead of 3:0) the extra bits will just be cut off without warning.

===Jac

KeithE

This is why I suggested using a case statement which should create the same hardware but perhaps be less error prone, and work with older tools.

Ariba

KeithE wrote: »

Here's another way to write that (unchecked code here) - note I tend to put in extra begins/ends because it saves some errors when you add code later and forget to add them in. The following should be plain old verilog with usr_r and r defined as type reg.

always @(*) begin
    case (i[1:0])
        2'd0: usr_r = {26'b0, i};
        2'd1: usr_r = {23'b0, p};
        2'd2: usr_r = d;
        2'd3: usr_r = s;
    endcase
end

always @(*) begin
    if (i[5]) begin
        r = add_r;
    end
    else if (i[4]) begin
        r = log_r;
    end
    else if (i[3]] begin
        r = rot_r;
    end
    else if (i[2]) begin
        r = usr_r;
    end
    else if (run || ~&p[8:4]) begin
        r = bus_q;
    end
    else begin
        r = 32'b0;
    end
end

How does "ALWAYS @(*)" work? It seems not to generate a combinatorial multiplexer. As you said you need to define the target as a register, so there must be a clock - but what clock is used?

Chips way with 2-dimensional wires generates a non clocked combinatorial multiplexer, so I think this can not be a direct replacement.

Andy

Seairth

Ariba wrote: »

How does "ALWAYS @(*)" work? It seems not to generate a combinatorial multiplexer. As you said you need to define the target as a register, so there must be a clock - but what clock is used?

Chips way with 2-dimensional wires generates a non clocked combinatorial multiplexer, so I think this can not be a direct replacement.

In the above code, the "ALWAYS @(*)", the asterisk is a wildcard for the event list (added in Verilog 2001, also called sensitivity list). In the first ALWAYS block, this is effectively the same as ALWAYS @(i[1:0]). In the second ALWAYS block, this is effectively the same as ALWAYS @(i[5:2] or p[8:4] or run). Obviously, the asterisk version is easier to type, maintain, etc. Also, these are not clocked ALWAYS blocks. They "execute" as soon as any parameters in the event/sensitivity list change value.

pik33

And you can always write simply:

always

  begin
 //some code
 end

Ale

an always @(*) or similar with continuous assignment '=' will generate combinatorial logic, gates and no registers. But it could create latches in the case that not all cases are covered i.e. add else's to all if's .

always @(*)
  if (a == 1'b0)
    out = 3'd5;

that will generate a latch for out because it is unknown (means will keep the value) what happens when a is not zero.

The best is to initialize all used signals and then do the conditionals:

always @(*)
  begin
    out = 3'd0; // default value
    if (a == 1'b0)
      out = 3'd5;
  end

That avoids the latch

KeithE

Ale - good point about latches. One should always scan the lint or synthesis report and be on the lookout. And always put in default cases (I skipped that above.) Before @(*) people often used verilog mode for emacs to fill them in.

Also you can put combinatorial logic into a function and call it. e.g. create a mux4to1 function which can be reused whenever you need a 4to1 mux.

Ariba

Thank you all for the explanations. But there are still a few questions:

Seairth wrote: »

In the above code, the "ALWAYS @(*)", the asterisk is a wildcard for the event list (added in Verilog 2001, also called sensitivity list). In the first ALWAYS block, this is effectively the same as ALWAYS @(i[1:0])....

Must the sensitivity list not also include all the signals that get multiplexed? The register must be updated not only if the select signals changes, but also if one of the multiplexed signal changes. This would result in a very big sensitivity list.

If I take the simplest example, a 2:1 multiplexer, then I expect a synthesis result shown in the following picture.
A combinatorial logic without a register just needs two ANDs,an OR and an Inverter. In an FPGA this would be implemented as a single LUT with 3 inputs and one output.
With a sensitivity list some kind of Change-detector is needed to clock the register, I have no idea how this is implemented.

Ale wrote: »

an always @(*) or similar with continuous assignment '=' will generate combinatorial logic, gates and no registers. But it could create latches in the case that not all cases are covered i.e. add else's to all if's .
....

Hmm, but if I declare the Q output as a wire, I get always the error message that I need to define Q as a register. Do I need to declare it as register and just have to trust in the compiler that the register gets removed later?

Andy

Ale

Ariba,

that is a good point, Any assignment in an always has to be to a register, even as destination for combinatorial logic. You cannot use a wire in an always as left hand of an assignment. The sensitivity list is only for the inputs not for the outputs. Latches are removed when all combinations of inputs are taken into account (using for instance a default value for the outputs). The simulator works differently as the compiler and there are things that you cannot synthesize because everything happens in parallel in an always instead of sequentially as in the simulator, that could bring some extra gotchas

This BAD code:

always @(*)
  begin
    b = ~q;
    q = 3'd0;
  end

note that q is assign after being used, for the synthesizer this is a no issue, because things happen in parallel... for the simulator otoh, b gets an old value for a small amount of time...

pik33

There is another verilog trap: = vs <=

initial
  begin
  a<=1;
  b<=0;
  c<=0;
  end

// variant 1

always @(posedge clk)
 begin
 a<=0;
 b<=a;
 c<=b;
 end

// variant 2

always @(posedge clk)
 begin
 a=0;
 b=a;
 c=b;
 end

In the variant 1 after first clock we will have a=0,b=1,c=0. After the second clock a=0, b=0, c=1. After third clock, a=b=c=0

In the variant 2 we will have a=b=c=0 after first clock.

They say you should use <= only, but both are synthesable - you are only not allowed to mix <= and = to one register in the same process.
Both are useful. I put the vga circuit in this forum. It used <=. Then I had a problem - the characters was shifted by one pixel. Then, the first pixel after the border can display some unwanted dots.
This was caused by using <=. To display a pixel you have to get a character definition (first clock), get a pixel from it (second clock) and display it (third). So many things have to be computed in advance. Or maybe it will be simpler using = where needed.
I used both of these resolutions. Some computing are done one pixel earlier Then I used = instead of <= when computing a pixel color. No more garbage, no more pixel shift.

The code will be published in its topic when I patch a bug - I computed too many things in advance and then the driver gets border colors from $FFFEF..$FFFF1 instead of $FFFF0..$FFFF2.

KeithE

pik33 - I recommend that people take a look at the two papers about nonblocking assignments on http://www.sunburst-design.com/papers/. They will explain the issues much better than I can. Many of the other papers are interesting.