Incrementing a counter variable in verilog: combinational or sequential

Question

I am implementing an FSM controller for a datapath circuit. The controller increments a counter internally. When I simulated the program below, the counter was never updated.

reg[3:0] counter;

//incrementing counter in combinational block
counter = counter + 4'b1;

However, on creating an extra variable, counter_next, as described in Verilog Best Practice - Incrementing a variable and incrementing the counter only in the sequential block, the counter gets incremented.

reg[3:0] counter, counter_next;

//sequential block
always @(posedge clk) 
   counter <= counter_next;

//combinational block
counter_next = counter + 4'b1;

Why doesn't the counter get incremented in the previous case? Anything I'm missing?

Answer 1

Ok. I'm assuming that you left some code out of your first example since that shouldn't even compile. However, I think I can elucidate the issue for you anyway.

In a block that looks like this:

always @(*) begin // or always @(counter)
    counter = counter + 4'b1;
end

there are two problems.

1) counter is never initialized. All 'reg' type variables are X at the start of simulation time so adding 1 to X is X.

2) This is what is considered a combinational loop. The block is sensitive to changes in 'counter' so even assuming that 'counter' was initialized to 0 the simulator would loop forever updating 'counter' and simulation time will never advance. i.e.

always block executes -> counter = 1
counter has changed
always block executes -> counter = 2
counter has changed
and so on...

If you were to put a $display statement in there you could see this loop occurring. Otherwise it'll just appear that the simulator is hung and no waves will be written.

The reason the 2nd example works is that you have a flip-flop breaking the combinational loop. At each clock edge 'counter' gets updated with the current value of 'counter_next'. Then the combinational block executes once (and only once) to calculate the new version of 'counter_next'.

You're still lacking an initialization of 'counter' through a reset clause or initial statement so for completeness.

reg [3:0] counter;
reg [3:0] counter_next;

always @(*) begin
   counter_next = counter + 1;
end

always @(posedge clk or negedge rst_l) begin
   if (!rst_l)
      counter <= 4'b0;
   else
      counter <= counter_next;
end