Mixing blocking and non-blocking assign in Verilog (or not!)

Question

I'm implementing a simple serializer in Verilog, but I do not understand the nuances of when blocking assigns can cause problems. I'm specifically having trouble understanding part of this answer. "However, you should never use blocking assignments for synchronous communication, as this is nondeterministic."

I'm building a block that takes, as an input:

• A bit clock
• A 5-bit parallel data input (the value to be serialized)
• A "Data valid" signal that indicates valid 5-bit data is present

As an output, I have:

• Serial data out
• A "Complete" signal that indicates it's time for a new 5-bit value
• A "Transmitting" signal that's high whenever there's valid serial data going out on the bus

Whenever data valid goes high, the block starts outputting the 5-bit value, one bit a time, starting at the next rising edge of the bit clock. When the last bit is out on the wire, the block signals "complete" so a new 5-bit value can be made available.

Omitting some of the reset logic, the code to do this looks like this:

always @ (posedge clk) begin
    if(shiftIndex == 0) begin
        if(dataValid == 1) transmitting = 1; //Blocking assign
        else transmitting = 0; //Blocking assign
    end

   //Need the blocking assign up above to get this part to run
   //for the 1st bit
   if(transmitting == 1) begin
       shiftIndex <= shiftIndex + 1;
       dataOut <= data5b[shiftIndex];

       if(shiftIndex == 4) begin
           complete <= 1;
           shiftIndex <= 0;
       end
       else begin
           complete <= 0;
       end
   end
end

Now, I can write the block with all non-blocking assigns, but I feel that it hurts readability. That would look something like this:

always @ (posedge clk) begin
    if(shiftIndex == 0) begin
        if(dataValid == 1) begin
            transmitting <= 1; //Non-blocking now
            shiftIndex <= shiftIndex + 1;  //Duplicated code
            dataOut <= data5b[shiftIndex]; //Duplicated code
            complete <= 0;                 //Duplicated code
        end
        else transmitting <= 0;
    end

   //Now, this only runs for the 2nd, 3rd, 4th, and 5th bit.
   else if(transmitting == 1) begin
       shiftIndex <= shiftIndex + 1;
       dataOut <= data5b[shiftIndex];

       if(shiftIndex == 4) begin
           complete <= 1;
           shiftIndex <= 0;
       end
       else begin
           complete <= 0;
       end
   end
end

Both appear to do what I want in simulation, and I favor the 1st one because it's easier for me to read but since I don't understand why using blocking assignments for synchronous communication is nondeterministic, I'm worried that I've coded up a ticking time bomb

The Question: Am I doing something wrong in the 1st code that's going to blow up when I try to synthesize this? Is the 2nd code preferable despite being a bit harder (for me anyway) to read? Is there some 3rd thing I should be doing?

Answer 1

When using the blocking (=) assignment the value is available to use in the next line of code. This implies it is combinatorial and not driven from a flip-flop.

In simulation it looks like it is driven from a flip-flop because the block is only evaluated on positive clock edge, in reality it is not which might break the interface.

I am of the faction which says never mix styles, as it can be a problem in code reviews and refactoring. The refactor, if a module needs to output a new signal and it is seen that it already exists they just change to be an output. At first glance looked like it was a flip-flop because it was in a always @(posedge clk block.

Therefore I would recommend to NOT mix styles, but pull out the section that is combinatorial and put it in its own block. Does this still meet your requirements? if not then you would have had problem.

I do not see how data valid is controlled but it can change the output transmitting, potentially transmitting could also glitch as it is from a combinatorial decode, not driven cleanly from a flip-flop. The receiving interface might be async, glitches could cause lock up etc.

always @* begin
  if(shiftIndex == 0) begin
    if(dataValid == 1) transmitting = 1; //Blocking assign
    else transmitting = 0; //Blocking assign
  end
end

always @ (posedge clk) begin
  if(transmitting == 1) begin
    shiftIndex <= shiftIndex + 1;
    dataOut    <= data5b[shiftIndex];

   if(shiftIndex == 4) begin
       complete   <= 1;
       shiftIndex <= 0;
   end
   else begin
       complete   <= 0;
   end
  end
end

Answer 2

As far as correctness is concerned, there is no problem with mixing blocking and non-blocking assignments, but you need to have a clear understanding of which signal is sequential and which signal is combinational block (note that the inputs of this combinational block come either from other sequential blocks or primary inputs). Also, you need to decide if you want any latches in your design or not.

If you use blocking assignments for a variable that you don't mean to be sequential, make sure to always assign to it, otherwise, it might be interpreted as a sequential element.

In your first code, you don't assign to transmitting when (shiftIndex != 0). This implies the previous value of transmitting should be used when (shiftIndex != 0), hence it would be a sequential element. But you need its value in the current clock, hence you used a blocking assignment.

Below is another version of your code, where firstBit_comb for the first bit is used and is always assigned to.

always @ (posedge clk) begin
    //Default value to avoid sequentials. Will be overwritten later if necessary
    firstBit_comb = 0;
    if(shiftIndex == 0) begin
        if(dataValid == 1) begin 
            transmitting_seq <= 1;
            firstBit_comb = 1;
        end
        else begin
         transmitting_seq <= 0; 
         firstBit_comb = 1;
        end
    end

   //Need the blocking assign up above to get this part to run
   //for the 1st bit
   if(firstBit_comb || transmitting_seq) begin
       shiftIndex <= shiftIndex + 1;
       dataOut <= data5b[shiftIndex];

       if(shiftIndex == 4) begin
           complete <= 1;
           shiftIndex <= 0;
       end
       else begin
           complete <= 0;
       end
   end
end

It is however more clear if you separate sequential and combinational blocks. Note that the next state of your sequential elements is often an output of a combinational block.

//combinational block
always_comb
begin
    //The default value of next state is the previous state
    transmitting_next = transmitting_seq;

    //The default value of firstBit_comb=0. It would be overwritten if necessary
    firstBit_comb = 0; 

    if(shiftIndex == 0 && dataValid == 1) begin
        firstBit_comb = 1;
        transmitting_next = 1;
    end
    else begin
        firstBit_comb = 0;
        transmitting_next = 0;
    end

end

//Sequential block
always @ (posedge clk) begin
    //update transmitting_seq with its next state
    transmitting_seq <= transmitting_next; 
   if(firstBit_comb || transmitting_seq) begin
       shiftIndex <= shiftIndex + 1;
       dataOut <= data5b[shiftIndex];

       if(shiftIndex == 4) begin
           complete <= 1;
           shiftIndex <= 0;
       end
       else begin
           complete <= 0;
       end
   end
end