Actually, the structure of your verilog looks just fine. It's the details that are wrong, in a lot of places. Here are some:
reg [3:1] y2, y1, Y2, Y1
Yes, the missing semicolon throws the compiler off. More to the point, [3:1]
tells it that each of these regs is three bits wide, but they're only one bit wide in the planning. Traditionally we use 0 for the least significant bit (such that the binary interpretation has weight 2^n at bit n). The parameter line would thus be [1:0]
, as it is you extend it to three bits wide.
always @(w,y2,y1)
always @(negedge Resetn, posedge Clock)
The sensitivity list is separated by or
, not comma.
case (y2)
A: if (w) Y2 = 0;
Y1 = 1;
else Y2 = 0;
Y1 = 0;
y2
in this case statement only matches one bit in your planning (three in the code, but that made less sense). You can concatenate bits using {y2,y1}
; in fact, extending the case to case ({y2,y1,w})
will let you use case matches like {A,1'b0}:
and remove the if
statements entirely.
Secondly, you are trying to manage groups of statements (both assignments to Y2 and Y1) with if
; doing so requires enclosing them with begin
and end
. Alternatively, you could make a wider assignment such as {Y2,Y1} <= B;
, which ends up more readable as it can use your named states.
Thirdly, assignment using =
can cause some confusion (it acts more like sequential languages, while <=
doesn't modify the meaning of a reg within your always). In this case, it is fine as the block is fully combinatorial and does not depend on its own outputs.
Finally (for the case
section), you can simply add more matches. You don't even need a default
match, but it's probably convenient to use default
to go to state A in this case.
always @(negedge Resetn, posedge Clock)
if (Resetn == 0) //something :/
else //something else :/
Something and something else would be register updates, such as {y2,y1} <= {Y2,Y1};
. It is the clock edge sensitivity that turns the regs into flipflops.
Finally, since you should now understand what defines a reg width, why don't you make two bit wide regs named state
and next_state
to replace {y2,y1}
and {Y2,Y1}
respectively?
A bus enables you to define values that are wider than one bit. If you want to store or transmit (in parallel) a value between 0 and 15, you need a 4-bit bus. An array lets you store multiple values under a single name.
The difference between "[7:0] data" and "data[7:0]" is that the first is a single 8-bit-wide value while the second is eight single-bit values.
It's totally normal and useful to create an array of busses. They're sometimes called memories, because you can use them to model ROM or RAM. Your synthesis software can usually recognize when you've defined something that looks like ROM or RAM (for example, reg [7:0] data[1023:0] is basically 1K by 8-bit RAM) and synthesize it as actual physical ROM or RAM on the chip.
To understand which construct to use, just ask yourself what you have. Do you have a single multi-bit value? That's a bus. Do you have a bunch of single-bit values that you want to reference using a single name? Then use an array. Do you have a bunch of multi-bit values? Then use a memory.
Best Answer
This is somewhat historical. Prior to SystemVerilog, you had to declare the loop index separately, and prior to Verilog-2001, you had to enclose a generate-for loop with the keywords
generate/endgenerate
This was to indicate that
i
was not an index variable, but was part of an elaboration unrolling construct. Verilog-2001 got rid of the need for the extra keywords because it could determine that the outer for-loop was a not procedural loop, and the inner for-loop was procedural.SystemVerilog added the ability to declare the loop iteration index variable inside the for-loop, but it still requires you to use the
genvar
index declaration to inidicate that it is not really a variable.